diff mbox series

[1/9] drm/i915/perf: Drop wakeref on GuC RC error

Message ID 20230215005419.2100887-2-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series Add OAM support for MTL | expand

Commit Message

Umesh Nerlige Ramappa Feb. 15, 2023, 12:54 a.m. UTC 
From: Chris Wilson <chris.p.wilson@linux.intel.com>

If we fail to adjust the GuC run-control on opening the perf stream,
make sure we unwind the wakeref just taken.

Fixes: 01e742746785 ("drm/i915/guc: Support OA when Wa_16011777198 is enabled")
Signed-off-by: Chris Wilson <chris.p.wilson@linux.intel.com>
Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c       | 18 +++++++++++-------
 drivers/gpu/drm/i915/i915_perf_types.h |  6 ++++++
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Dixit, Ashutosh Feb. 16, 2023, 3:56 a.m. UTC | #1 
On Tue, 14 Feb 2023 16:54:11 -0800, Umesh Nerlige Ramappa wrote:
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 824a34ec0b83..393a0da8b7c8 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1592,9 +1592,7 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>	/*
>	 * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
>	 */
> -	if (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)))
> +	if (stream->override_gucrc)
>		drm_WARN_ON(&gt->i915->drm,
>			    intel_guc_slpc_unset_gucrc_mode(&gt->uc.guc.slpc));
>
> @@ -3305,13 +3303,15 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>		if (ret) {
>			drm_dbg(&stream->perf->i915->drm,
>				"Unable to override gucrc mode\n");
> -			goto err_config;
> +			goto err_fw;
>		}
> +
> +		stream->override_gucrc = true;
>	}
>
>	ret = alloc_oa_buffer(stream);
>	if (ret)
> -		goto err_oa_buf_alloc;
> +		goto err_gucrc;
>
>	stream->ops = &i915_oa_stream_ops;
>
> @@ -3344,12 +3344,16 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
>
>	free_oa_buffer(stream);
>
> -err_oa_buf_alloc:
> -	free_oa_configs(stream);
> +err_gucrc:
> +	if (stream->override_gucrc)
> +		intel_guc_slpc_unset_gucrc_mode(&gt->uc.guc.slpc);
>
> +err_fw:
>	intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
>	intel_engine_pm_put(stream->engine);
>
> +	free_oa_configs(stream);
> +
>  err_config:
>	free_noa_wait(stream);

[nice-to-have] The previous naming scheme for labels in the function is the
place from which the goto is issued so err_fw should be named err_gucrc and
err_gucrc should be called the preevious name err_oa_buf_alloc but
otherwise the code seems correct so this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 824a34ec0b83..393a0da8b7c8 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1592,9 +1592,7 @@  static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
 	/*
 	 * Wa_16011777198:dg2: Unset the override of GUCRC mode to enable rc6.
 	 */
-	if (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)))
+	if (stream->override_gucrc)
 		drm_WARN_ON(&gt->i915->drm,
 			    intel_guc_slpc_unset_gucrc_mode(&gt->uc.guc.slpc));
 
@@ -3305,13 +3303,15 @@  static int i915_oa_stream_init(struct i915_perf_stream *stream,
 		if (ret) {
 			drm_dbg(&stream->perf->i915->drm,
 				"Unable to override gucrc mode\n");
-			goto err_config;
+			goto err_fw;
 		}
+
+		stream->override_gucrc = true;
 	}
 
 	ret = alloc_oa_buffer(stream);
 	if (ret)
-		goto err_oa_buf_alloc;
+		goto err_gucrc;
 
 	stream->ops = &i915_oa_stream_ops;
 
@@ -3344,12 +3344,16 @@  static int i915_oa_stream_init(struct i915_perf_stream *stream,
 
 	free_oa_buffer(stream);
 
-err_oa_buf_alloc:
-	free_oa_configs(stream);
+err_gucrc:
+	if (stream->override_gucrc)
+		intel_guc_slpc_unset_gucrc_mode(&gt->uc.guc.slpc);
 
+err_fw:
 	intel_uncore_forcewake_put(stream->uncore, FORCEWAKE_ALL);
 	intel_engine_pm_put(stream->engine);
 
+	free_oa_configs(stream);
+
 err_config:
 	free_noa_wait(stream);
 
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index ca150b7af3f2..e36f046fe2b6 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -316,6 +316,12 @@  struct i915_perf_stream {
 	 * buffer should be checked for available data.
 	 */
 	u64 poll_oa_period;
+
+	/**
+	 * @override_gucrc: GuC RC has been overridden for the perf stream,
+	 * and we need to restore the default configuration on release.
+	 */
+	bool override_gucrc:1;
 };
 
 /**