On Sat, Aug 20, 2016 at 10:39:09AM +0530, Sagar Arun Kamble wrote: > From: Tom O'Rourke <Tom.O'Rourke@xxxxxxxxx> > > SLPC shared data is used to pass information > to/from SLPC in GuC firmware. > > For Skylake, platform sku type and slice count > are identified from device id and fuse values. > > Support for other platforms needs to be added. > > v2: Update for SLPC interface version 2015.2.4 > intel_slpc_active() returns 1 if slpc initialized (Paulo) > v3: change default host_os to "Windows" > v4: Spelling fixes (Sagar Kamble and Nick Hoath) > v5: Added WARN for checking if upper 32bits of GTT offset > of shared object are zero. (ChrisW) > Changed function call from gem_allocate/release_guc_obj to > i915_guc_allocate/release_gem_obj. (Sagar) > Updated commit message and moved POWER_PLAN and POWER_SOURCE > definition from later patch. (Akash) > Add struct_mutex locking while allocating/releasing slpc shared > object. This was caught by CI BAT. Adding SLPC state variable > to determine if it is active as it not just dependent on shared > data setup. > Rebase with guc_allocate_vma related changes. > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@xxxxxxxxx> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_drv.h | 7 ++- > drivers/gpu/drm/i915/intel_guc.h | 2 + > drivers/gpu/drm/i915/intel_pm.c | 6 ++- > drivers/gpu/drm/i915/intel_slpc.c | 90 ++++++++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_slpc.h | 78 +++++++++++++++++++++++++++++++++ > 5 files changed, 178 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 16fe13d..c46d619 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1694,7 +1694,12 @@ bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy, > > static inline int intel_slpc_active(struct drm_i915_private *dev_priv) > { > - return 0; > + int ret = 0; > + > + if (dev_priv->guc.slpc.vma && dev_priv->guc.slpc.enabled) > + ret = 1; > + > + return ret; > } > > /* intel_pm.c */ > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h > index cd23c4e..4a551f6 100644 > --- a/drivers/gpu/drm/i915/intel_guc.h > +++ b/drivers/gpu/drm/i915/intel_guc.h > @@ -145,6 +145,8 @@ struct intel_guc { > > uint64_t submissions[I915_NUM_ENGINES]; > uint32_t last_seqno[I915_NUM_ENGINES]; > + > + struct intel_slpc slpc; > }; > > static inline int intel_slpc_enabled(void) > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 3b3f487..6dc33cf 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -6559,7 +6559,8 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv) > > void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv) > { > - if (intel_slpc_enabled()) > + if (intel_slpc_enabled() && > + dev_priv->guc.slpc.vma) > intel_slpc_cleanup(dev_priv); > else if (IS_VALLEYVIEW(dev_priv)) > valleyview_cleanup_gt_powersave(dev_priv); > @@ -6649,7 +6650,8 @@ void intel_enable_gt_powersave(struct drm_i915_private *dev_priv) > > mutex_lock(&dev_priv->rps.hw_lock); > > - if (intel_slpc_enabled()) { > + if (intel_slpc_enabled() && > + dev_priv->guc.slpc.vma) { > gen9_enable_rc6(dev_priv); > intel_slpc_enable(dev_priv); > if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) > diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c > index b2e8d91..ba5b23a 100644 > --- a/drivers/gpu/drm/i915/intel_slpc.c > +++ b/drivers/gpu/drm/i915/intel_slpc.c > @@ -22,17 +22,103 @@ > * > */ > #include <linux/firmware.h> > +#include <asm/msr-index.h> > #include "i915_drv.h" > #include "intel_guc.h" > > +static u8 slpc_get_platform_sku(struct drm_device *dev) > +{ > + enum slpc_platform_sku platform_sku; > + > + if (IS_SKL_ULX(dev)) > + platform_sku = SLPC_PLATFORM_SKU_ULX; > + else if (IS_SKL_ULT(dev)) > + platform_sku = SLPC_PLATFORM_SKU_ULT; dev_priv for the two of these, pass in drm_i915_private * instead. > + else > + platform_sku = SLPC_PLATFORM_SKU_DT; > + > + return (u8) platform_sku; (u8)platform_sku; But the cast isn't a good idea here. In the case where your enum ever grows to have so many entries that it no longer fits in an u8, or if you use negative values in the enum, you'd want a warning, not a cast that silences it. > +} > + > +static u8 slpc_get_slice_count(struct drm_device *dev) > +{ > + u8 slice_count = 1; > + > + if (IS_SKYLAKE(dev)) > + slice_count = INTEL_INFO(dev)->slice_total; dev_priv, dev_priv, drm_i915_private * passed in. > + > + return slice_count; > +} > + > +static void slpc_shared_data_init(struct drm_i915_private *dev_priv) > +{ > + struct drm_i915_gem_object *obj; > + struct page *page; > + struct slpc_shared_data *data; > + u64 msr_value; > + > + if (!dev_priv->guc.slpc.vma) > + return; > + > + obj = dev_priv->guc.slpc.vma->obj; > + > + page = i915_gem_object_get_page(obj, 0); > + if (page) { > + data = kmap_atomic(page); > + memset(data, 0, sizeof(struct slpc_shared_data)); > + > + data->slpc_version = SLPC_VERSION; > + data->shared_data_size = sizeof(struct slpc_shared_data); > + data->global_state = (u32) SLPC_GLOBAL_STATE_NOT_RUNNING; > + data->platform_info.platform_sku = slpc_get_platform_sku(&dev_priv->drm); > + data->platform_info.slice_count = slpc_get_slice_count(&dev_priv->drm); If you follow my advice above you can pass dev_priv here. > + data->platform_info.host_os = (u8) SLPC_HOST_OS_WINDOWS_8; > + data->platform_info.power_plan_source = > + (u8) SLPC_POWER_PLAN_SOURCE(SLPC_POWER_PLAN_BALANCED, > + SLPC_POWER_SOURCE_AC); > + rdmsrl(MSR_TURBO_RATIO_LIMIT, msr_value); > + data->platform_info.P0_freq = (u8) msr_value; > + rdmsrl(MSR_PLATFORM_INFO, msr_value); > + data->platform_info.P1_freq = (u8) (msr_value >> 8); > + data->platform_info.Pe_freq = (u8) (msr_value >> 40); > + data->platform_info.Pn_freq = (u8) (msr_value >> 48); No spaces after the cast. > + rdmsrl(MSR_PKG_POWER_LIMIT, msr_value); > + data->platform_info.package_rapl_limit_high = > + (u32) (msr_value >> 32); > + data->platform_info.package_rapl_limit_low = (u32) msr_value; And again for these. > + > + kunmap_atomic(data); > + } > +} > + > void intel_slpc_init(struct drm_i915_private *dev_priv) > { > - return; > + struct intel_guc *guc = &dev_priv->guc; > + struct i915_vma *vma; > + > + /* Allocate shared data structure */ > + vma = dev_priv->guc.slpc.vma; > + if (!vma) { > + vma = guc_allocate_vma(guc, > + PAGE_ALIGN(sizeof(struct slpc_shared_data))); > + if (IS_ERR(vma)) { > + DRM_ERROR("slpc_shared_data allocation failed\n"); > + i915.enable_slpc = 0; > + return; > + } > + > + dev_priv->guc.slpc.vma = vma; > + } > + > + slpc_shared_data_init(dev_priv); > } > > void intel_slpc_cleanup(struct drm_i915_private *dev_priv) > { > - return; > + struct intel_guc *guc = &dev_priv->guc; > + > + /* Release shared data structure */ > + i915_vma_unpin_and_release(&guc->slpc.vma); > } > > void intel_slpc_suspend(struct drm_i915_private *dev_priv) > diff --git a/drivers/gpu/drm/i915/intel_slpc.h b/drivers/gpu/drm/i915/intel_slpc.h > index ae52146..e951289 100644 > --- a/drivers/gpu/drm/i915/intel_slpc.h > +++ b/drivers/gpu/drm/i915/intel_slpc.h > @@ -24,6 +24,84 @@ > #ifndef _INTEL_SLPC_H_ > #define _INTEL_SLPC_H_ > > +#define SLPC_MAJOR_VER 2 > +#define SLPC_MINOR_VER 4 > +#define SLPC_VERSION ((2015 << 16) | (SLPC_MAJOR_VER << 8) | (SLPC_MINOR_VER)) > + > +enum slpc_global_state { > + SLPC_GLOBAL_STATE_NOT_RUNNING = 0, > + SLPC_GLOBAL_STATE_INITIALIZING = 1, > + SLPC_GLOBAL_STATE_RESETTING = 2, > + SLPC_GLOBAL_STATE_RUNNING = 3, > + SLPC_GLOBAL_STATE_SHUTTING_DOWN = 4, > + SLPC_GLOBAL_STATE_ERROR = 5 > +}; > + > +enum slpc_host_os { > + SLPC_HOST_OS_UNDEFINED = 0, > + SLPC_HOST_OS_WINDOWS_8 = 1, > +}; > + > +enum slpc_platform_sku { > + SLPC_PLATFORM_SKU_UNDEFINED = 0, > + SLPC_PLATFORM_SKU_ULX = 1, > + SLPC_PLATFORM_SKU_ULT = 2, > + SLPC_PLATFORM_SKU_T = 3, > + SLPC_PLATFORM_SKU_MOBL = 4, > + SLPC_PLATFORM_SKU_DT = 5, > + SLPC_PLATFORM_SKU_UNKNOWN = 6, > +}; > + > +enum slpc_power_plan { > + SLPC_POWER_PLAN_UNDEFINED = 0, > + SLPC_POWER_PLAN_BATTERY_SAVER = 1, > + SLPC_POWER_PLAN_BALANCED = 2, > + SLPC_POWER_PLAN_PERFORMANCE = 3, > + SLPC_POWER_PLAN_UNKNOWN = 4, > +}; > + > +enum slpc_power_source { > + SLPC_POWER_SOURCE_UNDEFINED = 0, > + SLPC_POWER_SOURCE_AC = 1, > + SLPC_POWER_SOURCE_DC = 2, > + SLPC_POWER_SOURCE_UNKNOWN = 3, > +}; > + > +#define SLPC_POWER_PLAN_SOURCE(plan, source) ((plan) | ((source) << 6)) > +#define SLPC_POWER_PLAN(plan_source) ((plan_source) & 0x3F) > +#define SLPC_POWER_SOURCE(plan_source) ((plan_source) >> 6) > + > +struct slpc_platform_info { > + u8 platform_sku; > + u8 slice_count; > + u8 host_os; > + u8 power_plan_source; > + u8 P0_freq; > + u8 P1_freq; > + u8 Pe_freq; > + u8 Pn_freq; > + u32 package_rapl_limit_high; > + u32 package_rapl_limit_low; > +} __packed; > + > +#define SLPC_MAX_OVERRIDE_PARAMETERS 192 > +#define SLPC_OVERRIDE_BITFIELD_SIZE ((SLPC_MAX_OVERRIDE_PARAMETERS + 31) / 32) > + > +struct slpc_shared_data { > + u32 slpc_version; > + u32 shared_data_size; > + u32 global_state; > + struct slpc_platform_info platform_info; > + u32 task_state_data; > + u32 override_parameters_set_bits[SLPC_OVERRIDE_BITFIELD_SIZE]; > + u32 override_parameters_values[SLPC_MAX_OVERRIDE_PARAMETERS]; > +} __packed; > + > +struct intel_slpc { > + struct i915_vma *vma; > + bool enabled; > +}; > + > /* intel_slpc.c */ > void intel_slpc_init(struct drm_i915_private *dev_priv); > void intel_slpc_cleanup(struct drm_i915_private *dev_priv); With those changes: Reviewed-by: David Weinehall <david.weinehall@xxxxxxxxxxxxxxx> -- /) David Weinehall <tao@xxxxxxxxxx> /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/ (/ Full colour fire (/ _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx