Re: [PATCH] drm/xe/debugfs: Expose debugfs entry to check guc pc support

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

On 19-01-2024 15:27, Riana Tauro wrote:
> Hi Badal
> 
> On 1/19/2024 3:05 PM, Gupta, Anshuman wrote:
>>
>>
>>> -----Original Message-----
>>> From: Nilawar, Badal <badal.nilawar@intel.com>
>>> Sent: Friday, January 19, 2024 3:01 PM
>>> To: Gupta, Anshuman <anshuman.gupta@intel.com>; intel-
>>> xe@lists.freedesktop.org
>>> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Roper, Matthew D
>>> <matthew.d.roper@intel.com>; Dixit, Ashutosh <ashutosh.dixit@intel.com>;
>>> Belgaumkar, Vinay <vinay.belgaumkar@intel.com>
>>> Subject: Re: [PATCH] drm/xe/debugfs: Expose debugfs entry to check 
>>> guc pc
>>> support
>>>
>>>
>>>
>>> On 19-01-2024 14:58, Gupta, Anshuman wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Nilawar, Badal <badal.nilawar@intel.com>
>>>>> Sent: Friday, January 19, 2024 1:49 PM
>>>>> To: intel-xe@lists.freedesktop.org
>>>>> Cc: Gupta, Anshuman <anshuman.gupta@intel.com>; Vivi, Rodrigo
>>>>> <rodrigo.vivi@intel.com>; Roper, Matthew D
>>>>> <matthew.d.roper@intel.com>; Dixit, Ashutosh
>>>>> <ashutosh.dixit@intel.com>; Belgaumkar, Vinay
>>>>> <vinay.belgaumkar@intel.com>
>>>>> Subject: [PATCH] drm/xe/debugfs: Expose debugfs entry to check guc pc
>>>>> support
>>>>>
>>>>> Expose debugfs entry to check if platform support GuC PC (SLPC) 
>>>>> feature.
>>>>>
>>>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/xe/xe_debugfs.c | 8 ++++++++
>>>>>    1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c
>>>>> b/drivers/gpu/drm/xe/xe_debugfs.c index c56fd7d59f05..2f474bf0e7c7
>>>>> 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>>>>> @@ -30,6 +30,13 @@ static struct xe_device *node_to_xe(struct
>>>>> drm_info_node *node)
>>>>>        return to_xe_device(node->minor->dev);  }
>>>>>
>>>>> +static int is_gucpc_supported(struct seq_file *m, void *data) {
>>>>> +    struct xe_device *xe = node_to_xe(m->private);
>>>>> +
>>>>> +    return (int)!xe->info.skip_guc_pc;
>>>>> +}
>>>>> +
>>>>>    static int info(struct seq_file *m, void *data)  {
>>>>>        struct xe_device *xe = node_to_xe(m->private); @@ -67,6 
>>>>> +74,7 @@
>>>>> static int info(struct seq_file *m, void *data)
>>>>>
>>>>>    static const struct drm_info_list debugfs_list[] = {
>>>>>        {"info", info, 0},
>>>>> +    {"is_gucpc_supported", is_gucpc_supported, 0},
>>>> Above debugfs is per device.
>>>> We need this debugfs for each gt.
>>> The field skip_guc_pc is device specific so kept debugfs device 
>>> specific.
>> That make sense but then it is better to have a print in existing info 
>> debugfs
>> as this is filed in xe->info ?
>>  From igt we can grep the skip_guc_pc either "Yes" or "No".
>> Thanks,
>> Anshuman.
>>
> But shouldn't this be under gt0/uc/? if its named as guc_pc_supported
> or instead should we have something similar to i915 gt0/uc/guc_slpc_info?
Intention of this patch is to check if platform support slpc or not. As 
suggested above adding skip_guc_pc in info is sufficient no need to add 
new debugfs entry.

In addition to above, similar to i915 gt0/uc/guc_slpc_info can be 
implemented for xe kmd as well.  guc_slpc_info gives the state of SLPC 
(RUNNING, ERROR, so on). AFAIK recent times we are not seeing SLPC 
issues so not sure how helpful adding this would be. Lets see what 
others think about this. If needed this can be taken care in separate 
patch.

Regards,
Badal
> 
> Thanks
> Riana
>>>
>>> Regards,
>>> Badal
>>>> Thanks,
>>>> Anshuman.
>>>>>    };
>>>>>
>>>>>    static int forcewake_open(struct inode *inode, struct file *file)
>>>>> -- 
>>>>> 2.25.1
>>>>