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

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Sep 26, 2022 at 08:56:01AM -0700, Dixit, Ashutosh wrote:
>On Fri, 23 Sep 2022 13:11:53 -0700, Umesh Nerlige Ramappa wrote:
>>
>> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
>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?

When using host based rc6, existing OA code is using forcewake and a 
reference to engine_pm to prevent rc6. Other questions, directing to 
@Vinay.

Thanks,
Umesh

>
>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
>>