Re: [PATCH] drm/xe: Check skip_guc_pc before setting SLPC flag

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

On 1/3/2024 10:15 PM, Nilawar, Badal wrote:
>
>
> On 04-01-2024 11:36, Belgaumkar, Vinay wrote:
>>
>> On 1/3/2024 8:56 PM, Nilawar, Badal wrote:
>>> Hi Vinay,
>>>
>>> On 23-12-2023 02:31, Vinay Belgaumkar wrote:
>>>> Don't set SLPC GuC feature ctl flag if skip_guc_pc is true.
>>>>
>>>> Fixes: 975e4a3795d4 ("drm/xe: Manually setup C6 when skip_guc_pc is 
>>>> set")
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/xe/xe_guc.c | 7 ++++++-
>>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>>>> index 811e8b201270..0e112b144ec8 100644
>>>> --- a/drivers/gpu/drm/xe/xe_guc.c
>>>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>>>> @@ -62,7 +62,12 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc)
>>>>     static u32 guc_ctl_feature_flags(struct xe_guc *guc)
>>>>   {
>>>> -    return GUC_CTL_ENABLE_SLPC;
>>>> +    u32 flags = 0;
>>>> +
>>>> +    if (!(guc_to_xe(guc))->info.skip_guc_pc)
>>> Frequency attributes freq_min/freq_max may not work when skip_guc_pc 
>>> is set. freq_curr also may not work. I think these freq attributes 
>>> shouldn't be exposed for skip_guc_pc case.
>>
>> We already skip pc_init() (whihc creates the sysfs attribs) when this 
>> flag is set, that was in a previously merged patch.
> With recent changes in bef52b5c7a19 ("drm/xe: Create a xe_gt_freq 
> component for raw management and sysfs") attributes are created by 
> xe_gt_freq_init() which is called in xe_gt_init().

Yup, good catch. These patches were merged after the skip_guc_pc flag 
was added. I have sent another version of this patch with the fix for it.

Thanks,

Vinay.

>
> Regards,
> Badal
>>
>> Thanks,
>>
>> Vinay.
>>
>>>
>>> Regards,
>>> Badal
>>>> +        flags |= GUC_CTL_ENABLE_SLPC;
>>>> +
>>>> +    return flags;
>>>>   }
>>>>     static u32 guc_ctl_log_params_flags(struct xe_guc *guc)