13 Mar
2022
13 Mar
'22
11:09 p.m.
Hi Andrzej,
[...]
+static ssize_t act_freq_mhz_show(struct device *dev,
struct device_attribute *attr, char *buff)+{
- s64 actual_freq = sysfs_gt_attribute_r_func(dev, attr,
__act_freq_mhz_show);- return sysfs_emit(buff, "%u\n", (u32) actual_freq);
Again, variable can be just u32.
yes
[...]
+static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val) +{
- struct intel_rps *rps = >->rps;
- bool boost = false;
- /* Validate against (static) hardware limits */
- val = intel_freq_opcode(rps, val);
- if (val < rps->min_freq || val > rps->max_freq)
return -EINVAL;- mutex_lock(&rps->lock);
- if (val != rps->boost_freq) {
rps->boost_freq = val;boost = atomic_read(&rps->num_waiters);- }
- mutex_unlock(&rps->lock);
- if (boost)
schedule_work(&rps->work);- return 0;
Why not intel_rps_set_boost_frequency? Why copy/paste from rps_set_boost_freq?
you are right... this must be some ugly leftover. Thanks!
[...]
+/* For now we have a static number of RP states */ +static ssize_t rps_rp_mhz_show(struct device *dev,
struct device_attribute *attr,char *buff)+{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
- struct intel_rps *rps = >->rps;
- u32 val;
- if (attr == &dev_attr_gt_RP0_freq_mhz ||
attr == &dev_attr_rps_RP0_freq_mhz) {val = intel_rps_get_rp0_frequency(rps);- } else if (attr == &dev_attr_gt_RP1_freq_mhz ||
attr == &dev_attr_rps_RP1_freq_mhz) {val = intel_rps_get_rp1_frequency(rps);- } else if (attr == &dev_attr_gt_RPn_freq_mhz ||
attr == &dev_attr_rps_RPn_freq_mhz) {val = intel_rps_get_rpn_frequency(rps);- } else {
GEM_WARN_ON(1);return -ENODEV;I guess this is not necessary, otherwise similar path should be in other helpers.
yeah... it's a bit hacky, I must agree... will split it properly.
[...]
Thanks, Andi