[Intel-gfx] [PATCH v4 10/26] drm/i915/slpc: Allocate/Release/Initialize SLPC shared data
Kamble, Sagar A
sagar.a.kamble at intel.com
Thu Sep 15 10:41:28 UTC 2016
On 9/9/2016 10:38 PM, Chris Wilson wrote:
> On Fri, Sep 09, 2016 at 06:21:29PM +0530, Sagar Arun Kamble wrote:
>> From: Tom O'Rourke <Tom.O'Rourke at intel.com>
>>
>> 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.
>>
>> v1: Update for SLPC interface version 2015.2.4
>> intel_slpc_active() returns 1 if slpc initialized (Paulo)
>> change default host_os to "Windows"
>> Spelling fixes (Sagar Kamble and Nick Hoath)
>> 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.
>>
>> v2: WARN_ON for platform_sku validity and space changes. (David)
>> Checkpatch update.
>>
>> v3: Fixing WARNING in igt at drv_module_reload_basic found in trybot BAT
>> with SLPC Enabled.
>>
>> v4: Updated support for GuC v9. s/slice_total/hweight8(slice_mask)/ (Dave).
>>
>> Reviewed-by: David Weinehall <david.weinehall at linux.intel.com>
>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>> 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 | 88 ++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_slpc.h | 99 +++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 199 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index cf9aa24..796c52f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1707,7 +1707,12 @@ bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
>>
> I am going to need an idiot's guide here as to the difference between
> enabled() and active().
Idea was to set active only when shared data is initialized. enabled was
used to do initial/final setup.
Will change this and make consistent everywhere by keeping only one state.
>
>> static inline int intel_slpc_active(struct drm_i915_private *dev_priv)
> bool.
will update
>
>> {
>> - 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 83dec66..6e24e60 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 d187066..2211f7b 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -6656,7 +6656,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);
>> @@ -6746,7 +6747,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 be9e84c..972db18 100644
>> --- a/drivers/gpu/drm/i915/intel_slpc.c
>> +++ b/drivers/gpu/drm/i915/intel_slpc.c
>> @@ -22,15 +22,103 @@
>> *
>> */
>> #include <linux/firmware.h>
>> +#include <asm/msr-index.h>
>> #include "i915_drv.h"
>> #include "intel_guc.h"
>>
>> +static unsigned int slpc_get_platform_sku(struct drm_i915_private *dev_priv)
>> +{
>> + enum slpc_platform_sku platform_sku;
>> +
>> + if (IS_SKL_ULX(dev_priv))
>> + platform_sku = SLPC_PLATFORM_SKU_ULX;
>> + else if (IS_SKL_ULT(dev_priv))
>> + platform_sku = SLPC_PLATFORM_SKU_ULT;
>> + else
>> + platform_sku = SLPC_PLATFORM_SKU_DT;
>> +
>> + WARN_ON(platform_sku > 0xFF);
>> +
>> + return platform_sku;
>> +}
>> +
>> +static unsigned int slpc_get_slice_count(struct drm_i915_private *dev_priv)
>> +{
>> + unsigned int slice_count = 1;
>> +
>> + if (IS_SKYLAKE(dev_priv))
>> + slice_count = hweight8(INTEL_INFO(dev_priv)->sseu.slice_mask);
>> +
>> + 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);
> page = i915_vma_first_pgae(dev_priv->guc.slpc.vma);
>
> and cannot be NULL (by construction in guc_allocate_vma).
will update
>> + if (page) {
>> + data = kmap_atomic(page);
>> + memset(data, 0, sizeof(struct slpc_shared_data));
>> +
>> + data->shared_data_size = sizeof(struct slpc_shared_data);
>> + data->global_state = (u32)SLPC_GLOBAL_STATE_NOT_RUNNING;
>> + data->platform_info.platform_sku =
>> + (u8)slpc_get_platform_sku(dev_priv);
>> + data->platform_info.slice_count =
>> + (u8)slpc_get_slice_count(dev_priv);
>> + 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);
> The u8 et al are implied anyway, are they not?
yes. will update
>> +
>> + kunmap_atomic(data);
>> + }
>> +}
>> +
>> void intel_slpc_init(struct drm_i915_private *dev_priv)
>> {
>> + struct intel_guc *guc = &dev_priv->guc;
>> + struct i915_vma *vma;
>> +
>> + /* Allocate shared data structure */
>> + vma = dev_priv->guc.slpc.vma;
>> + if (!vma) {
> There's always something fishy about init routines called multiple
> times.
intel_slpc_init will be called only once during intel_init_gt_powersave
> -Chris
>
More information about the Intel-gfx
mailing list