On Fri, Sep 09, 2016 at 06:21:29PM +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.
>
> 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@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@xxxxxxxxxxxxxxx>
> 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 | 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().
> static inline int intel_slpc_active(struct drm_i915_private *dev_priv)
bool.
> {
> - 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).
> + 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?
> +
> + 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.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
