On Mon, Sep 26, 2022 at 04:28:44PM -0700, Dixit, Ashutosh wrote:
On Mon, 26 Sep 2022 14:17:21 -0700, Belgaumkar, Vinay wrote:
On 9/26/2022 11:19 AM, Umesh Nerlige Ramappa wrote:
> 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@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(>->uc.guc) &&
>>> + intel_uc_uses_guc_rc(>->uc) &&
>>
>> Is this condition above correct? E.g. what happens when:
>>
>> a. GuCRC is used but SLPC is not used?
Currently, we have no way to separate out GuC RC and SLPC. Both are on when
guc submission is enabled/supported. So, the above check is kind of
redundant anyways.
That is why I was suggesting changing the if check to an assert or
drm_err. So looks like it will work with or without GuC RC, but if we are
using GuC RC we should be using SLPC. So:
if (GuC_RC && !SLPC)
drm_err();
I am little confused. What's the ask here? Should I just use one of
these conditions? i.e.
if (intel_guc_slpc_is_used(>->uc.guc))
...
Thanks,
Umesh
Thanks,
Vinay.
>> 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(>->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
>>>
