Re: [PATCH v2 14/15] drm/i915/guc: Support OA when Wa_16011777198 is enabled

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

On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote:
>
> From: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>

Hi Umesh/Vinay,

> @@ -3254,6 +3265,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 this condition above correct? E.g. what happens when:

a. GuCRC is used but SLPC is not used?
b. GuCRC is not used. Don't we need to disable RC6 in host based RC6
   control?

Do we need to worry about these cases?

Or if we always expect both GuCRC and SLPC to be used on DG2 then I think
let's get rid of these from the if condition and add a drm_err() if we see
these not being used and OA is being enabled on DG2?

Thanks.
--
Ashutosh

> +	    (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;
> --
> 2.25.1
>