diff mbox series

[18/19] drm/i915/guc: Support OA when Wa_16011777198 is enabled

Message ID 20220823204155.8178-19-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series Add DG2 OA support | expand

Commit Message

Umesh Nerlige Ramappa Aug. 23, 2022, 8:41 p.m. UTC
From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>

There is a w/a to reset RCS/CCS before it goes into RC6. This breaks
OA. Fix it by disabling RC6.

Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h |  9 ++++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 45 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |  2 +
 drivers/gpu/drm/i915/i915_perf.c              | 29 ++++++++++++
 4 files changed, 85 insertions(+)

Comments

Dixit, Ashutosh Sept. 16, 2022, 9:41 p.m. UTC | #1
On Tue, 23 Aug 2022 13:41:54 -0700, Umesh Nerlige Ramappa wrote:
>
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
> There is a w/a to reset RCS/CCS before it goes into RC6. This breaks
> OA. Fix it by disabling RC6.

Need to mention DG2 in the commit message?

> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h |  9 ++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 45 +++++++++++++++++++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |  2 +
>  drivers/gpu/drm/i915/i915_perf.c              | 29 ++++++++++++
>  4 files changed, 85 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> index 4c840a2639dc..811add10c30d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
> @@ -128,6 +128,15 @@ enum slpc_media_ratio_mode {
>	SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO = 2,
>  };
>
> +enum slpc_gucrc_mode {
> +	SLPC_GUCRC_MODE_HW = 0,
> +	SLPC_GUCRC_MODE_GUCRC_NO_RC6 = 1,
> +	SLPC_GUCRC_MODE_GUCRC_STATIC_TIMEOUT = 2,
> +	SLPC_GUCRC_MODE_GUCRC_DYNAMIC_HYSTERESIS = 3,
> +
> +	SLPC_GUCRC_MODE_MAX,
> +};
> +
>  enum slpc_event_id {
>	SLPC_EVENT_RESET = 0,
>	SLPC_EVENT_SHUTDOWN = 1,
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index e1fa1f32f29e..23989f5452a7 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -642,6 +642,51 @@ static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
>		slpc->boost_freq = slpc->rp0_freq;
>  }
>
> +/**
> + * intel_guc_slpc_override_gucrc_mode() - override GUCRC mode
> + * @slpc: pointer to intel_guc_slpc.
> + * @mode: new value of the mode.
> + *
> + * This function will override the GUCRC mode.
> + *
> + * Return: 0 on success, non-zero error code on failure.
> + */
> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode)
> +{
> +	int ret;
> +	struct drm_i915_private *i915 = slpc_to_i915(slpc);
> +	intel_wakeref_t wakeref;
> +
> +	if (mode >= SLPC_GUCRC_MODE_MAX)
> +		return -EINVAL;
> +
> +	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
> +
> +	ret = slpc_set_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
> +	if (ret)
> +		drm_err(&i915->drm,
> +			"Override gucrc mode %d failed %d\n",
> +			mode, ret);
> +
> +	intel_runtime_pm_put(&i915->runtime_pm, wakeref);

nit but I think let's switch to with_intel_runtime_pm() since all other
slpc functions use that.

> +
> +	return ret;
> +}
> +
> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc)
> +{
> +	struct drm_i915_private *i915 = slpc_to_i915(slpc);
> +	int ret = 0;
> +
> +	ret = slpc_unset_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE);

Looks like slpc_unset_param() is not present so that needs to be added to
the patch too, otherwise probably doesn't even compile.

> +	if (ret)
> +		drm_err(&i915->drm,
> +			"Unsetting gucrc mode failed %d\n",
> +			ret);
> +
> +	return ret;
> +}
> +
>  /*
>   * intel_guc_slpc_enable() - Start SLPC
>   * @slpc: pointer to intel_guc_slpc.
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> index 82a98f78f96c..ccf483730d9d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
> @@ -42,5 +42,7 @@ int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val);
>  void intel_guc_pm_intrmsk_enable(struct intel_gt *gt);
>  void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
>  void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc);
> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode);
>
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 132c2ce8b33b..ce1b6ad4d107 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -208,6 +208,7 @@
>  #include "gt/intel_lrc.h"
>  #include "gt/intel_lrc_reg.h"
>  #include "gt/intel_ring.h"
> +#include "gt/uc/intel_guc_slpc.h"
>
>  #include "i915_drv.h"
>  #include "i915_file_private.h"
> @@ -1651,6 +1652,16 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>
>	free_oa_buffer(stream);
>
> +	/*
> +	 * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
> +	 */
> +	if (intel_guc_slpc_is_used(&gt->uc.guc) &&
> +	    intel_uc_uses_guc_rc(&gt->uc) &&
> +	    (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
> +	     IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))

