Re: [Intel-gfx] [PATCH 1/9] drm/i915/perf: Drop wakeref on GuC RC error

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

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>