| Message ID | 20220217144158.21555-7-andi.shyti@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce multitile support | expand |
Hi Andi,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next drm/drm-next tegra-drm/drm/tegra/for-next airlied/drm-next v5.17-rc4 next-20220217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andi-Shyti/Introduce-multitile-support/20220217-224547
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a011 (https://download.01.org/0day-ci/archive/20220218/202202180224.l042viYj-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/f1802e7224006bf4801fe56193bf5eb223a3f4d0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andi-Shyti/Introduce-multitile-support/20220217-224547
git checkout f1802e7224006bf4801fe56193bf5eb223a3f4d0
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 'act_freq_mhz_show':
>> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:250:20: error: implicit declaration of function 'sysfs_gt_attribute_r_func' [-Werror=implicit-function-declaration]
250 | s64 actual_freq = sysfs_gt_attribute_r_func(dev, attr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c: In function 'boost_freq_mhz_store':
>> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:318:9: error: implicit declaration of function 'sysfs_gt_attribute_w_func' [-Werror=implicit-function-declaration]
318 | return sysfs_gt_attribute_w_func(dev, attr,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/sysfs_gt_attribute_r_func +250 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
246
247 static ssize_t act_freq_mhz_show(struct device *dev,
248 struct device_attribute *attr, char *buff)
249 {
> 250 s64 actual_freq = sysfs_gt_attribute_r_func(dev, attr,
251 __act_freq_mhz_show);
252
253 return sysfs_emit(buff, "%u\n", (u32) actual_freq);
254 }
255
256 static s64 __cur_freq_mhz_show(struct intel_gt *gt)
257 {
258 return intel_rps_get_requested_frequency(>->rps);
259 }
260
261 static ssize_t cur_freq_mhz_show(struct device *dev,
262 struct device_attribute *attr, char *buff)
263 {
264 s64 cur_freq = sysfs_gt_attribute_r_func(dev, attr,
265 __cur_freq_mhz_show);
266
267 return sysfs_emit(buff, "%u\n", (u32) cur_freq);
268 }
269
270 static s64 __boost_freq_mhz_show(struct intel_gt *gt)
271 {
272 return intel_rps_get_boost_frequency(>->rps);
273 }
274
275 static ssize_t boost_freq_mhz_show(struct device *dev,
276 struct device_attribute *attr,
277 char *buff)
278 {
279 s64 boost_freq = sysfs_gt_attribute_r_func(dev, attr,
280 __boost_freq_mhz_show);
281
282 return sysfs_emit(buff, "%u\n", (u32) boost_freq);
283 }
284
285 static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
286 {
287 struct intel_rps *rps = >->rps;
288 bool boost = false;
289
290 /* Validate against (static) hardware limits */
291 val = intel_freq_opcode(rps, val);
292 if (val < rps->min_freq || val > rps->max_freq)
293 return -EINVAL;
294
295 mutex_lock(&rps->lock);
296 if (val != rps->boost_freq) {
297 rps->boost_freq = val;
298 boost = atomic_read(&rps->num_waiters);
299 }
300 mutex_unlock(&rps->lock);
301 if (boost)
302 schedule_work(&rps->work);
303
304 return 0;
305 }
306
307 static ssize_t boost_freq_mhz_store(struct device *dev,
308 struct device_attribute *attr,
309 const char *buff, size_t count)
310 {
311 ssize_t ret;
312 u32 val;
313
314 ret = kstrtou32(buff, 0, &val);
315 if (ret)
316 return ret;
317
> 318 return sysfs_gt_attribute_w_func(dev, attr,
319 __boost_freq_mhz_store, val) ?: count;
320 }
321
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 17.02.2022 15:41, Andi Shyti wrote: > Now tiles have their own sysfs interfaces under the gt/ > directory. Because RPS is a property that can be configured on a > tile basis, then each tile should have its own interface > > The new sysfs structure will have a similar layout for the 4 tile > case: > > /sys/.../card0 > ├── gt > │ ├── gt0 > │ │ ├── id > │ │ ├── rc6_enable > │ │ ├── rc6_residency_ms > │ │ ├── rps_act_freq_mhz > │ │ ├── rps_boost_freq_mhz > │ │ ├── rps_cur_freq_mhz > │ │ ├── rps_max_freq_mhz > │ │ ├── rps_min_freq_mhz > │ │ ├── rps_RP0_freq_mhz > │ │ ├── rps_RP1_freq_mhz > │ │ └── rps_RPn_freq_mhz > . . > . . > . . > │ └── gtN > │ ├── id > │ ├── rc6_enable > │ ├── rc6_residency_ms > │ ├── rps_act_freq_mhz > │ ├── rps_boost_freq_mhz > │ ├── rps_cur_freq_mhz > │ ├── rps_max_freq_mhz > │ ├── rps_min_freq_mhz > │ ├── rps_RP0_freq_mhz > │ ├── rps_RP1_freq_mhz > │ └── rps_RPn_freq_mhz > ├── gt_act_freq_mhz -+ > ├── gt_boost_freq_mhz | > ├── gt_cur_freq_mhz | Original interface > ├── gt_max_freq_mhz +─-> kept as existing ABI; > ├── gt_min_freq_mhz | it points to gt0/ > ├── gt_RP0_freq_mhz | > ├── gt_RP1_freq_mhz | > └── gt_RPn_freq_mhz -+ > > The existing interfaces have been kept in their original location > to preserve the existing ABI. They act on all the GTs: when > writing they loop through all the GTs and write the information > on each interface. When reading they provide the average value > from all the GTs. > > This patch is not really adding exposing new interfaces (new > ABI) other than adapting the existing one to more tiles. In any > case this new set of interfaces will be a basic tool for system > managers and administrators when using i915. > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 276 ++++++++++++++++++++ > drivers/gpu/drm/i915/i915_sysfs.c | 177 ------------- > 2 files changed, 276 insertions(+), 177 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c > index 27d93a36894a..8e86b8f675f1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c > @@ -14,8 +14,33 @@ > #include "intel_gt_sysfs.h" > #include "intel_gt_sysfs_pm.h" > #include "intel_rc6.h" > +#include "intel_rps.h" > > #ifdef CONFIG_PM > +static int > +sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr, > + int (func)(struct intel_gt *gt, u32 val), u32 val) > +{ > + struct intel_gt *gt; > + int ret; > + > + if (!is_object_gt(&dev->kobj)) { > + int i; > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > + > + for_each_gt(gt, i915, i) { > + ret = func(gt, val); > + if (ret) > + break; > + } > + } else { > + gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name); > + ret = func(gt, val); > + } > + > + return ret; > +} > + > static s64 > sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr, > s64 (func)(struct intel_gt *gt)) > @@ -214,7 +239,258 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > } > #endif /* CONFIG_PM */ > > +static s64 __act_freq_mhz_show(struct intel_gt *gt) > +{ > + return intel_rps_read_actual_frequency(>->rps); > +} > + > +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. > +} > + > +static s64 __cur_freq_mhz_show(struct intel_gt *gt) > +{ > + return intel_rps_get_requested_frequency(>->rps); > +} > + > +static ssize_t cur_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + s64 cur_freq = sysfs_gt_attribute_r_func(dev, attr, > + __cur_freq_mhz_show); > + > + return sysfs_emit(buff, "%u\n", (u32) cur_freq); > +} > + > +static s64 __boost_freq_mhz_show(struct intel_gt *gt) > +{ > + return intel_rps_get_boost_frequency(>->rps); > +} > + > +static ssize_t boost_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + s64 boost_freq = sysfs_gt_attribute_r_func(dev, attr, > + __boost_freq_mhz_show); > + > + return sysfs_emit(buff, "%u\n", (u32) boost_freq); > +} > + > +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? > +} > + > +static ssize_t boost_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + return sysfs_gt_attribute_w_func(dev, attr, > + __boost_freq_mhz_store, val) ?: count; > +} > + > +static s64 __vlv_rpe_freq_mhz_show(struct intel_gt *gt) > +{ > + struct intel_rps *rps = >->rps; > + > + return intel_gpu_freq(rps, rps->efficient_freq); > +} > + > +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + s64 rpe_freq = sysfs_gt_attribute_r_func(dev, attr, > + __vlv_rpe_freq_mhz_show); > + > + return sysfs_emit(buff, "%d\n", (int) rpe_freq); > +} > + > +static s64 __max_freq_mhz_show(struct intel_gt *gt) > +{ > + return intel_rps_get_max_frequency(>->rps); > +} > + > +static int __set_max_freq(struct intel_gt *gt, u32 val) > +{ > + return intel_rps_set_max_frequency(>->rps, val); > +} > + > +static s64 __min_freq_mhz_show(struct intel_gt *gt) > +{ > + return intel_rps_get_min_frequency(>->rps); > +} > + > +static int __set_min_freq(struct intel_gt *gt, u32 val) > +{ > + return intel_rps_set_min_frequency(>->rps, val); > +} > + > +static ssize_t min_max_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count); > +static ssize_t min_max_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff); > + > +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \ > + struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \ > + struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store) > + > +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \ > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL) > +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \ > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store) > + > +static INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz); > +static INTEL_GT_RPS_SYSFS_ATTR_RO(cur_freq_mhz); > +static INTEL_GT_RPS_SYSFS_ATTR_RW(boost_freq_mhz); > + > +static DEVICE_ATTR_RO(vlv_rpe_freq_mhz); > + > +static ssize_t rps_rp_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff); > + > +static INTEL_GT_RPS_SYSFS_ATTR(RP0_freq_mhz, 0444, rps_rp_mhz_show, NULL); > +static INTEL_GT_RPS_SYSFS_ATTR(RP1_freq_mhz, 0444, rps_rp_mhz_show, NULL); > +static INTEL_GT_RPS_SYSFS_ATTR(RPn_freq_mhz, 0444, rps_rp_mhz_show, NULL); > + > +INTEL_GT_RPS_SYSFS_ATTR(max_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store); > +INTEL_GT_RPS_SYSFS_ATTR(min_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store); > + > +#define GEN6_ATTR(s) { \ > + &dev_attr_##s##_act_freq_mhz.attr, \ > + &dev_attr_##s##_cur_freq_mhz.attr, \ > + &dev_attr_##s##_boost_freq_mhz.attr, \ > + &dev_attr_##s##_max_freq_mhz.attr, \ > + &dev_attr_##s##_min_freq_mhz.attr, \ > + &dev_attr_##s##_RP0_freq_mhz.attr, \ > + &dev_attr_##s##_RP1_freq_mhz.attr, \ > + &dev_attr_##s##_RPn_freq_mhz.attr, \ > + NULL, \ > + } > + > +#define GEN6_RPS_ATTR GEN6_ATTR(rps) > +#define GEN6_GT_ATTR GEN6_ATTR(gt) > + > +static ssize_t min_max_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + long ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + ret = (attr == &dev_attr_gt_min_freq_mhz || > + attr == &dev_attr_rps_min_freq_mhz) ? > + sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val) : > + sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val); > + > + return ret ?: count; > +} > + > +static ssize_t min_max_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + s64 val; > + > + val = (attr == &dev_attr_gt_min_freq_mhz || > + attr == &dev_attr_rps_min_freq_mhz) ? > + sysfs_gt_attribute_r_func(dev, attr, __min_freq_mhz_show) : > + sysfs_gt_attribute_r_func(dev, attr, __max_freq_mhz_show); > + > + return sysfs_emit(buff, "%u\n", (u32) val); > +} > + > +/* 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. Regards Andrzej > + } > + > + return sysfs_emit(buff, "%d\n", val); > +} > + > +static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR; > +static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR; > + > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj, > + const struct attribute * const *attrs) > +{ > + int ret; > + > + if (GRAPHICS_VER(gt->i915) < 6) > + return 0; > + > + ret = sysfs_create_files(kobj, attrs); > + if (ret) > + return ret; > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr); > + > + return ret; > +} > + > void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) > { > + int ret; > + > intel_sysfs_rc6_init(gt, kobj); > + > + ret = is_object_gt(kobj) ? > + intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) : > + intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs); > + if (ret) > + drm_warn(>->i915->drm, > + "failed to create gt%u RPS sysfs files", gt->info.id); > } > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index b08a959e5ccc..0a0057940718 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -156,171 +156,6 @@ static const struct bin_attribute dpf_attrs_1 = { > .private = (void *)1 > }; > > -static ssize_t gt_act_freq_mhz_show(struct device *kdev, > - struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(i915)->rps; > - > - return sysfs_emit(buf, "%d\n", intel_rps_read_actual_frequency(rps)); > -} > - > -static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > - struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(i915)->rps; > - > - return sysfs_emit(buf, "%d\n", intel_rps_get_requested_frequency(rps)); > -} > - > -static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(i915)->rps; > - > - return sysfs_emit(buf, "%d\n", intel_rps_get_boost_frequency(rps)); > -} > - > -static ssize_t gt_boost_freq_mhz_store(struct device *kdev, > - struct device_attribute *attr, > - const char *buf, size_t count) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(dev_priv)->rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - ret = intel_rps_set_boost_frequency(rps, val); > - > - return ret ?: count; > -} > - > -static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, > - struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(dev_priv)->rps; > - > - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->efficient_freq)); > -} > - > -static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_gt *gt = to_gt(dev_priv); > - struct intel_rps *rps = >->rps; > - > - return sysfs_emit(buf, "%d\n", intel_rps_get_max_frequency(rps)); > -} > - > -static ssize_t gt_max_freq_mhz_store(struct device *kdev, > - struct device_attribute *attr, > - const char *buf, size_t count) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_gt *gt = to_gt(dev_priv); > - struct intel_rps *rps = >->rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - ret = intel_rps_set_max_frequency(rps, val); > - > - return ret ?: count; > -} > - > -static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > - struct intel_gt *gt = to_gt(i915); > - struct intel_rps *rps = >->rps; > - > - return sysfs_emit(buf, "%d\n", intel_rps_get_min_frequency(rps)); > -} > - > -static ssize_t gt_min_freq_mhz_store(struct device *kdev, > - struct device_attribute *attr, > - const char *buf, size_t count) > -{ > - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(i915)->rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - ret = intel_rps_set_min_frequency(rps, val); > - > - return ret ?: count; > -} > - > -static DEVICE_ATTR_RO(gt_act_freq_mhz); > -static DEVICE_ATTR_RO(gt_cur_freq_mhz); > -static DEVICE_ATTR_RW(gt_boost_freq_mhz); > -static DEVICE_ATTR_RW(gt_max_freq_mhz); > -static DEVICE_ATTR_RW(gt_min_freq_mhz); > - > -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz); > - > -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf); > -static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); > -static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); > -static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); > - > -/* For now we have a static number of RP states */ > -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &to_gt(dev_priv)->rps; > - u32 val; > - > - if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_rps_get_rp0_frequency(rps); > - else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_rps_get_rp1_frequency(rps); > - else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_rps_get_rpn_frequency(rps); > - else > - BUG(); > - > - return sysfs_emit(buf, "%d\n", val); > -} > - > -static const struct attribute * const gen6_attrs[] = { > - &dev_attr_gt_act_freq_mhz.attr, > - &dev_attr_gt_cur_freq_mhz.attr, > - &dev_attr_gt_boost_freq_mhz.attr, > - &dev_attr_gt_max_freq_mhz.attr, > - &dev_attr_gt_min_freq_mhz.attr, > - &dev_attr_gt_RP0_freq_mhz.attr, > - &dev_attr_gt_RP1_freq_mhz.attr, > - &dev_attr_gt_RPn_freq_mhz.attr, > - NULL, > -}; > - > -static const struct attribute * const vlv_attrs[] = { > - &dev_attr_gt_act_freq_mhz.attr, > - &dev_attr_gt_cur_freq_mhz.attr, > - &dev_attr_gt_boost_freq_mhz.attr, > - &dev_attr_gt_max_freq_mhz.attr, > - &dev_attr_gt_min_freq_mhz.attr, > - &dev_attr_gt_RP0_freq_mhz.attr, > - &dev_attr_gt_RP1_freq_mhz.attr, > - &dev_attr_gt_RPn_freq_mhz.attr, > - &dev_attr_vlv_rpe_freq_mhz.attr, > - NULL, > -}; > - > #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) > > static ssize_t error_state_read(struct file *filp, struct kobject *kobj, > @@ -411,14 +246,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) > } > } > > - ret = 0; > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - ret = sysfs_create_files(&kdev->kobj, vlv_attrs); > - else if (GRAPHICS_VER(dev_priv) >= 6) > - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); > - if (ret) > - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); > - > dev_priv->sysfs_gt = i915_setup_gt_sysfs(&kdev->kobj); > if (!dev_priv->sysfs_gt) > drm_warn(&dev_priv->drm, > @@ -435,10 +262,6 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv) > > i915_teardown_error_capture(kdev); > > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) > - sysfs_remove_files(&kdev->kobj, vlv_attrs); > - else > - sysfs_remove_files(&kdev->kobj, gen6_attrs); > device_remove_bin_file(kdev, &dpf_attrs_1); > device_remove_bin_file(kdev, &dpf_attrs); > }
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
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c index 27d93a36894a..8e86b8f675f1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c @@ -14,8 +14,33 @@ #include "intel_gt_sysfs.h" #include "intel_gt_sysfs_pm.h" #include "intel_rc6.h" +#include "intel_rps.h" #ifdef CONFIG_PM +static int +sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr, + int (func)(struct intel_gt *gt, u32 val), u32 val) +{ + struct intel_gt *gt; + int ret; + + if (!is_object_gt(&dev->kobj)) { + int i; + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); + + for_each_gt(gt, i915, i) { + ret = func(gt, val); + if (ret) + break; + } + } else { + gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name); + ret = func(gt, val); + } + + return ret; +} + static s64 sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr, s64 (func)(struct intel_gt *gt)) @@ -214,7 +239,258 @@ static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) } #endif /* CONFIG_PM */ +static s64 __act_freq_mhz_show(struct intel_gt *gt) +{ + return intel_rps_read_actual_frequency(>->rps); +} + +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); +} + +static s64 __cur_freq_mhz_show(struct intel_gt *gt) +{ + return intel_rps_get_requested_frequency(>->rps); +} + +static ssize_t cur_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + s64 cur_freq = sysfs_gt_attribute_r_func(dev, attr, + __cur_freq_mhz_show); + + return sysfs_emit(buff, "%u\n", (u32) cur_freq); +} + +static s64 __boost_freq_mhz_show(struct intel_gt *gt) +{ + return intel_rps_get_boost_frequency(>->rps); +} + +static ssize_t boost_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + s64 boost_freq = sysfs_gt_attribute_r_func(dev, attr, + __boost_freq_mhz_show); + + return sysfs_emit(buff, "%u\n", (u32) boost_freq); +} + +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; +} + +static ssize_t boost_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + return sysfs_gt_attribute_w_func(dev, attr, + __boost_freq_mhz_store, val) ?: count; +} + +static s64 __vlv_rpe_freq_mhz_show(struct intel_gt *gt) +{ + struct intel_rps *rps = >->rps; + + return intel_gpu_freq(rps, rps->efficient_freq); +} + +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + s64 rpe_freq = sysfs_gt_attribute_r_func(dev, attr, + __vlv_rpe_freq_mhz_show); + + return sysfs_emit(buff, "%d\n", (int) rpe_freq); +} + +static s64 __max_freq_mhz_show(struct intel_gt *gt) +{ + return intel_rps_get_max_frequency(>->rps); +} + +static int __set_max_freq(struct intel_gt *gt, u32 val) +{ + return intel_rps_set_max_frequency(>->rps, val); +} + +static s64 __min_freq_mhz_show(struct intel_gt *gt) +{ + return intel_rps_get_min_frequency(>->rps); +} + +static int __set_min_freq(struct intel_gt *gt, u32 val) +{ + return intel_rps_set_min_frequency(>->rps, val); +} + +static ssize_t min_max_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count); +static ssize_t min_max_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff); + +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \ + struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \ + struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store) + +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \ + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL) +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \ + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store) + +static INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz); +static INTEL_GT_RPS_SYSFS_ATTR_RO(cur_freq_mhz); +static INTEL_GT_RPS_SYSFS_ATTR_RW(boost_freq_mhz); + +static DEVICE_ATTR_RO(vlv_rpe_freq_mhz); + +static ssize_t rps_rp_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff); + +static INTEL_GT_RPS_SYSFS_ATTR(RP0_freq_mhz, 0444, rps_rp_mhz_show, NULL); +static INTEL_GT_RPS_SYSFS_ATTR(RP1_freq_mhz, 0444, rps_rp_mhz_show, NULL); +static INTEL_GT_RPS_SYSFS_ATTR(RPn_freq_mhz, 0444, rps_rp_mhz_show, NULL); + +INTEL_GT_RPS_SYSFS_ATTR(max_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store); +INTEL_GT_RPS_SYSFS_ATTR(min_freq_mhz, 0644, min_max_freq_mhz_show, min_max_freq_mhz_store); + +#define GEN6_ATTR(s) { \ + &dev_attr_##s##_act_freq_mhz.attr, \ + &dev_attr_##s##_cur_freq_mhz.attr, \ + &dev_attr_##s##_boost_freq_mhz.attr, \ + &dev_attr_##s##_max_freq_mhz.attr, \ + &dev_attr_##s##_min_freq_mhz.attr, \ + &dev_attr_##s##_RP0_freq_mhz.attr, \ + &dev_attr_##s##_RP1_freq_mhz.attr, \ + &dev_attr_##s##_RPn_freq_mhz.attr, \ + NULL, \ + } + +#define GEN6_RPS_ATTR GEN6_ATTR(rps) +#define GEN6_GT_ATTR GEN6_ATTR(gt) + +static ssize_t min_max_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + long ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + ret = (attr == &dev_attr_gt_min_freq_mhz || + attr == &dev_attr_rps_min_freq_mhz) ? + sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val) : + sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val); + + return ret ?: count; +} + +static ssize_t min_max_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + s64 val; + + val = (attr == &dev_attr_gt_min_freq_mhz || + attr == &dev_attr_rps_min_freq_mhz) ? + sysfs_gt_attribute_r_func(dev, attr, __min_freq_mhz_show) : + sysfs_gt_attribute_r_func(dev, attr, __max_freq_mhz_show); + + return sysfs_emit(buff, "%u\n", (u32) val); +} + +/* 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; + } + + return sysfs_emit(buff, "%d\n", val); +} + +static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR; +static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR; + +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj, + const struct attribute * const *attrs) +{ + int ret; + + if (GRAPHICS_VER(gt->i915) < 6) + return 0; + + ret = sysfs_create_files(kobj, attrs); + if (ret) + return ret; + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr); + + return ret; +} + void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) { + int ret; + intel_sysfs_rc6_init(gt, kobj); + + ret = is_object_gt(kobj) ? + intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) : + intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs); + if (ret) + drm_warn(>->i915->drm, + "failed to create gt%u RPS sysfs files", gt->info.id); } diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index b08a959e5ccc..0a0057940718 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -156,171 +156,6 @@ static const struct bin_attribute dpf_attrs_1 = { .private = (void *)1 }; -static ssize_t gt_act_freq_mhz_show(struct device *kdev, - struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(i915)->rps; - - return sysfs_emit(buf, "%d\n", intel_rps_read_actual_frequency(rps)); -} - -static ssize_t gt_cur_freq_mhz_show(struct device *kdev, - struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(i915)->rps; - - return sysfs_emit(buf, "%d\n", intel_rps_get_requested_frequency(rps)); -} - -static ssize_t gt_boost_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(i915)->rps; - - return sysfs_emit(buf, "%d\n", intel_rps_get_boost_frequency(rps)); -} - -static ssize_t gt_boost_freq_mhz_store(struct device *kdev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(dev_priv)->rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - ret = intel_rps_set_boost_frequency(rps, val); - - return ret ?: count; -} - -static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, - struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(dev_priv)->rps; - - return sysfs_emit(buf, "%d\n", intel_gpu_freq(rps, rps->efficient_freq)); -} - -static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - struct intel_gt *gt = to_gt(dev_priv); - struct intel_rps *rps = >->rps; - - return sysfs_emit(buf, "%d\n", intel_rps_get_max_frequency(rps)); -} - -static ssize_t gt_max_freq_mhz_store(struct device *kdev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - struct intel_gt *gt = to_gt(dev_priv); - struct intel_rps *rps = >->rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - ret = intel_rps_set_max_frequency(rps, val); - - return ret ?: count; -} - -static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); - struct intel_gt *gt = to_gt(i915); - struct intel_rps *rps = >->rps; - - return sysfs_emit(buf, "%d\n", intel_rps_get_min_frequency(rps)); -} - -static ssize_t gt_min_freq_mhz_store(struct device *kdev, - struct device_attribute *attr, - const char *buf, size_t count) -{ - struct drm_i915_private *i915 = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(i915)->rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - ret = intel_rps_set_min_frequency(rps, val); - - return ret ?: count; -} - -static DEVICE_ATTR_RO(gt_act_freq_mhz); -static DEVICE_ATTR_RO(gt_cur_freq_mhz); -static DEVICE_ATTR_RW(gt_boost_freq_mhz); -static DEVICE_ATTR_RW(gt_max_freq_mhz); -static DEVICE_ATTR_RW(gt_min_freq_mhz); - -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz); - -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf); -static DEVICE_ATTR(gt_RP0_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); -static DEVICE_ATTR(gt_RP1_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); -static DEVICE_ATTR(gt_RPn_freq_mhz, S_IRUGO, gt_rp_mhz_show, NULL); - -/* For now we have a static number of RP states */ -static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &to_gt(dev_priv)->rps; - u32 val; - - if (attr == &dev_attr_gt_RP0_freq_mhz) - val = intel_rps_get_rp0_frequency(rps); - else if (attr == &dev_attr_gt_RP1_freq_mhz) - val = intel_rps_get_rp1_frequency(rps); - else if (attr == &dev_attr_gt_RPn_freq_mhz) - val = intel_rps_get_rpn_frequency(rps); - else - BUG(); - - return sysfs_emit(buf, "%d\n", val); -} - -static const struct attribute * const gen6_attrs[] = { - &dev_attr_gt_act_freq_mhz.attr, - &dev_attr_gt_cur_freq_mhz.attr, - &dev_attr_gt_boost_freq_mhz.attr, - &dev_attr_gt_max_freq_mhz.attr, - &dev_attr_gt_min_freq_mhz.attr, - &dev_attr_gt_RP0_freq_mhz.attr, - &dev_attr_gt_RP1_freq_mhz.attr, - &dev_attr_gt_RPn_freq_mhz.attr, - NULL, -}; - -static const struct attribute * const vlv_attrs[] = { - &dev_attr_gt_act_freq_mhz.attr, - &dev_attr_gt_cur_freq_mhz.attr, - &dev_attr_gt_boost_freq_mhz.attr, - &dev_attr_gt_max_freq_mhz.attr, - &dev_attr_gt_min_freq_mhz.attr, - &dev_attr_gt_RP0_freq_mhz.attr, - &dev_attr_gt_RP1_freq_mhz.attr, - &dev_attr_gt_RPn_freq_mhz.attr, - &dev_attr_vlv_rpe_freq_mhz.attr, - NULL, -}; - #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR) static ssize_t error_state_read(struct file *filp, struct kobject *kobj, @@ -411,14 +246,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) } } - ret = 0; - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) - ret = sysfs_create_files(&kdev->kobj, vlv_attrs); - else if (GRAPHICS_VER(dev_priv) >= 6) - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); - if (ret) - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); - dev_priv->sysfs_gt = i915_setup_gt_sysfs(&kdev->kobj); if (!dev_priv->sysfs_gt) drm_warn(&dev_priv->drm, @@ -435,10 +262,6 @@ void i915_teardown_sysfs(struct drm_i915_private *dev_priv) i915_teardown_error_capture(kdev); - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) - sysfs_remove_files(&kdev->kobj, vlv_attrs); - else - sysfs_remove_files(&kdev->kobj, gen6_attrs); device_remove_bin_file(kdev, &dpf_attrs_1); device_remove_bin_file(kdev, &dpf_attrs); }
Now tiles have their own sysfs interfaces under the gt/ directory. Because RPS is a property that can be configured on a tile basis, then each tile should have its own interface The new sysfs structure will have a similar layout for the 4 tile case: /sys/.../card0 ├── gt │ ├── gt0 │ │ ├── id │ │ ├── rc6_enable │ │ ├── rc6_residency_ms │ │ ├── rps_act_freq_mhz │ │ ├── rps_boost_freq_mhz │ │ ├── rps_cur_freq_mhz │ │ ├── rps_max_freq_mhz │ │ ├── rps_min_freq_mhz │ │ ├── rps_RP0_freq_mhz │ │ ├── rps_RP1_freq_mhz │ │ └── rps_RPn_freq_mhz . . . . . . │ └── gtN │ ├── id │ ├── rc6_enable │ ├── rc6_residency_ms │ ├── rps_act_freq_mhz │ ├── rps_boost_freq_mhz │ ├── rps_cur_freq_mhz │ ├── rps_max_freq_mhz │ ├── rps_min_freq_mhz │ ├── rps_RP0_freq_mhz │ ├── rps_RP1_freq_mhz │ └── rps_RPn_freq_mhz ├── gt_act_freq_mhz -+ ├── gt_boost_freq_mhz | ├── gt_cur_freq_mhz | Original interface ├── gt_max_freq_mhz +─-> kept as existing ABI; ├── gt_min_freq_mhz | it points to gt0/ ├── gt_RP0_freq_mhz | ├── gt_RP1_freq_mhz | └── gt_RPn_freq_mhz -+ The existing interfaces have been kept in their original location to preserve the existing ABI. They act on all the GTs: when writing they loop through all the GTs and write the information on each interface. When reading they provide the average value from all the GTs. This patch is not really adding exposing new interfaces (new ABI) other than adapting the existing one to more tiles. In any case this new set of interfaces will be a basic tool for system managers and administrators when using i915. Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 276 ++++++++++++++++++++ drivers/gpu/drm/i915/i915_sysfs.c | 177 ------------- 2 files changed, 276 insertions(+), 177 deletions(-)