Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Restore efficient freq earlier

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

On 7/21/2023 2:23 PM, Rodrigo Vivi wrote:
> On Fri, Jul 21, 2023 at 01:44:34PM -0700, Belgaumkar, Vinay wrote:
>> On 7/21/2023 1:41 PM, Rodrigo Vivi wrote:
>>> On Fri, Jul 21, 2023 at 11:03:49AM -0700, Vinay Belgaumkar wrote:
>>>> This should be done before the soft min/max frequencies are restored.
>>>> When we disable the "Ignore efficient frequency" flag, GuC does not
>>>> actually bring the requested freq down to RPn.
>>>>
>>>> Specifically, this scenario-
>>>>
>>>> - ignore efficient freq set to true
>>>> - reduce min to RPn (from efficient)
>>>> - suspend
>>>> - resume (includes GuC load, restore soft min/max, restore efficient freq)
>>>> - validate min freq has been resored to RPn
>>>>
>>>> This will fail if we didn't first restore(disable, in this case) efficient
>>>> freq flag before setting the soft min frequency.
>>> that's strange. so guc is returning the rpe when we request the min freq
>>> during the soft config?
>>>
>>> we could alternatively change the soft config to actually get the min
>>> and not be tricked by this.
>>>
>>> But also the patch below doesn't hurt.
>>>
>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> (Although I'm still curious and want to understand exactly why
>>> the soft min gets messed up when we don't tell guc to ignore the
>>> efficient freq beforehand. Please help me to understand.)
>> The soft min does not get messed up, but GuC keeps requesting RPe even after
>> disabling efficient freq. (unless we manually set min freq to RPn AFTER
>> disabling efficient).
> so it looks to me that the right solution would be to ensure that everytime
> that we disable the efficient freq we make sure to also set the mim freq to RPn,
> no?!

Hmm, may not be applicable every time. What if someone disables 
efficient frequency while running a workload or with frequency fixed to 
800, for example?

Thanks,

Vinay.

>
>> Thanks,
>>
>> Vinay.
>>
>>>
>>>> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8736
>>>> Fixes: 55f9720dbf23 ("drm/i915/guc/slpc: Provide sysfs for efficient freq")
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>>> index ee9f83af7cf6..f16dff7c3185 100644
>>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>>> @@ -743,6 +743,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>>>>    	intel_guc_pm_intrmsk_enable(slpc_to_gt(slpc));
>>>> +	/* Set cached value of ignore efficient freq */
>>>> +	intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
>>>> +
>>>>    	slpc_get_rp_values(slpc);
>>>>    	/* Handle the case where min=max=RPmax */
>>>> @@ -765,9 +768,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>>>>    	/* Set cached media freq ratio mode */
>>>>    	intel_guc_slpc_set_media_ratio_mode(slpc, slpc->media_ratio_mode);
>>>> -	/* Set cached value of ignore efficient freq */
>>>> -	intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
>>>> -
>>>>    	return 0;
>>>>    }
>>>> -- 
>>>> 2.38.1
>>>>