Re: [PATCH] drm/i915/rc6: Disable RPG during workload execution

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 22-10-2024 22:39, Rodrigo Vivi wrote:
> On Tue, Oct 22, 2024 at 03:28:43PM +0200, Andi Shyti wrote:
>> Hi Badal,
>>
>> On Tue, Oct 22, 2024 at 06:52:26PM +0530, Badal Nilawar wrote:
>>> Encountering forcewake errors related to render power gating;
>>
>> Can you please expand your explanation here?
> 
> yeap. More explanation please. All platforms? really?

We are seeing Render forcewake timeouts on ADLP, ADLM, ADLN, TWL, DG1, 
rpl. Issue disappears after disabling RPG. Instead of fully disabling 
RPG I am disabling it during active submissions i.e. during unpark.
For MTL and ARL RPG is already disabled permanently.

Impact of doing this change should be performance improvement so kept 
for all platform otherwise I will add platform check.

This is the issue https://gitlab.freedesktop.org/drm/intel/issues/9413. 
Will add it in commit message.

Regards,
Badal

> 
>>
>>> therefore, disable it during workload execution.
>>
>> ... and here.
>>
>>> Cc: Chris Wilson <chris.p.wilson@linux.intel.com>
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/gt/intel_rc6.c       | 18 +++++++++++++++++-
>>>   drivers/gpu/drm/i915/gt/intel_rc6_types.h |  1 +
>>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> index c864d101faf9..459394ab5258 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
>>> @@ -140,6 +140,7 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
>>>   					      VDN_MFX_POWERGATE_ENABLE(i));
>>>   	}
>>>   
>>> +	rc6->pg_enable = pg_enable;
>>
>> this looks borderline racy, it's fine only because this function
>> is called during resume which normally runs in atomic context.
>>
>>>   	intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, pg_enable);
>>>   }
>>>   
>>> @@ -572,8 +573,11 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
>>>   	intel_guc_rc_disable(gt_to_guc(gt));
>>>   
>>>   	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
>>> -	if (GRAPHICS_VER(i915) >= 9)
>>> +	if (GRAPHICS_VER(i915) >= 9) {
>>> +		rc6->pg_enable = 0;
>>>   		intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, 0);
>>> +	}
>>> +
>>>   	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, 0);
>>>   	intel_uncore_write_fw(uncore, GEN6_RC_STATE, 0);
>>>   	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
>>> @@ -687,6 +691,15 @@ void intel_rc6_unpark(struct intel_rc6 *rc6)
>>>   
>>>   	/* Restore HW timers for automatic RC6 entry while busy */
>>>   	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, rc6->ctl_enable);
>>> +
>>> +	/*
>>> +	 * Seeing render forcewake timeouts during active submissions so disable render PG
>>> +	 * while workloads are under execution.
>>
>> Can you please improve this sentence? If I never new about the
>> issue I would be a bit confused.
>>
>>> +	 * FIXME Remove this change once real cause of render force wake timeout is fixed
>>> +	 */
>>> +	if (rc6->pg_enable == GEN9_RENDER_PG_ENABLE)
>>
>> is this supposed to be "pg_enable == GEN9_RENDER_PG_ENABLE" or
>> "pg_enable & GEN9_RENDER_PG_ENABLE" ?
>>
>> Andi