[PATCH] drm/xe/guc: Request max GT freq during resume

[Belgaumkar, Vinay]

On 6/3/2024 12:17 PM, Michal Wajdeczko wrote:
>
> On 03.06.2024 21:06, Belgaumkar, Vinay wrote:
>> On 6/3/2024 8:28 AM, Michal Wajdeczko wrote:
>>> On 31.05.2024 23:42, Vinay Belgaumkar wrote:
>>>> We already request max freq in the load path, moving it
>>>> to __xe_guc_upload will ensure this speeds up GuC load in
>>>> the resume path as well.
>>>>
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_guc.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>>>> index f7886c00af01..63e1b685bd4f 100644
>>>> --- a/drivers/gpu/drm/xe/xe_guc.c
>>>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>>>> @@ -694,6 +694,9 @@ static int __xe_guc_upload(struct xe_guc *guc)
>>>>    {
>>>>        int ret;
>>>>    +    /* Raise GT freq to speed up HuC/GuC load */
>>>> +    xe_guc_pc_init_early(&guc->pc);
>>> maybe it's just me, but usually we were using _init_early() name suffix
>>> for functions with some early, one-time, likely software-only
>>> initialization, while here this xe_guc_pc_init_early() seems to be doing
>>> something else and now it could even be called many times
>> It is initializing the internal variables to the fused frequency values
>> as well. It is still being called only once so far. This patch just
>> changes where it is called from.
> this is now called from xe_uc_init_hw(), so not once per driver load:
True, more like once per guc load. It is called 
intel_rps_raise_unslice() in i915, so might be better to call it 
something similar.
>
> /*
>   * Should be called during driver load, after every GT reset, and after
> every
>   * suspend to reload / auth the firmwares.
>   */
> int xe_uc_init_hw(struct xe_uc *uc)
> {
> ...	ret = xe_guc_upload(&uc->guc);
>
>
> int xe_guc_upload(struct xe_guc *guc)
> {
> ...	return __xe_guc_upload(guc);
>
>> Also, as of now, I don't see any other location where we could call this
>> from. If that happens, we can rename the function.
>>
>> Thanks,
>>
>> Vinay.
>>
>>> maybe it should be split/renamed to xe_guc_pc_boost() or something?
>>>
>>>> +
>>>>        guc_write_params(guc);
>>>>        guc_prepare_xfer(guc);
>>>>    @@ -779,9 +782,6 @@ int xe_guc_min_load_for_hwconfig(struct xe_guc
>>>> *guc)
>>>>          xe_guc_ads_populate_minimal(&guc->ads);
>>>>    -    /* Raise GT freq to speed up HuC/GuC load */
>>>> -    xe_guc_pc_init_early(&guc->pc);
>>>> -
>>>>        ret = __xe_guc_upload(guc);
>>>>        if (ret)
>>>>            return ret;