Re: [PATCH v4 10/26] drm/i915/slpc: Allocate/Release/Initialize SLPC shared data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@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().
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


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux