[PATCH 3/3] drm/xe/pf: Expose SR-IOV policy settings over debugfs

[Michal Wajdeczko]

On 22.04.2024 19:02, Piotr Piórkowski wrote:
> Michal Wajdeczko <michal.wajdeczko at intel.com> wrote on czw [2024-kwi-18 22:34:42 +0200]:
>> We already have functions to configure SR-IOV policies.
>> Allow to tweak those policy settings over debugfs.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c | 50 +++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>> index 8909bb950a8b..3a83af7aa039 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>> @@ -17,6 +17,7 @@
>>  #include "xe_gt_sriov_pf_control.h"
>>  #include "xe_gt_sriov_pf_debugfs.h"
>>  #include "xe_gt_sriov_pf_helpers.h"
>> +#include "xe_gt_sriov_pf_policy.h"
>>  #include "xe_pm.h"
>>  
>>  /*
>> @@ -76,6 +77,54 @@ static const struct drm_info_list pf_info[] = {
>>  	},
>>  };
>>  
>> +/*
>> + *      /sys/kernel/debug/dri/0/
>> + *      ├── gt0
>> + *      │   ├── pf
>> + *      │   │   ├── reset_engine
>> + *      │   │   ├── sample_period
>> + *      │   │   ├── sched_if_idle
>> + */
>> +
>> +#define DEFINE_SRIOV_GT_POLICY_DEBUGFS_ATTRIBUTE(POLICY, TYPE, FORMAT)		\
>> +			
> The same as I wrote in e earlier patch, only here also TYPE seems unnecessary.
> What's the point of creating a macro with parameters you don't use?
> \

as explained earlier, FORMAT parameter allows flexibility as provided by
DEFINE_DEBUGFS_ATTRIBUTE

for TYPE, I clearly missed to add code to reject out of range input
values, will try to fix in v2

>> +static int POLICY##_set(void *data, u64 val)					\
>> +{										\
>> +	struct xe_gt *gt = extract_gt(data);					\
>> +	struct xe_device *xe = gt_to_xe(gt);					\
>> +	int err;								\
>> +										\
>> +	xe_pm_runtime_get(xe);							\
>> +	err = xe_gt_sriov_pf_policy_set_##POLICY(gt, val);			\
>> +	xe_pm_runtime_put(xe);							\
>> +										\
>> +	return err;								\
>> +}										\
>> +										\
>> +static int POLICY##_get(void *data, u64 *val)					\
>> +{										\
>> +	struct xe_gt *gt = extract_gt(data);					\
>> +										\
>> +	*val = xe_gt_sriov_pf_policy_get_##POLICY(gt);				\
>> +	return 0;								\
>> +}										\
>> +										\
>> +DEFINE_DEBUGFS_ATTRIBUTE(POLICY##_fops, POLICY##_get, POLICY##_set, FORMAT)
>> +
>> +DEFINE_SRIOV_GT_POLICY_DEBUGFS_ATTRIBUTE(reset_engine, bool, "%llu\n");
>> +DEFINE_SRIOV_GT_POLICY_DEBUGFS_ATTRIBUTE(sched_if_idle, bool, "%llu\n");
>> +DEFINE_SRIOV_GT_POLICY_DEBUGFS_ATTRIBUTE(sample_period, u32, "%llu\n");
>> +
>> +static void pf_add_policy_attrs(struct xe_gt *gt, struct dentry *parent)
>> +{
>> +	xe_gt_assert(gt, gt == extract_gt(parent));
>> +	xe_gt_assert(gt, PFID == extract_vfid(parent));
>> +
>> +	debugfs_create_file_unsafe("reset_engine", 0644, parent, parent, &reset_engine_fops);
>> +	debugfs_create_file_unsafe("sched_if_idle", 0644, parent, parent, &sched_if_idle_fops);
>> +	debugfs_create_file_unsafe("sample_period_ms", 0644, parent, parent, &sample_period_fops);
>> +}
>> +
>>  /*
>>   *      /sys/kernel/debug/dri/0/
>>   *      ├── gt0
>> @@ -261,6 +310,7 @@ void xe_gt_sriov_pf_debugfs_register(struct xe_gt *gt, struct dentry *root)
>>  	pfdentry->d_inode->i_private = gt;
>>  
>>  	drm_debugfs_create_files(pf_info, ARRAY_SIZE(pf_info), pfdentry, minor);
>> +	pf_add_policy_attrs(gt, pfdentry);
>>  	pf_add_config_attrs(gt, pfdentry, PFID);
>>  
>>  	for (n = 1; n <= totalvfs; n++) {
>> -- 
>> 2.43.0
> 
> One comment above. 
> Overall the functionality is ok:
> Reviewed-by: Piotr Piórkowski <piotr.piorkowski at intel.com>

Thanks,

> 
>>
>