[Intel-xe] [PATCH 1/1] drm/xe: Add a new sub directory for gt frequencies

Sundaresan, Sujaritha sujaritha.sundaresan at intel.com
Mon Nov 27 10:22:30 UTC 2023


On 11/27/2023 3:45 PM, Jani Nikula wrote:
> On Mon, 27 Nov 2023, Sujaritha Sundaresan <sujaritha.sundaresan at intel.com> wrote:
>> Add a new sub directory "freq" for gt frequencies and rename
>> exisiting frequency sysfs attributes to align with devfreq
>> requirements.
> The cover letter says this does much more, so the commit message here is
> not accurate.
>
> OTOH when a commit message says "Also", it's usually a hint it should be
> a separate patch. Often functional and non-functional changes should be
> separated, so it's easier to review each part.

Hi Jani,

Sure I can probably split the name change and the directory addition.

Thanks,

Suja

>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_types.h     |   4 +
>>   drivers/gpu/drm/xe/xe_guc_pc.c       | 125 ++++++++++++++++-----------
>>   drivers/gpu/drm/xe/xe_guc_pc_types.h |   3 +
>>   3 files changed, 82 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>> index d3f2793684e2..ea1e05a46288 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>> @@ -8,6 +8,7 @@
>>   
>>   #include "xe_force_wake_types.h"
>>   #include "xe_gt_idle_sysfs_types.h"
>> +#include "xe_guc_pc_types.h"
>>   #include "xe_hw_engine_types.h"
>>   #include "xe_hw_fence_types.h"
>>   #include "xe_reg_sr_types.h"
>> @@ -270,6 +271,9 @@ struct xe_gt {
>>   	/** @gtidle: idle properties of GT */
>>   	struct xe_gt_idle gtidle;
>>   
>> +	/** @frq: frequencies of GT */
>> +	struct xe_gt_frq frq;
> This seems unnecessarily terse to me.
>
> BR,
> Jani.
>
>> +
>>   	/** @exec_queue_ops: submission backend exec queue operations */
>>   	const struct xe_exec_queue_ops *exec_queue_ops;
>>   
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
>> index e9dd6c3d750b..d0b625d8b1dc 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
>> @@ -57,15 +57,15 @@
>>    * Xe's GuC PC provides a sysfs API for frequency management:
>>    *
>>    * device/gt#/freq_* *read-only* files:
>> - * - freq_act: The actual resolved frequency decided by PCODE.
>> - * - freq_cur: The current one requested by GuC PC to the Hardware.
>> - * - freq_rpn: The Render Performance (RP) N level, which is the minimal one.
>> - * - freq_rpe: The Render Performance (RP) E level, which is the efficient one.
>> - * - freq_rp0: The Render Performance (RP) 0 level, which is the maximum one.
>> + * - act_freq: The actual resolved frequency decided by PCODE.
>> + * - cur_freq: The current one requested by GuC PC to the Hardware.
>> + * - rpn_freq: The Render Performance (RP) N level, which is the minimal one.
>> + * - rpe_freq: The Render Performance (RP) E level, which is the efficient one.
>> + * - rp0_freq: The Render Performance (RP) 0 level, which is the maximum one.
>>    *
>>    * device/gt#/freq_* *read-write* files:
>> - * - freq_min: GuC PC min request.
>> - * - freq_max: GuC PC max request.
>> + * - min_freq: GuC PC min request.
>> + * - max_freq: GuC PC max request.
>>    *             If max <= min, then freq_min becomes a fixed frequency request.
>>    *
>>    * Render-C States:
>> @@ -97,18 +97,29 @@ pc_to_gt(struct xe_guc_pc *pc)
>>   	return container_of(pc, struct xe_gt, uc.guc.pc);
>>   }
>>   
>> -static struct xe_guc_pc *
>> -dev_to_pc(struct device *dev)
>> -{
>> -	return &kobj_to_gt(&dev->kobj)->uc.guc.pc;
>> -}
>> -
>>   static struct iosys_map *
>>   pc_to_maps(struct xe_guc_pc *pc)
>>   {
>>   	return &pc->bo->vmap;
>>   }
>>   
>> +static struct xe_gt_frq *dev_to_frq(struct device *dev)
>> +{
>> +	struct kobject *kobj = &dev->kobj;
>> +
>> +	return &kobj_to_gt(kobj->parent)->frq;
>> +}
>> +
>> +static struct xe_gt *frq_to_gt(struct xe_gt_frq *frq)
>> +{
>> +	return container_of(frq, struct xe_gt, frq);
>> +}
>> +
>> +static struct xe_guc_pc *frq_to_pc(struct xe_gt_frq *frq)
>> +{
>> +	return &frq_to_gt(frq)->uc.guc.pc;
>> +}
>> +
>>   #define slpc_shared_data_read(pc_, field_) \
>>   	xe_map_rd_field(pc_to_xe(pc_), pc_to_maps(pc_), 0, \
>>   			struct slpc_shared_data, field_)
>> @@ -385,11 +396,11 @@ static void pc_update_rp_values(struct xe_guc_pc *pc)
>>   	pc->rpn_freq = min(pc->rpn_freq, pc->rpe_freq);
>>   }
>>   
>> -static ssize_t freq_act_show(struct device *dev,
>> +static ssize_t act_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct kobject *kobj = &dev->kobj;
>> -	struct xe_gt *gt = kobj_to_gt(kobj);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_gt *gt = frq_to_gt(frq);
>>   	struct xe_device *xe = gt_to_xe(gt);
>>   	u32 freq;
>>   	ssize_t ret;
>> @@ -410,13 +421,13 @@ static ssize_t freq_act_show(struct device *dev,
>>   	xe_device_mem_access_put(gt_to_xe(gt));
>>   	return ret;
>>   }
>> -static DEVICE_ATTR_RO(freq_act);
>> +static DEVICE_ATTR_RO(act_freq);
>>   
>> -static ssize_t freq_cur_show(struct device *dev,
>> +static ssize_t cur_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct kobject *kobj = &dev->kobj;
>> -	struct xe_gt *gt = kobj_to_gt(kobj);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_gt *gt = frq_to_gt(frq);
>>   	u32 freq;
>>   	ssize_t ret;
>>   
>> @@ -439,21 +450,23 @@ static ssize_t freq_cur_show(struct device *dev,
>>   	xe_device_mem_access_put(gt_to_xe(gt));
>>   	return ret;
>>   }
>> -static DEVICE_ATTR_RO(freq_cur);
>> +static DEVICE_ATTR_RO(cur_freq);
>>   
>> -static ssize_t freq_rp0_show(struct device *dev,
>> +static ssize_t rp0_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   
>>   	return sysfs_emit(buf, "%d\n", pc->rp0_freq);
>>   }
>> -static DEVICE_ATTR_RO(freq_rp0);
>> +static DEVICE_ATTR_RO(rp0_freq);
>>   
>> -static ssize_t freq_rpe_show(struct device *dev,
>> +static ssize_t rpe_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   	struct xe_gt *gt = pc_to_gt(pc);
>>   	struct xe_device *xe = gt_to_xe(gt);
>>   
>> @@ -462,22 +475,24 @@ static ssize_t freq_rpe_show(struct device *dev,
>>   	xe_device_mem_access_put(xe);
>>   	return sysfs_emit(buf, "%d\n", pc->rpe_freq);
>>   }
>> -static DEVICE_ATTR_RO(freq_rpe);
>> +static DEVICE_ATTR_RO(rpe_freq);
>>   
>> -static ssize_t freq_rpn_show(struct device *dev,
>> +static ssize_t rpn_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   
>>   	return sysfs_emit(buf, "%d\n", pc->rpn_freq);
>>   }
>> -static DEVICE_ATTR_RO(freq_rpn);
>> +static DEVICE_ATTR_RO(rpn_freq);
>>   
>> -static ssize_t freq_min_show(struct device *dev,
>> +static ssize_t min_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> -	struct xe_gt *gt = pc_to_gt(pc);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>> +	struct xe_gt *gt = frq_to_gt(frq);
>>   	ssize_t ret;
>>   
>>   	xe_device_mem_access_get(pc_to_xe(pc));
>> @@ -510,10 +525,11 @@ static ssize_t freq_min_show(struct device *dev,
>>   	return ret;
>>   }
>>   
>> -static ssize_t freq_min_store(struct device *dev, struct device_attribute *attr,
>> +static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
>>   			      const char *buff, size_t count)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   	u32 freq;
>>   	ssize_t ret;
>>   
>> @@ -540,12 +556,13 @@ static ssize_t freq_min_store(struct device *dev, struct device_attribute *attr,
>>   	xe_device_mem_access_put(pc_to_xe(pc));
>>   	return ret ?: count;
>>   }
>> -static DEVICE_ATTR_RW(freq_min);
>> +static DEVICE_ATTR_RW(min_freq);
>>   
>> -static ssize_t freq_max_show(struct device *dev,
>> +static ssize_t max_freq_show(struct device *dev,
>>   			     struct device_attribute *attr, char *buf)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   	ssize_t ret;
>>   
>>   	xe_device_mem_access_get(pc_to_xe(pc));
>> @@ -568,10 +585,11 @@ static ssize_t freq_max_show(struct device *dev,
>>   	return ret;
>>   }
>>   
>> -static ssize_t freq_max_store(struct device *dev, struct device_attribute *attr,
>> +static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>>   			      const char *buff, size_t count)
>>   {
>> -	struct xe_guc_pc *pc = dev_to_pc(dev);
>> +	struct xe_gt_frq *frq = dev_to_frq(dev);
>> +	struct xe_guc_pc *pc = frq_to_pc(frq);
>>   	u32 freq;
>>   	ssize_t ret;
>>   
>> @@ -598,7 +616,7 @@ static ssize_t freq_max_store(struct device *dev, struct device_attribute *attr,
>>   	xe_device_mem_access_put(pc_to_xe(pc));
>>   	return ret ?: count;
>>   }
>> -static DEVICE_ATTR_RW(freq_max);
>> +static DEVICE_ATTR_RW(max_freq);
>>   
>>   /**
>>    * xe_guc_pc_c_status - get the current GT C state
>> @@ -664,13 +682,13 @@ u64 xe_guc_pc_mc6_residency(struct xe_guc_pc *pc)
>>   }
>>   
>>   static const struct attribute *pc_attrs[] = {
>> -	&dev_attr_freq_act.attr,
>> -	&dev_attr_freq_cur.attr,
>> -	&dev_attr_freq_rp0.attr,
>> -	&dev_attr_freq_rpe.attr,
>> -	&dev_attr_freq_rpn.attr,
>> -	&dev_attr_freq_min.attr,
>> -	&dev_attr_freq_max.attr,
>> +	&dev_attr_act_freq.attr,
>> +	&dev_attr_cur_freq.attr,
>> +	&dev_attr_rp0_freq.attr,
>> +	&dev_attr_rpe_freq.attr,
>> +	&dev_attr_rpn_freq.attr,
>> +	&dev_attr_min_freq.attr,
>> +	&dev_attr_max_freq.attr,
>>   	NULL
>>   };
>>   
>> @@ -950,6 +968,7 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>>   	struct xe_tile *tile = gt_to_tile(gt);
>>   	struct xe_device *xe = gt_to_xe(gt);
>>   	struct xe_bo *bo;
>> +	struct kobject *kobj;
>>   	u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data));
>>   	int err;
>>   
>> @@ -965,9 +984,15 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>>   
>>   	pc->bo = bo;
>>   
>> -	err = sysfs_create_files(gt->sysfs, pc_attrs);
>> -	if (err)
>> +	kobj = kobject_create_and_add("freq", gt->sysfs);
>> +	if (!kobj)
>> +		return -ENOMEM;
>> +
>> +	err = sysfs_create_files(kobj, pc_attrs);
>> +	if (err) {
>> +		kobject_put(kobj);
>>   		return err;
>> +	}
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/gpu/drm/xe/xe_guc_pc_types.h b/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> index 2afd0dbc3542..a48c22687ca8 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> +++ b/drivers/gpu/drm/xe/xe_guc_pc_types.h
>> @@ -31,4 +31,7 @@ struct xe_guc_pc {
>>   	bool freq_ready;
>>   };
>>   
>> +struct xe_gt_frq {
>> +};
>> +
>>   #endif	/* _XE_GUC_PC_TYPES_H_ */


More information about the Intel-xe mailing list