Re: [PATCH v12 06/17] drm/i915/guc/slpc: Allocate/initialize/release SLPC shared data

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

 



On Fri, 30 Mar 2018 10:31:51 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> wrote:

Populate SLPC shared data with required default values for slice count,
power source/plan, IA perf MSRs.

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 and Nick Hoath). Added WARN for checking if
    upper 32bits of GTT offset of shared object are zero. (Chris)
    Updated commit message and moved POWER_PLAN and POWER_SOURCE defn.
    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).

v5: SLPC vma map changes and removed explicit type conversions.(Chris).
    s/freq_unslice_max|min/unslice__max|min_freq.

v6: Commit message update. s/msr_value/val for reuse later.

v7: Set default values for tasks and min frequency parameters. Moved init
    with allocation of data so that post GuC load earlier params persist.

v8: Added check for SLPC status during cleanup of shared data. SLPC
    disabling is asynchronous and should complete within 10us.

v9: Enabling Balancer task in SLPC.

v10: Rebase.

v11: Rebase. Added lock specific to SLPC.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@xxxxxxxxx>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
---
 drivers/gpu/drm/i915/i915_drv.h       |   5 +
drivers/gpu/drm/i915/intel_guc_slpc.c | 204 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_slpc.h |   3 +
 3 files changed, 212 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5176801..d17e778 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2416,6 +2416,11 @@ intel_info(const struct drm_i915_private *dev_priv)
#define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
+#define IS_ULX_SKU(dev_priv) (IS_SKL_ULX(dev_priv) || IS_KBL_ULX(dev_priv))
+#define IS_ULT_SKU(dev_priv)	(IS_SKL_ULT(dev_priv) || \
+				 IS_KBL_ULT(dev_priv) || \
+				 IS_CFL_ULT(dev_priv))
+
 #define SKL_REVID_A0		0x0
 #define SKL_REVID_B0		0x1
 #define SKL_REVID_C0		0x2
diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c b/drivers/gpu/drm/i915/intel_guc_slpc.c
index 63f100c..974a3c0 100644
--- a/drivers/gpu/drm/i915/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/intel_guc_slpc.c
@@ -4,10 +4,203 @@
  * Copyright © 2015-2018 Intel Corporation
  */
+#include <asm/msr-index.h>
+
+#include "i915_drv.h"
 #include "intel_guc_slpc.h"
+static inline struct intel_guc *slpc_to_guc(struct intel_guc_slpc *slpc)
+{
+	return container_of(slpc, struct intel_guc, slpc);
+}
+
+static unsigned int slpc_get_platform_sku(struct drm_i915_private *dev_priv)

If you want to use 'slpc_' prefix for your functions, then always pass
struct intel_guc_slpc *slpc as first param

+{
+	enum slpc_platform_sku platform_sku;
+
+	if (IS_ULX_SKU(dev_priv))
+		platform_sku = SLPC_PLATFORM_SKU_ULX;
+	else if (IS_ULT_SKU(dev_priv))
+		platform_sku = SLPC_PLATFORM_SKU_ULT;
+	else
+		platform_sku = SLPC_PLATFORM_SKU_DT;

Do we really need to pass this to SLPC/GuC?

+
+	WARN_ON(platform_sku > 0xFF);

Maybe just define this function to return u16 ?

+
+	return platform_sku;
+}
+
+static unsigned int slpc_get_slice_count(struct drm_i915_private *dev_priv)

Is this really slpc specific function ?
Maybe this can be added as inline from intel_device_info.h ?