Do these steppings need to be tweaked, otherwise ok as is too.
Umesh Nerlige Ramappa Sept. 16, 2022, 9:48 p.m. UTC | #2
On Fri, Sep 16, 2022 at 02:41:01PM -0700, Dixit, Ashutosh wrote:
>On Tue, 23 Aug 2022 13:41:54 -0700, Umesh Nerlige Ramappa wrote:
>>
>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>
>> There is a w/a to reset RCS/CCS before it goes into RC6. This breaks
>> OA. Fix it by disabling RC6.
>
>Need to mention DG2 in the commit message?
>

will do

>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>  .../drm/i915/gt/uc/abi/guc_actions_slpc_abi.h |  9 ++++
>>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c   | 45 +++++++++++++++++++
>>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h   |  2 +
>>  drivers/gpu/drm/i915/i915_perf.c              | 29 ++++++++++++
>>  4 files changed, 85 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
>> index 4c840a2639dc..811add10c30d 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
>> @@ -128,6 +128,15 @@ enum slpc_media_ratio_mode {
>>	SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO = 2,
>>  };
>>
>> +enum slpc_gucrc_mode {
>> +	SLPC_GUCRC_MODE_HW = 0,
>> +	SLPC_GUCRC_MODE_GUCRC_NO_RC6 = 1,
>> +	SLPC_GUCRC_MODE_GUCRC_STATIC_TIMEOUT = 2,
>> +	SLPC_GUCRC_MODE_GUCRC_DYNAMIC_HYSTERESIS = 3,
>> +
>> +	SLPC_GUCRC_MODE_MAX,
>> +};
>> +
>>  enum slpc_event_id {
>>	SLPC_EVENT_RESET = 0,
>>	SLPC_EVENT_SHUTDOWN = 1,
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> index e1fa1f32f29e..23989f5452a7 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>> @@ -642,6 +642,51 @@ static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
>>		slpc->boost_freq = slpc->rp0_freq;
>>  }
>>
>> +/**
>> + * intel_guc_slpc_override_gucrc_mode() - override GUCRC mode
>> + * @slpc: pointer to intel_guc_slpc.
>> + * @mode: new value of the mode.
>> + *
>> + * This function will override the GUCRC mode.
>> + *
>> + * Return: 0 on success, non-zero error code on failure.
>> + */
>> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode)
>> +{
>> +	int ret;
>> +	struct drm_i915_private *i915 = slpc_to_i915(slpc);
>> +	intel_wakeref_t wakeref;
>> +
>> +	if (mode >= SLPC_GUCRC_MODE_MAX)
>> +		return -EINVAL;
>> +
>> +	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>> +
>> +	ret = slpc_set_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
>> +	if (ret)
>> +		drm_err(&i915->drm,
>> +			"Override gucrc mode %d failed %d\n",
>> +			mode, ret);
>> +
>> +	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>
>nit but I think let's switch to with_intel_runtime_pm() since all other
>slpc functions use that.
>

will do
>> +
>> +	return ret;
>> +}
>> +
>> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc)
>> +{
>> +	struct drm_i915_private *i915 = slpc_to_i915(slpc);
>> +	int ret = 0;
>> +
>> +	ret = slpc_unset_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE);
>
>Looks like slpc_unset_param() is not present so that needs to be added to
>the patch too, otherwise probably doesn't even compile.
>

yes, looks like a recent patch removed it. I will add it back here.

>> +	if (ret)
>> +		drm_err(&i915->drm,
>> +			"Unsetting gucrc mode failed %d\n",
>> +			ret);
>> +
>> +	return ret;
>> +}
>> +
>>  /*
>>   * intel_guc_slpc_enable() - Start SLPC
>>   * @slpc: pointer to intel_guc_slpc.
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>> index 82a98f78f96c..ccf483730d9d 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
>> @@ -42,5 +42,7 @@ int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val);
>>  void intel_guc_pm_intrmsk_enable(struct intel_gt *gt);
>>  void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
>>  void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
>> +int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc);
>> +int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode);
>>
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 132c2ce8b33b..ce1b6ad4d107 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -208,6 +208,7 @@
>>  #include "gt/intel_lrc.h"
>>  #include "gt/intel_lrc_reg.h"
>>  #include "gt/intel_ring.h"
>> +#include "gt/uc/intel_guc_slpc.h"
>>
>>  #include "i915_drv.h"
>>  #include "i915_file_private.h"
>> @@ -1651,6 +1652,16 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>>
>>	free_oa_buffer(stream);
>>
>> +	/*
>> +	 * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
>> +	 */
>> +	if (intel_guc_slpc_is_used(&gt->uc.guc) &&
>> +	    intel_uc_uses_guc_rc(&gt->uc) &&
>> +	    (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
>> +	     IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))
>
>Do these steppings need to be tweaked, otherwise ok as is too.

Probably the G11 can be dropped, but I would leave it as is.

Thanks,
Umesh
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
index 4c840a2639dc..811add10c30d 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_slpc_abi.h
@@ -128,6 +128,15 @@  enum slpc_media_ratio_mode {
 	SLPC_MEDIA_RATIO_MODE_FIXED_ONE_TO_TWO = 2,
 };
 