+{
+	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_mem_set_param(struct slpc_shared_data *data,
+			       u32 id,
+			       u32 value)
+{
+	data->override_params_set_bits[id >> 5] |= (1 << (id % 32));
+	data->override_params_values[id] = value;
+}
+
+static void slpc_mem_unset_param(struct slpc_shared_data *data,
+				 u32 id)
+{
+	data->override_params_set_bits[id >> 5] &= (~(1 << (id % 32)));
+	data->override_params_values[id] = 0;
+}
+
+static int slpc_mem_task_control(struct slpc_shared_data *data,
+				 u64 val, u32 enable_id, u32 disable_id)
+{
+	int ret = 0;
+
+	if (val == SLPC_PARAM_TASK_DEFAULT) {
+		/* set default */
+		slpc_mem_unset_param(data, enable_id);
+		slpc_mem_unset_param(data, disable_id);
+	} else if (val == SLPC_PARAM_TASK_ENABLED) {
+		/* set enable */
+		slpc_mem_set_param(data, enable_id, 1);
+		slpc_mem_unset_param(data, disable_id);
+	} else if (val == SLPC_PARAM_TASK_DISABLED) {
+		/* set disable */
+		slpc_mem_set_param(data, disable_id, 1);
+		slpc_mem_unset_param(data, enable_id);
+	} else {
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static void slpc_shared_data_init(struct intel_guc_slpc *slpc)
+{
+	struct intel_guc *guc = slpc_to_guc(slpc);
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct slpc_platform_info *platform_info;
+	struct slpc_shared_data *data;
+	struct page *page;
+	u64 val;
+
+	lockdep_assert_held(&slpc->lock);
+
+	page = i915_vma_first_page(slpc->vma);
+	data = kmap_atomic(page);
+	platform_info = &data->platform_info;
+
+	memset(data, 0, sizeof(struct slpc_shared_data));
+
+	data->shared_data_size = sizeof(struct slpc_shared_data);
+	data->global_state = SLPC_GLOBAL_STATE_NOT_RUNNING;
+	platform_info->sku = slpc_get_platform_sku(dev_priv);
+	platform_info->slice_count = slpc_get_slice_count(dev_priv);
+	platform_info->power_plan_source =
+			SLPC_POWER_PLAN_SOURCE(SLPC_POWER_PLAN_BALANCED,
+					       SLPC_POWER_SOURCE_AC);
+
+	rdmsrl(MSR_TURBO_RATIO_LIMIT, val);
+	platform_info->p0_freq = val;
+	rdmsrl(MSR_PLATFORM_INFO, val);
+	platform_info->p1_freq = val >> 8;
+	platform_info->pe_freq = val >> 40;
+	platform_info->pn_freq = val >> 48;
+
+	/* Enable only GTPERF task, Disable others */
+	val = SLPC_PARAM_TASK_ENABLED;
+	slpc_mem_task_control(data, val,
+			      SLPC_PARAM_TASK_ENABLE_GTPERF,
+			      SLPC_PARAM_TASK_DISABLE_GTPERF);
+
+	slpc_mem_task_control(data, val,
+			      SLPC_PARAM_TASK_ENABLE_BALANCER,
+			      SLPC_PARAM_TASK_DISABLE_BALANCER);
+
+	val = SLPC_PARAM_TASK_DISABLED;
+	slpc_mem_task_control(data, val,
+			      SLPC_PARAM_TASK_ENABLE_DCC,
+			      SLPC_PARAM_TASK_DISABLE_DCC);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GTPERF_THRESHOLD_MAX_FPS,
+			   0);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GTPERF_ENABLE_FRAMERATE_STALLING,
+			   0);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_ENABLE_IA_GT_BALANCING,
+			   1);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_ENABLE_ADAPTIVE_BURST_TURBO,
+			   0);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_ENABLE_EVAL_MODE,
+			   0);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_ENABLE_BALANCER_IN_NON_GAMING_MODE,
+			   1);
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+			   intel_gpu_freq(dev_priv,
+					  dev_priv->gt_pm.rps.efficient_freq));
+
+	slpc_mem_set_param(data,
+			   SLPC_PARAM_GLOBAL_MIN_GT_SLICE_FREQ_MHZ,
+			   intel_gpu_freq(dev_priv,
+					  dev_priv->gt_pm.rps.efficient_freq));
+
+	kunmap_atomic(data);
+}
+
+/**
+ * intel_guc_slpc_init() - Initialize the SLPC shared data structure.
+ * @slpc: pointer to intel_guc_slpc.
+ *
+ * This function will create object to be shared with GuC SLPC and
+ * initialize it with required initial parameter values for various
+ * SLPC knobs such as min frequency limit, enabling/disabling of SLPC
+ * tasks etc.
+ *
+ * Return: 0 on success, non-zero error code on failure.
+ */
+
 int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
 {
+	struct intel_guc *guc = slpc_to_guc(slpc);
+	u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
+	struct i915_vma *vma;
+
+	mutex_init(&slpc->lock);
+
+	mutex_lock(&slpc->lock);

Do we need this lock here ?

+
+	vma = slpc->vma;
+	if (!vma) {
+		/* Allocate shared data structure */
+		vma = intel_guc_allocate_vma(guc, size);
+		if (IS_ERR(vma)) {
+			DRM_ERROR("Shared data allocation failed\n");
+			mutex_unlock(&slpc->lock);
+			return PTR_ERR(vma);
+		}
+		slpc->vma = vma;
+	}
+
+	slpc_shared_data_init(slpc);
+
+	mutex_unlock(&slpc->lock);
+
 	return 0;
 }
@@ -20,6 +213,17 @@ void intel_guc_slpc_disable(struct intel_guc_slpc *slpc)
 {
 }
+/**
+ * intel_guc_slpc_fini() - Release the SLPC shared data structure.
+ * @slpc: pointer to intel_guc_slpc.
+ *
+ * This function will release the shared data. SLPC needs to be disabled
+ * prior to this.
+ */
 void intel_guc_slpc_fini(struct intel_guc_slpc *slpc)
 {
+	/* Release shared data structure */
+	mutex_lock(&slpc->lock);
+	i915_vma_unpin_and_release(&slpc->vma);
+	mutex_unlock(&slpc->lock);
 }
diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h b/drivers/gpu/drm/i915/intel_guc_slpc.h
index 81250c0..87dac07 100644
--- a/drivers/gpu/drm/i915/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/intel_guc_slpc.h
@@ -9,6 +9,9 @@
 #include <intel_guc_slpc_fwif.h>
struct intel_guc_slpc {
+	/* Protects access to vma and SLPC actions */
+	struct mutex lock;
+	struct i915_vma *vma;
 };
int intel_guc_slpc_init(struct intel_guc_slpc *slpc);
_______________________________________________
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