+enum slpc_gucrc_mode {
+	SLPC_GUCRC_MODE_HW = 0,
+	SLPC_GUCRC_MODE_GUCRC_NO_RC6 = 1,
+	SLPC_GUCRC_MODE_GUCRC_STATIC_TIMEOUT = 2,
+	SLPC_GUCRC_MODE_GUCRC_DYNAMIC_HYSTERESIS = 3,
+
+	SLPC_GUCRC_MODE_MAX,
+};
+
 enum slpc_event_id {
 	SLPC_EVENT_RESET = 0,
 	SLPC_EVENT_SHUTDOWN = 1,
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..23989f5452a7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -642,6 +642,51 @@  static void slpc_get_rp_values(struct intel_guc_slpc *slpc)
 		slpc->boost_freq = slpc->rp0_freq;
 }
 
+/**
+ * intel_guc_slpc_override_gucrc_mode() - override GUCRC mode
+ * @slpc: pointer to intel_guc_slpc.
+ * @mode: new value of the mode.
+ *
+ * This function will override the GUCRC mode.
+ *
+ * Return: 0 on success, non-zero error code on failure.
+ */
+int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode)
+{
+	int ret;
+	struct drm_i915_private *i915 = slpc_to_i915(slpc);
+	intel_wakeref_t wakeref;
+
+	if (mode >= SLPC_GUCRC_MODE_MAX)
+		return -EINVAL;
+
+	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+
+	ret = slpc_set_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE, mode);
+	if (ret)
+		drm_err(&i915->drm,
+			"Override gucrc mode %d failed %d\n",
+			mode, ret);
+
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
+
+	return ret;
+}
+
+int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc)
+{
+	struct drm_i915_private *i915 = slpc_to_i915(slpc);
+	int ret = 0;
+
+	ret = slpc_unset_param(slpc, SLPC_PARAM_PWRGATE_RC_MODE);
+	if (ret)
+		drm_err(&i915->drm,
+			"Unsetting gucrc mode failed %d\n",
+			ret);
+
+	return ret;
+}
+
 /*
  * intel_guc_slpc_enable() - Start SLPC
  * @slpc: pointer to intel_guc_slpc.
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
index 82a98f78f96c..ccf483730d9d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.h
@@ -42,5 +42,7 @@  int intel_guc_slpc_set_media_ratio_mode(struct intel_guc_slpc *slpc, u32 val);
 void intel_guc_pm_intrmsk_enable(struct intel_gt *gt);
 void intel_guc_slpc_boost(struct intel_guc_slpc *slpc);
 void intel_guc_slpc_dec_waiters(struct intel_guc_slpc *slpc);
+int intel_guc_slpc_unset_gucrc_mode(struct intel_guc_slpc *slpc);
+int intel_guc_slpc_override_gucrc_mode(struct intel_guc_slpc *slpc, u32 mode);
 
 #endif
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 132c2ce8b33b..ce1b6ad4d107 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -208,6 +208,7 @@ 
 #include "gt/intel_lrc.h"
 #include "gt/intel_lrc_reg.h"
 #include "gt/intel_ring.h"
+#include "gt/uc/intel_guc_slpc.h"
 
 #include "i915_drv.h"
 #include "i915_file_private.h"
@@ -1651,6 +1652,16 @@  static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
 
 	free_oa_buffer(stream);
 
+	/*
+	 * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
+	 */
+	if (intel_guc_slpc_is_used(&gt->uc.guc) &&
+	    intel_uc_uses_guc_rc(&gt->uc) &&
+	    (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
+	     IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0)))
+		drm_WARN_ON(&gt->i915->drm,
+			    intel_guc_slpc_unset_gucrc_mode(&gt->uc.guc.slpc));
+
 	intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
 	intel_engine_pm_put(stream->engine);
 
@@ -3339,6 +3350,24 @@  static int i915_oa_stream_init(struct i915_perf_stream *stream,
 	intel_engine_pm_get(stream->engine);
 	intel_uncore_forcewake_get(stream->uncore, FORCEWAKE_ALL);
 
+	/*
+	 * Wa_16011777198:dg2: GuC resets render as part of the Wa. This causes
+	 * OA to lose the configuration state. Prevent this by overriding GUCRC
+	 * mode.
+	 */
+	if (intel_guc_slpc_is_used(&gt->uc.guc) &&
+	    intel_uc_uses_guc_rc(&gt->uc) &&
+	    (IS_DG2_GRAPHICS_STEP(gt->i915, G10, STEP_A0, STEP_C0) ||
+	     IS_DG2_GRAPHICS_STEP(gt->i915, G11, STEP_A0, STEP_B0))) {
+		ret = intel_guc_slpc_override_gucrc_mode(&gt->uc.guc.slpc,
+							 SLPC_GUCRC_MODE_GUCRC_NO_RC6);
+		if (ret) {
+			drm_dbg(&stream->perf->i915->drm,
+				"Unable to override gucrc mode\n");
+			goto err_config;
+		}
+	}
+
 	ret = alloc_oa_buffer(stream);
 	if (ret)
 		goto err_oa_buf_alloc;