| Message ID | 20200219193020.17673-1-andi.shyti@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] drm/i915/gt: make a gt sysfs group and move power management files | expand |
On Wed, 19 Feb 2020, Andi Shyti <andi.shyti@intel.com> wrote: > The GT has its own properties and in sysfs they should be grouped > in the 'gt/' directory. > > Create the 'gt/' directory in sysfs and move the power management > related files. > > The new interfaces are: > > gt/gt_act_freq_mhz > gt/gt_boost_freq_mhz > gt/gt_cur_freq_mhz > gt/gt_info > gt/gt_max_freq_mhz > gt/gt_min_freq_mhz > gt/gt_RP0_freq_mhz > gt/gt_RP1_freq_mhz > gt/gt_RPn_freq_mhz > gt/rc6_enable > gt/rc6_residency_ms > > The once in the root directory will be marked as deprecated, if > accessed a warning message is printed. > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > --- > v4 -> v5: > - removed spurious ghost 'vvv' file that was never meant to be > there... sorry for spamming. > v3 -> v4: > - fixed Tvrtko's comments: > - some renaming > - some clumsy unbalanced kobject_put/get > - the warning print is more descriptive and printed with > limited rate > - TODO: drm_print doesn't have a drm_warn_unlimited, to > be added > v2 -> v3: > - fix some cleanups that I forgot in the previous patch > - fix reference pointers to the gt structure > - and many other small changes here and there. > v1 -> v2: > - keep the existing files as they are > - use "intel_gt_*" as prefix instead of "sysfs_*" > > drivers/gpu/drm/i915/Makefile | 4 +- > drivers/gpu/drm/i915/gt/intel_gt.c | 3 + > drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + > drivers/gpu/drm/i915/gt/sysfs_gt.c | 79 +++++ > drivers/gpu/drm/i915/gt/sysfs_gt.h | 22 ++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 432 +++++++++++++++++++++++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 17 + > drivers/gpu/drm/i915/i915_sysfs.c | 375 +------------------- > drivers/gpu/drm/i915/i915_sysfs.h | 3 + > 9 files changed, 561 insertions(+), 375 deletions(-) > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index b314d44ded5e..ff9e17c97dc2 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -107,7 +107,9 @@ gt-y += \ > gt/intel_rps.o \ > gt/intel_sseu.o \ > gt/intel_timeline.o \ > - gt/intel_workarounds.o > + gt/intel_workarounds.o \ > + gt/sysfs_gt.o \ > + gt/sysfs_gt_pm.o > # autogenerated null render state > gt-y += \ > gt/gen6_renderstate.o \ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index f1f1b306e0af..e794d05d69a1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -15,6 +15,7 @@ > #include "intel_rps.h" > #include "intel_uncore.h" > #include "intel_pm.h" > +#include "sysfs_gt.h" > > void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) > { > @@ -321,6 +322,7 @@ void intel_gt_driver_register(struct intel_gt *gt) > intel_rps_driver_register(>->rps); > > debugfs_gt_register(gt); > + intel_gt_sysfs_register(gt); > } > > static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size) > @@ -641,6 +643,7 @@ void intel_gt_driver_remove(struct intel_gt *gt) > > void intel_gt_driver_unregister(struct intel_gt *gt) > { > + intel_gt_sysfs_unregister(gt); > intel_rps_driver_unregister(>->rps); > } > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index 96890dd12b5f..7f0b4f8d9e28 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -32,6 +32,7 @@ struct intel_gt { > struct drm_i915_private *i915; > struct intel_uncore *uncore; > struct i915_ggtt *ggtt; > + struct kobject sysfs_root; > > struct intel_uc uc; > > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c b/drivers/gpu/drm/i915/gt/sysfs_gt.c > new file mode 100644 > index 000000000000..9335a92d5248 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c > @@ -0,0 +1,79 @@ > +// SPDX-License-Identifier: MIT > + Superfluous newline. > +/* > + * Copyright © 2019 Intel Corporation > + */ It's 2020 now. ;) > + > +#include <linux/sysfs.h> > +#include <drm/drm_device.h> > +#include <linux/kobject.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" No need for "../", it's in include path. > +#include "intel_gt.h" > +#include "intel_gt_types.h" > +#include "intel_rc6.h" > + > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +static inline struct kobject *gt_get_parent_obj(struct intel_gt *gt) In .c files just drop the inline keyword and let the compiler do what's best. > +{ > + return >->i915->drm.primary->kdev->kobj; > +} > + > +static ssize_t gt_info_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + return snprintf(buff, PAGE_SIZE, "0\n"); > +} > + > +static DEVICE_ATTR_RO(gt_info); > + > +static struct kobj_type sysfs_gt_ktype = { > + .sysfs_ops = &kobj_sysfs_ops, > +}; > + > +void intel_gt_sysfs_register(struct intel_gt *gt) > +{ > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); > + int ret; > + > + ret = kobject_init_and_add(>->sysfs_root, > + &sysfs_gt_ktype, > + parent, "gt"); > + if (ret) { > + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); > + kobject_put(>->sysfs_root); > + goto parent_files; > + } > + > + ret = sysfs_create_file(>->sysfs_root, &dev_attr_gt_info.attr); > + if (ret) > + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); > + > + intel_gt_sysfs_pm_init(gt, >->sysfs_root); > + > +parent_files: > + /* > + * we need to make things right with the > + * ABI compatibility. The files were originally > + * generated under the parent directory. > + */ > + intel_gt_sysfs_pm_init(gt, parent); > +} > + > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > +{ > + struct kobject *parent = gt_get_parent_obj(gt); > + > + /* > + * the name gt tells us wether sysfs_root > + * object was initialized properly > + */ > + if (!strcmp(gt->sysfs_root.name, "gt")) > + kobject_put(>->sysfs_root); > + > + kobject_put(parent); > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.h b/drivers/gpu/drm/i915/gt/sysfs_gt.h > new file mode 100644 > index 000000000000..2e479aa902e5 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: MIT */ > + Superfluous newline. > +/* > + * Copyright © 2019 Intel Corporation > + */ 2020. > + > +#ifndef SYSFS_GT_H > +#define SYSFS_GT_H Please add some underscores, e.g. __SYSFS_GT_H__. > + > +#include "intel_gt_types.h" > + > +struct intel_gt; > + > +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj) > +{ > + return container_of(kobj, struct intel_gt, sysfs_root); > +} > + > +void intel_gt_sysfs_register(struct intel_gt *gt); > +void intel_gt_sysfs_unregister(struct intel_gt *gt); > + > +#endif /* SYSFS_GT_H */ > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > new file mode 100644 > index 000000000000..c548eb851a70 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > @@ -0,0 +1,432 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ You know the drill. ;) > + > +#include <drm/drm_device.h> > +#include <linux/sysfs.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "../i915_sysfs.h" "../" is unnecessary. > +#include "intel_gt.h" > +#include "intel_rc6.h" > +#include "intel_rps.h" > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > +{ > + struct kobject *kobj = &dev->kobj; > + /* > + * We are interested at knowing from where the interface > + * has been called, whether it's called from gt/ or from > + * the parent directory. > + * From the interface position it depends also the value of > + * the private data. > + * If the interface is called from gt/ then private data is > + * of the "struct intel_gt *" type, otherwise it's * a > + * "struct drm_i915_private *" type. > + */ > + if (strcmp(dev->kobj.name, "gt")) { > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > + > + pr_warn_ratelimited(DEPRECATED > + "(%s, %d) trying to access deprecated interface, " > + "use the corresponding interface in gt/\n", > + current->comm, task_pid_nr(current)); > + return &i915->gt; > + } > + > + return kobj_to_gt(kobj); > +} > + > +#ifdef CONFIG_PM > +static u32 get_residency(struct intel_gt *gt, i915_reg_t reg) > +{ > + intel_wakeref_t wakeref; > + u64 res = 0; > + > + with_intel_runtime_pm(gt->uncore->rpm, wakeref) > + res = intel_rc6_residency_us(>->rc6, reg); > + > + return DIV_ROUND_CLOSEST_ULL(res, 1000); > +} > + > +static ssize_t rc6_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u8 mask = 0; > + > + if (HAS_RC6(gt->i915)) > + mask |= BIT(0); > + if (HAS_RC6p(gt->i915)) > + mask |= BIT(1); > + if (HAS_RC6pp(gt->i915)) > + mask |= BIT(2); > + > + return snprintf(buff, PAGE_SIZE, "%x\n", mask); > +} > + > +static ssize_t rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static ssize_t rc6p_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency); > +} > + > +static ssize_t rc6pp_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency); > +} > + > +static ssize_t media_rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static DEVICE_ATTR_RO(rc6_enable); > +static DEVICE_ATTR_RO(rc6_residency_ms); > +static DEVICE_ATTR_RO(rc6p_residency_ms); > +static DEVICE_ATTR_RO(rc6pp_residency_ms); > +static DEVICE_ATTR_RO(media_rc6_residency_ms); > + > +static const struct attribute *rc6_attrs[] = { > + &dev_attr_rc6_enable.attr, > + &dev_attr_rc6_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *rc6p_attrs[] = { > + &dev_attr_rc6p_residency_ms.attr, > + &dev_attr_rc6pp_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *media_rc6_attrs[] = { > + &dev_attr_media_rc6_residency_ms.attr, > + NULL > +}; > + > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret = 0; > + > + if (HAS_RC6(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6 sysfs files\n"); > + } > + > + if (HAS_RC6p(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6p_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6p sysfs files\n"); > + } > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { > + ret = sysfs_create_files(kobj, media_rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create media RC6 sysfs files\n"); > + } > +} > +#else > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + return 0; > +} > +#endif /* CONFIG_PM */ > + > +static ssize_t gt_act_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_rps_read_actual_frequency(>->rps)); > +} > + > +static ssize_t gt_cur_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->cur_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->boost_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + bool boost = false; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + /* 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 count; > +} > + > +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->efficient_freq)); > +} > + > +static ssize_t gt_max_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->max_freq_softlimit)); > +} > + > +static ssize_t gt_max_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val < rps->min_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (val > rps->rp0_freq) > + DRM_DEBUG("User requested overclocking to %d\n", > + intel_gpu_freq(rps, val)); > + > + rps->max_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new max_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret ?: count; > +} > + > +static ssize_t gt_min_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->min_freq_softlimit)); > +} > + > +static ssize_t gt_min_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val > rps->max_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + rps->min_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new min_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + 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 *dev, > + struct device_attribute *attr, > + char *buff); > + > +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); > + > +/* For now we have a static number of RP states */ > +static ssize_t gt_rp_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + u32 val; > + > + if (attr == &dev_attr_gt_RP0_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp0_freq); > + else if (attr == &dev_attr_gt_RP1_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp1_freq); > + else if (attr == &dev_attr_gt_RPn_freq_mhz) > + val = intel_gpu_freq(rps, rps->min_freq); > + else > + BUG(); > + > + return snprintf(buff, PAGE_SIZE, "%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, > +}; > + > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + return sysfs_create_files(kobj, vlv_attrs); > + > + if (INTEL_GEN(gt->i915) >= 6) > + return sysfs_create_files(kobj, gen6_attrs); > + > + return 0; > +} > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret; > + > + intel_sysfs_rc6_init(gt, kobj); > + > + ret = intel_sysfs_rps_init(gt, kobj); > + if (ret) > + drm_err(>->i915->drm, "failed to create RPS sysfs files"); > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > new file mode 100644 > index 000000000000..758d0c3cb998 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_RC6_H > +#define SYSFS_RC6_H All the things mentioned before; newline, year, underscores. Additionally make the include protection match the file name. > + > +#include <linux/kobject.h> > + > +#include "intel_gt_types.h" > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj); > +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj); > + > +#endif /* SYSFS_RC6_H */ > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index c14d762bd652..1298977fc08b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -38,113 +38,12 @@ > #include "intel_pm.h" > #include "intel_sideband.h" > > -static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > { > struct drm_minor *minor = dev_get_drvdata(kdev); > return to_i915(minor->dev); > } > > -#ifdef CONFIG_PM > -static u32 calc_residency(struct drm_i915_private *dev_priv, > - i915_reg_t reg) > -{ > - intel_wakeref_t wakeref; > - u64 res = 0; > - > - with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) > - res = intel_rc6_residency_us(&dev_priv->gt.rc6, reg); > - > - return DIV_ROUND_CLOSEST_ULL(res, 1000); > -} > - > -static ssize_t > -show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - unsigned int mask; > - > - mask = 0; > - if (HAS_RC6(dev_priv)) > - mask |= BIT(0); > - if (HAS_RC6p(dev_priv)) > - mask |= BIT(1); > - if (HAS_RC6pp(dev_priv)) > - mask |= BIT(2); > - > - return snprintf(buf, PAGE_SIZE, "%x\n", mask); > -} > - > -static ssize_t > -show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static ssize_t > -show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6p_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6p); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency); > -} > - > -static ssize_t > -show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6pp_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6pp); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency); > -} > - > -static ssize_t > -show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, VLV_GT_MEDIA_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL); > -static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL); > -static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL); > -static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); > -static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO, show_media_rc6_ms, NULL); > - > -static struct attribute *rc6_attrs[] = { > - &dev_attr_rc6_enable.attr, > - &dev_attr_rc6_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group rc6_attr_group = { > - .name = power_group_name, > - .attrs = rc6_attrs > -}; > - > -static struct attribute *rc6p_attrs[] = { > - &dev_attr_rc6p_residency_ms.attr, > - &dev_attr_rc6pp_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group rc6p_attr_group = { > - .name = power_group_name, > - .attrs = rc6p_attrs > -}; > - > -static struct attribute *media_rc6_attrs[] = { > - &dev_attr_media_rc6_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group media_rc6_attr_group = { > - .name = power_group_name, > - .attrs = media_rc6_attrs > -}; > -#endif > - > static int l3_access_valid(struct drm_i915_private *i915, loff_t offset) > { > if (!HAS_L3_DPF(i915)) > @@ -256,239 +155,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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->cur_freq)); > -} > - > -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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->boost_freq)); > -} > - > -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 = &dev_priv->gt.rps; > - bool boost = false; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - /* 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 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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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_rps *rps = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->max_freq_softlimit)); > -} > - > -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_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val < rps->min_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - if (val > rps->rp0_freq) > - DRM_DEBUG("User requested overclocking to %d\n", > - intel_gpu_freq(rps, val)); > - > - rps->max_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new max_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - return ret ?: count; > -} > - > -static ssize_t gt_min_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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->min_freq_softlimit)); > -} > - > -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 *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val > rps->max_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - rps->min_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new min_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - 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 = &dev_priv->gt.rps; > - u32 val; > - > - if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp0_freq); > - else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp1_freq); > - else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_gpu_freq(rps, rps->min_freq); > - else > - BUG(); > - > - return snprintf(buf, PAGE_SIZE, "%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, > @@ -559,29 +225,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) > struct device *kdev = dev_priv->drm.primary->kdev; > int ret; > > -#ifdef CONFIG_PM > - if (HAS_RC6(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6 residency sysfs setup failed\n"); > - } > - if (HAS_RC6p(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6p_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6p residency sysfs setup failed\n"); > - } > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &media_rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "Media RC6 residency sysfs setup failed\n"); > - } > -#endif > if (HAS_L3_DPF(dev_priv)) { > ret = device_create_bin_file(kdev, &dpf_attrs); > if (ret) > @@ -597,14 +240,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 (INTEL_GEN(dev_priv) >= 6) > - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); > - if (ret) > - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); > - > i915_setup_error_capture(kdev); > } > > @@ -614,14 +249,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); > -#ifdef CONFIG_PM > - sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group); > - sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group); > -#endif > } > diff --git a/drivers/gpu/drm/i915/i915_sysfs.h b/drivers/gpu/drm/i915/i915_sysfs.h > index 41afd4366416..ad6114de81c9 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.h > +++ b/drivers/gpu/drm/i915/i915_sysfs.h > @@ -6,8 +6,11 @@ > #ifndef __I915_SYSFS_H__ > #define __I915_SYSFS_H__ > > +#include <linux/device.h> You'll only need a forward declaration. > + > struct drm_i915_private; > > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev); > void i915_setup_sysfs(struct drm_i915_private *i915); > void i915_teardown_sysfs(struct drm_i915_private *i915);
On 19/02/2020 19:30, Andi Shyti wrote: > The GT has its own properties and in sysfs they should be grouped > in the 'gt/' directory. > > Create the 'gt/' directory in sysfs and move the power management > related files. > > The new interfaces are: > > gt/gt_act_freq_mhz > gt/gt_boost_freq_mhz > gt/gt_cur_freq_mhz > gt/gt_info > gt/gt_max_freq_mhz > gt/gt_min_freq_mhz > gt/gt_RP0_freq_mhz > gt/gt_RP1_freq_mhz > gt/gt_RPn_freq_mhz > gt/rc6_enable > gt/rc6_residency_ms > > The once in the root directory will be marked as deprecated, if > accessed a warning message is printed. > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > --- > v4 -> v5: > - removed spurious ghost 'vvv' file that was never meant to be > there... sorry for spamming. > v3 -> v4: > - fixed Tvrtko's comments: > - some renaming > - some clumsy unbalanced kobject_put/get > - the warning print is more descriptive and printed with > limited rate > - TODO: drm_print doesn't have a drm_warn_unlimited, to > be added > v2 -> v3: > - fix some cleanups that I forgot in the previous patch > - fix reference pointers to the gt structure > - and many other small changes here and there. > v1 -> v2: > - keep the existing files as they are > - use "intel_gt_*" as prefix instead of "sysfs_*" > > drivers/gpu/drm/i915/Makefile | 4 +- > drivers/gpu/drm/i915/gt/intel_gt.c | 3 + > drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + > drivers/gpu/drm/i915/gt/sysfs_gt.c | 79 +++++ > drivers/gpu/drm/i915/gt/sysfs_gt.h | 22 ++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 432 +++++++++++++++++++++++ > drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 17 + > drivers/gpu/drm/i915/i915_sysfs.c | 375 +------------------- > drivers/gpu/drm/i915/i915_sysfs.h | 3 + > 9 files changed, 561 insertions(+), 375 deletions(-) > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index b314d44ded5e..ff9e17c97dc2 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -107,7 +107,9 @@ gt-y += \ > gt/intel_rps.o \ > gt/intel_sseu.o \ > gt/intel_timeline.o \ > - gt/intel_workarounds.o > + gt/intel_workarounds.o \ > + gt/sysfs_gt.o \ > + gt/sysfs_gt_pm.o > # autogenerated null render state > gt-y += \ > gt/gen6_renderstate.o \ > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c > index f1f1b306e0af..e794d05d69a1 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt.c > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c > @@ -15,6 +15,7 @@ > #include "intel_rps.h" > #include "intel_uncore.h" > #include "intel_pm.h" > +#include "sysfs_gt.h" > > void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) > { > @@ -321,6 +322,7 @@ void intel_gt_driver_register(struct intel_gt *gt) > intel_rps_driver_register(>->rps); > > debugfs_gt_register(gt); > + intel_gt_sysfs_register(gt); > } > > static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size) > @@ -641,6 +643,7 @@ void intel_gt_driver_remove(struct intel_gt *gt) > > void intel_gt_driver_unregister(struct intel_gt *gt) > { > + intel_gt_sysfs_unregister(gt); > intel_rps_driver_unregister(>->rps); > } > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h > index 96890dd12b5f..7f0b4f8d9e28 100644 > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > @@ -32,6 +32,7 @@ struct intel_gt { > struct drm_i915_private *i915; > struct intel_uncore *uncore; > struct i915_ggtt *ggtt; > + struct kobject sysfs_root; > > struct intel_uc uc; > > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c b/drivers/gpu/drm/i915/gt/sysfs_gt.c > new file mode 100644 > index 000000000000..9335a92d5248 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c > @@ -0,0 +1,79 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <linux/sysfs.h> > +#include <drm/drm_device.h> > +#include <linux/kobject.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "intel_gt.h" > +#include "intel_gt_types.h" > +#include "intel_rc6.h" > + > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +static inline struct kobject *gt_get_parent_obj(struct intel_gt *gt) > +{ > + return >->i915->drm.primary->kdev->kobj; > +} > + > +static ssize_t gt_info_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + return snprintf(buff, PAGE_SIZE, "0\n"); > +} > + > +static DEVICE_ATTR_RO(gt_info); > + > +static struct kobj_type sysfs_gt_ktype = { > + .sysfs_ops = &kobj_sysfs_ops, > +}; > + > +void intel_gt_sysfs_register(struct intel_gt *gt) > +{ > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); > + int ret; > + > + ret = kobject_init_and_add(>->sysfs_root, > + &sysfs_gt_ktype, > + parent, "gt"); > + if (ret) { > + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); I'd perhaps pin point the failure more by s/file/GT sysfs root/. > + kobject_put(>->sysfs_root); Is the reference needed for the registration steps? I am thinking if you could kobject_get only once everything worked to simplify. > + goto parent_files; > + } > + > + ret = sysfs_create_file(>->sysfs_root, &dev_attr_gt_info.attr); > + if (ret) > + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); > + > + intel_gt_sysfs_pm_init(gt, >->sysfs_root); If you put this first you can avoid the goto I think which makes the function smaller. > + > +parent_files: > + /* > + * we need to make things right with the > + * ABI compatibility. The files were originally > + * generated under the parent directory. > + */ > + intel_gt_sysfs_pm_init(gt, parent); > +} > + > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > +{ > + struct kobject *parent = gt_get_parent_obj(gt); > + > + /* > + * the name gt tells us wether sysfs_root > + * object was initialized properly > + */ > + if (!strcmp(gt->sysfs_root.name, "gt")) > + kobject_put(>->sysfs_root); Slightly nicer would be looking at kobj->state_initialized for this check I think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double put on sucess which makes me ask whether holding a reference on parent is even needed? It can't go away so perhaps it isn't. > + > + kobject_put(parent); > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.h b/drivers/gpu/drm/i915/gt/sysfs_gt.h > new file mode 100644 > index 000000000000..2e479aa902e5 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h > @@ -0,0 +1,22 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_GT_H > +#define SYSFS_GT_H > + > +#include "intel_gt_types.h" > + > +struct intel_gt; > + > +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj) > +{ > + return container_of(kobj, struct intel_gt, sysfs_root); > +} > + > +void intel_gt_sysfs_register(struct intel_gt *gt); > +void intel_gt_sysfs_unregister(struct intel_gt *gt); > + > +#endif /* SYSFS_GT_H */ > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > new file mode 100644 > index 000000000000..c548eb851a70 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > @@ -0,0 +1,432 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <drm/drm_device.h> > +#include <linux/sysfs.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "../i915_sysfs.h" > +#include "intel_gt.h" > +#include "intel_rc6.h" > +#include "intel_rps.h" > +#include "sysfs_gt.h" > +#include "sysfs_gt_pm.h" > + > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > +{ > + struct kobject *kobj = &dev->kobj; > + /* > + * We are interested at knowing from where the interface > + * has been called, whether it's called from gt/ or from > + * the parent directory. > + * From the interface position it depends also the value of > + * the private data. > + * If the interface is called from gt/ then private data is > + * of the "struct intel_gt *" type, otherwise it's * a > + * "struct drm_i915_private *" type. > + */ > + if (strcmp(dev->kobj.name, "gt")) { > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > + > + pr_warn_ratelimited(DEPRECATED > + "(%s, %d) trying to access deprecated interface, " > + "use the corresponding interface in gt/\n", Saying interface two times sounds a bit suboptimal but I leave this to native speakers to improve. Can you get to the name of the sysfs control here? "(%s, %d) is trying to access deprecated '%s' sysfs control. Please use 'gt/%s' instead.". Something like that? Regards, Tvrtko > + current->comm, task_pid_nr(current)); > + return &i915->gt; > + } > + > + return kobj_to_gt(kobj); > +} > + > +#ifdef CONFIG_PM > +static u32 get_residency(struct intel_gt *gt, i915_reg_t reg) > +{ > + intel_wakeref_t wakeref; > + u64 res = 0; > + > + with_intel_runtime_pm(gt->uncore->rpm, wakeref) > + res = intel_rc6_residency_us(>->rc6, reg); > + > + return DIV_ROUND_CLOSEST_ULL(res, 1000); > +} > + > +static ssize_t rc6_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u8 mask = 0; > + > + if (HAS_RC6(gt->i915)) > + mask |= BIT(0); > + if (HAS_RC6p(gt->i915)) > + mask |= BIT(1); > + if (HAS_RC6pp(gt->i915)) > + mask |= BIT(2); > + > + return snprintf(buff, PAGE_SIZE, "%x\n", mask); > +} > + > +static ssize_t rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static ssize_t rc6p_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency); > +} > + > +static ssize_t rc6pp_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency); > +} > + > +static ssize_t media_rc6_residency_ms_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6); > + > + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); > +} > + > +static DEVICE_ATTR_RO(rc6_enable); > +static DEVICE_ATTR_RO(rc6_residency_ms); > +static DEVICE_ATTR_RO(rc6p_residency_ms); > +static DEVICE_ATTR_RO(rc6pp_residency_ms); > +static DEVICE_ATTR_RO(media_rc6_residency_ms); > + > +static const struct attribute *rc6_attrs[] = { > + &dev_attr_rc6_enable.attr, > + &dev_attr_rc6_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *rc6p_attrs[] = { > + &dev_attr_rc6p_residency_ms.attr, > + &dev_attr_rc6pp_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute *media_rc6_attrs[] = { > + &dev_attr_media_rc6_residency_ms.attr, > + NULL > +}; > + > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret = 0; > + > + if (HAS_RC6(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6 sysfs files\n"); > + } > + > + if (HAS_RC6p(gt->i915)) { > + ret = sysfs_create_files(kobj, rc6p_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create RC6p sysfs files\n"); > + } > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { > + ret = sysfs_create_files(kobj, media_rc6_attrs); > + if (ret) > + drm_err(>->i915->drm, > + "failed to create media RC6 sysfs files\n"); > + } > +} > +#else > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + return 0; > +} > +#endif /* CONFIG_PM */ > + > +static ssize_t gt_act_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_rps_read_actual_frequency(>->rps)); > +} > + > +static ssize_t gt_cur_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->cur_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->boost_freq)); > +} > + > +static ssize_t gt_boost_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + bool boost = false; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + /* 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 count; > +} > + > +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->efficient_freq)); > +} > + > +static ssize_t gt_max_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->max_freq_softlimit)); > +} > + > +static ssize_t gt_max_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val < rps->min_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + if (val > rps->rp0_freq) > + DRM_DEBUG("User requested overclocking to %d\n", > + intel_gpu_freq(rps, val)); > + > + rps->max_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new max_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + return ret ?: count; > +} > + > +static ssize_t gt_min_freq_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + > + return snprintf(buff, PAGE_SIZE, "%d\n", > + intel_gpu_freq(rps, rps->min_freq_softlimit)); > +} > + > +static ssize_t gt_min_freq_mhz_store(struct device *dev, > + struct device_attribute *attr, > + const char *buff, size_t count) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + ssize_t ret; > + u32 val; > + > + ret = kstrtou32(buff, 0, &val); > + if (ret) > + return ret; > + > + mutex_lock(&rps->lock); > + > + val = intel_freq_opcode(rps, val); > + if (val < rps->min_freq || > + val > rps->max_freq || > + val > rps->max_freq_softlimit) { > + ret = -EINVAL; > + goto unlock; > + } > + > + rps->min_freq_softlimit = val; > + > + val = clamp_t(int, rps->cur_freq, > + rps->min_freq_softlimit, > + rps->max_freq_softlimit); > + > + /* > + * We still need *_set_rps to process the new min_delay and > + * update the interrupt limits and PMINTRMSK even though > + * frequency request may be unchanged. > + */ > + intel_rps_set(rps, val); > + > +unlock: > + mutex_unlock(&rps->lock); > + > + 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 *dev, > + struct device_attribute *attr, > + char *buff); > + > +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); > +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); > + > +/* For now we have a static number of RP states */ > +static ssize_t gt_rp_mhz_show(struct device *dev, > + struct device_attribute *attr, > + char *buff) > +{ > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); > + struct intel_rps *rps = >->rps; > + u32 val; > + > + if (attr == &dev_attr_gt_RP0_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp0_freq); > + else if (attr == &dev_attr_gt_RP1_freq_mhz) > + val = intel_gpu_freq(rps, rps->rp1_freq); > + else if (attr == &dev_attr_gt_RPn_freq_mhz) > + val = intel_gpu_freq(rps, rps->min_freq); > + else > + BUG(); > + > + return snprintf(buff, PAGE_SIZE, "%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, > +}; > + > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + return sysfs_create_files(kobj, vlv_attrs); > + > + if (INTEL_GEN(gt->i915) >= 6) > + return sysfs_create_files(kobj, gen6_attrs); > + > + return 0; > +} > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret; > + > + intel_sysfs_rc6_init(gt, kobj); > + > + ret = intel_sysfs_rps_init(gt, kobj); > + if (ret) > + drm_err(>->i915->drm, "failed to create RPS sysfs files"); > +} > diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > new file mode 100644 > index 000000000000..758d0c3cb998 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_RC6_H > +#define SYSFS_RC6_H > + > +#include <linux/kobject.h> > + > +#include "intel_gt_types.h" > + > +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj); > +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj); > + > +#endif /* SYSFS_RC6_H */ > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index c14d762bd652..1298977fc08b 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -38,113 +38,12 @@ > #include "intel_pm.h" > #include "intel_sideband.h" > > -static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) > { > struct drm_minor *minor = dev_get_drvdata(kdev); > return to_i915(minor->dev); > } > > -#ifdef CONFIG_PM > -static u32 calc_residency(struct drm_i915_private *dev_priv, > - i915_reg_t reg) > -{ > - intel_wakeref_t wakeref; > - u64 res = 0; > - > - with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) > - res = intel_rc6_residency_us(&dev_priv->gt.rc6, reg); > - > - return DIV_ROUND_CLOSEST_ULL(res, 1000); > -} > - > -static ssize_t > -show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - unsigned int mask; > - > - mask = 0; > - if (HAS_RC6(dev_priv)) > - mask |= BIT(0); > - if (HAS_RC6p(dev_priv)) > - mask |= BIT(1); > - if (HAS_RC6pp(dev_priv)) > - mask |= BIT(2); > - > - return snprintf(buf, PAGE_SIZE, "%x\n", mask); > -} > - > -static ssize_t > -show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static ssize_t > -show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6p_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6p); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency); > -} > - > -static ssize_t > -show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6pp_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6pp); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency); > -} > - > -static ssize_t > -show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) > -{ > - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); > - u32 rc6_residency = calc_residency(dev_priv, VLV_GT_MEDIA_RC6); > - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); > -} > - > -static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL); > -static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL); > -static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL); > -static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); > -static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO, show_media_rc6_ms, NULL); > - > -static struct attribute *rc6_attrs[] = { > - &dev_attr_rc6_enable.attr, > - &dev_attr_rc6_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group rc6_attr_group = { > - .name = power_group_name, > - .attrs = rc6_attrs > -}; > - > -static struct attribute *rc6p_attrs[] = { > - &dev_attr_rc6p_residency_ms.attr, > - &dev_attr_rc6pp_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group rc6p_attr_group = { > - .name = power_group_name, > - .attrs = rc6p_attrs > -}; > - > -static struct attribute *media_rc6_attrs[] = { > - &dev_attr_media_rc6_residency_ms.attr, > - NULL > -}; > - > -static const struct attribute_group media_rc6_attr_group = { > - .name = power_group_name, > - .attrs = media_rc6_attrs > -}; > -#endif > - > static int l3_access_valid(struct drm_i915_private *i915, loff_t offset) > { > if (!HAS_L3_DPF(i915)) > @@ -256,239 +155,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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->cur_freq)); > -} > - > -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 = &i915->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->boost_freq)); > -} > - > -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 = &dev_priv->gt.rps; > - bool boost = false; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - /* 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 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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%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_rps *rps = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->max_freq_softlimit)); > -} > - > -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_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val < rps->min_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - if (val > rps->rp0_freq) > - DRM_DEBUG("User requested overclocking to %d\n", > - intel_gpu_freq(rps, val)); > - > - rps->max_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new max_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - return ret ?: count; > -} > - > -static ssize_t gt_min_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 = &dev_priv->gt.rps; > - > - return snprintf(buf, PAGE_SIZE, "%d\n", > - intel_gpu_freq(rps, rps->min_freq_softlimit)); > -} > - > -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 *dev_priv = kdev_minor_to_i915(kdev); > - struct intel_rps *rps = &dev_priv->gt.rps; > - ssize_t ret; > - u32 val; > - > - ret = kstrtou32(buf, 0, &val); > - if (ret) > - return ret; > - > - mutex_lock(&rps->lock); > - > - val = intel_freq_opcode(rps, val); > - if (val < rps->min_freq || > - val > rps->max_freq || > - val > rps->max_freq_softlimit) { > - ret = -EINVAL; > - goto unlock; > - } > - > - rps->min_freq_softlimit = val; > - > - val = clamp_t(int, rps->cur_freq, > - rps->min_freq_softlimit, > - rps->max_freq_softlimit); > - > - /* > - * We still need *_set_rps to process the new min_delay and > - * update the interrupt limits and PMINTRMSK even though > - * frequency request may be unchanged. > - */ > - intel_rps_set(rps, val); > - > -unlock: > - mutex_unlock(&rps->lock); > - > - 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 = &dev_priv->gt.rps; > - u32 val; > - > - if (attr == &dev_attr_gt_RP0_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp0_freq); > - else if (attr == &dev_attr_gt_RP1_freq_mhz) > - val = intel_gpu_freq(rps, rps->rp1_freq); > - else if (attr == &dev_attr_gt_RPn_freq_mhz) > - val = intel_gpu_freq(rps, rps->min_freq); > - else > - BUG(); > - > - return snprintf(buf, PAGE_SIZE, "%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, > @@ -559,29 +225,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) > struct device *kdev = dev_priv->drm.primary->kdev; > int ret; > > -#ifdef CONFIG_PM > - if (HAS_RC6(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6 residency sysfs setup failed\n"); > - } > - if (HAS_RC6p(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &rc6p_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "RC6p residency sysfs setup failed\n"); > - } > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - ret = sysfs_merge_group(&kdev->kobj, > - &media_rc6_attr_group); > - if (ret) > - drm_err(&dev_priv->drm, > - "Media RC6 residency sysfs setup failed\n"); > - } > -#endif > if (HAS_L3_DPF(dev_priv)) { > ret = device_create_bin_file(kdev, &dpf_attrs); > if (ret) > @@ -597,14 +240,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 (INTEL_GEN(dev_priv) >= 6) > - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); > - if (ret) > - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); > - > i915_setup_error_capture(kdev); > } > > @@ -614,14 +249,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); > -#ifdef CONFIG_PM > - sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group); > - sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group); > -#endif > } > diff --git a/drivers/gpu/drm/i915/i915_sysfs.h b/drivers/gpu/drm/i915/i915_sysfs.h > index 41afd4366416..ad6114de81c9 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.h > +++ b/drivers/gpu/drm/i915/i915_sysfs.h > @@ -6,8 +6,11 @@ > #ifndef __I915_SYSFS_H__ > #define __I915_SYSFS_H__ > > +#include <linux/device.h> > + > struct drm_i915_private; > > +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev); > void i915_setup_sysfs(struct drm_i915_private *i915); > void i915_teardown_sysfs(struct drm_i915_private *i915); > >
> > +void intel_gt_sysfs_register(struct intel_gt *gt) > > +{ > > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); > > + int ret; > > + > > + ret = kobject_init_and_add(>->sysfs_root, > > + &sysfs_gt_ktype, > > + parent, "gt"); > > + if (ret) { > > + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); > > I'd perhaps pin point the failure more by s/file/GT sysfs root/. OK > > + kobject_put(>->sysfs_root); > > Is the reference needed for the registration steps? I am thinking if you > could kobject_get only once everything worked to simplify. I haven't really understood what you mean here. Are you saying that kobject_put not needed? in the lib/kobject.c it says as comment to kobject_init_and_add(): " * If this function returns an error, kobject_put() must be called to * properly clean up the memory associated with the object. This is the * same type of error handling after a call to kobject_add() and kobject * lifetime rules are the same here. */ " > > + ret = sysfs_create_file(>->sysfs_root, &dev_attr_gt_info.attr); > > + if (ret) > > + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); > > + > > + intel_gt_sysfs_pm_init(gt, >->sysfs_root); > > If you put this first you can avoid the goto I think which makes the > function smaller. True! > > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > > +{ > > + struct kobject *parent = gt_get_parent_obj(gt); > > + > > + /* > > + * the name gt tells us wether sysfs_root > > + * object was initialized properly > > + */ > > + if (!strcmp(gt->sysfs_root.name, "gt")) > > + kobject_put(>->sysfs_root); > > Slightly nicer would be looking at kobj->state_initialized for this check I > think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double > put on sucess which makes me ask whether holding a reference on parent is > even needed? It can't go away so perhaps it isn't. I'd rather use the state_initialized, even though I don't trust its value if the kobject has failed to initialise earlier, I trust it only if it's '1', maybe I'm paranoic. > > + /* > > + * We are interested at knowing from where the interface > > + * has been called, whether it's called from gt/ or from > > + * the parent directory. > > + * From the interface position it depends also the value of > > + * the private data. > > + * If the interface is called from gt/ then private data is > > + * of the "struct intel_gt *" type, otherwise it's * a > > + * "struct drm_i915_private *" type. > > + */ > > + if (strcmp(dev->kobj.name, "gt")) { > > + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); > > + > > + pr_warn_ratelimited(DEPRECATED > > + "(%s, %d) trying to access deprecated interface, " > > + "use the corresponding interface in gt/\n", > > Saying interface two times sounds a bit suboptimal but I leave this to > native speakers to improve. > > Can you get to the name of the sysfs control here? sure. > "(%s, %d) is trying to access deprecated '%s' sysfs control. Please use > 'gt/%s' instead.". Something like that? yes... it's not always easy to write logs when you have to stay within the 80 characters Thanks for the review! Andi
On 24/02/2020 16:30, Andi Shyti wrote: >>> +void intel_gt_sysfs_register(struct intel_gt *gt) >>> +{ >>> + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); >>> + int ret; >>> + >>> + ret = kobject_init_and_add(>->sysfs_root, >>> + &sysfs_gt_ktype, >>> + parent, "gt"); >>> + if (ret) { >>> + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); >> >> I'd perhaps pin point the failure more by s/file/GT sysfs root/. > > OK > >>> + kobject_put(>->sysfs_root); >> >> Is the reference needed for the registration steps? I am thinking if you >> could kobject_get only once everything worked to simplify. > > I haven't really understood what you mean here. Are you saying > that kobject_put not needed? in the lib/kobject.c it says as > comment to kobject_init_and_add(): > > " > * If this function returns an error, kobject_put() must be called to > * properly clean up the memory associated with the object. This is the > * same type of error handling after a call to kobject_add() and kobject > * lifetime rules are the same here. > */ > " My mistake, I confused the two objects. >>> + ret = sysfs_create_file(>->sysfs_root, &dev_attr_gt_info.attr); >>> + if (ret) >>> + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); >>> + >>> + intel_gt_sysfs_pm_init(gt, >->sysfs_root); >> >> If you put this first you can avoid the goto I think which makes the >> function smaller. > > True! > >>> +void intel_gt_sysfs_unregister(struct intel_gt *gt) >>> +{ >>> + struct kobject *parent = gt_get_parent_obj(gt); >>> + >>> + /* >>> + * the name gt tells us wether sysfs_root >>> + * object was initialized properly >>> + */ >>> + if (!strcmp(gt->sysfs_root.name, "gt")) >>> + kobject_put(>->sysfs_root); >> >> Slightly nicer would be looking at kobj->state_initialized for this check I >> think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double >> put on sucess which makes me ask whether holding a reference on parent is >> even needed? It can't go away so perhaps it isn't. > > I'd rather use the state_initialized, even though I don't trust > its value if the kobject has failed to initialise earlier, I > trust it only if it's '1', maybe I'm paranoic. But is the reference even needed? Regards, Tvrtko
> > > > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > > > > +{ > > > > + struct kobject *parent = gt_get_parent_obj(gt); > > > > + > > > > + /* > > > > + * the name gt tells us wether sysfs_root > > > > + * object was initialized properly > > > > + */ > > > > + if (!strcmp(gt->sysfs_root.name, "gt")) > > > > + kobject_put(>->sysfs_root); > > > > > > Slightly nicer would be looking at kobj->state_initialized for this check I > > > think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double > > > put on sucess which makes me ask whether holding a reference on parent is > > > even needed? It can't go away so perhaps it isn't. > > > > I'd rather use the state_initialized, even though I don't trust > > its value if the kobject has failed to initialise earlier, I > > trust it only if it's '1', maybe I'm paranoic. > > But is the reference even needed? yes, because I _get it here (i.e. above, during initialization): > > > > +void intel_gt_sysfs_register(struct intel_gt *gt) > > > > +{ > > > > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); > > > > + int ret; > > > > + and if I need to call kobject_put at the end. If for some reason the files have failed to be initialized, I would have an unbalanced put and a warning would be printed. I'll summarize in pseudo code: intel_gt_sysfs_register() { kobject_init_and_add(sysfs_root...); /* which calls kobject_get() inside */ if (fails) kobject_put(sysfs_root); /* reference goes to '0' */ } intel_gt_sysfs_unregister() { option1: I don't call kobject_put(), I have an unbalanced situation as you reviewed in patch 1. option2: I call kobject_put(), if it did fail during init there is an unbalanced situation, which is handled but an annoying WARN() is issued. option3: I check if "state_initialized" which I suppose has been properly initialised during declaration (maybe too paranoic?) and call _put() accordingly } Thanks, Andi
On 25/02/2020 01:35, Andi Shyti wrote: >>>>> +void intel_gt_sysfs_unregister(struct intel_gt *gt) >>>>> +{ >>>>> + struct kobject *parent = gt_get_parent_obj(gt); >>>>> + >>>>> + /* >>>>> + * the name gt tells us wether sysfs_root >>>>> + * object was initialized properly >>>>> + */ >>>>> + if (!strcmp(gt->sysfs_root.name, "gt")) >>>>> + kobject_put(>->sysfs_root); >>>> >>>> Slightly nicer would be looking at kobj->state_initialized for this check I >>>> think. Or even kref_get_unless_zero on kobj->kref? Ugliness there is double >>>> put on sucess which makes me ask whether holding a reference on parent is >>>> even needed? It can't go away so perhaps it isn't. >>> >>> I'd rather use the state_initialized, even though I don't trust >>> its value if the kobject has failed to initialise earlier, I >>> trust it only if it's '1', maybe I'm paranoic. >> >> But is the reference even needed? > > yes, because I _get it here (i.e. above, during initialization): > >>>>> +void intel_gt_sysfs_register(struct intel_gt *gt) >>>>> +{ >>>>> + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); >>>>> + int ret; >>>>> + > > and if I need to call kobject_put at the end. If for some reason > the files have failed to be initialized, I would have an > unbalanced put and a warning would be printed. > > I'll summarize in pseudo code: > > intel_gt_sysfs_register() > { > kobject_init_and_add(sysfs_root...); /* which calls kobject_get() inside */ > if (fails) > kobject_put(sysfs_root); /* reference goes to '0' */ > } > > intel_gt_sysfs_unregister() > { > option1: I don't call kobject_put(), I have an unbalanced > situation as you reviewed in patch 1. > > option2: I call kobject_put(), if it did fail during init > there is an unbalanced situation, which is > handled but an annoying WARN() is issued. > > option3: I check if "state_initialized" which I suppose > has been properly initialised during declaration > (maybe too paranoic?) and call _put() > accordingly > } Yes you are right, I confused the two parents again. :I Okay then, is the extra kobject_get/put on the parent (kobject_get(gt_get_parent_obj(gt) - this one) needed? Regards, Tvrtko
> > > > > > +void intel_gt_sysfs_register(struct intel_gt *gt) > > > > > > +{ > > > > > > + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); > > > > > > + int ret; > > > > > > + > > > > and if I need to call kobject_put at the end. If for some reason > > the files have failed to be initialized, I would have an > > unbalanced put and a warning would be printed. > > > > I'll summarize in pseudo code: > > > > intel_gt_sysfs_register() > > { > > kobject_init_and_add(sysfs_root...); /* which calls kobject_get() inside */ > > if (fails) > > kobject_put(sysfs_root); /* reference goes to '0' */ > > } > > > > intel_gt_sysfs_unregister() > > { > > option1: I don't call kobject_put(), I have an unbalanced > > situation as you reviewed in patch 1. > > > > option2: I call kobject_put(), if it did fail during init > > there is an unbalanced situation, which is > > handled but an annoying WARN() is issued. > > > > option3: I check if "state_initialized" which I suppose > > has been properly initialised during declaration > > (maybe too paranoic?) and call _put() > > accordingly > > } > > Yes you are right, I confused the two parents again. :I this little things are good for jambling the brain up :) > Okay then, is the extra kobject_get/put on the parent > (kobject_get(gt_get_parent_obj(gt) - this one) needed? I do not see any strong reason for calling kobject_get, I do it only because I am creating files in there and I don't want anyone to free those without my permission. Otherwise, the creation and the destruction of the son object would take care of the refcount. Will it ever happen that parent will be destroyed before I have time to release the files? I don't think so, but I do it more for the form than for the use. In the sense that "I have stuff in there and I declare I have stuff in there and I declare when I won't need them anymore". Andi
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index b314d44ded5e..ff9e17c97dc2 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -107,7 +107,9 @@ gt-y += \ gt/intel_rps.o \ gt/intel_sseu.o \ gt/intel_timeline.o \ - gt/intel_workarounds.o + gt/intel_workarounds.o \ + gt/sysfs_gt.o \ + gt/sysfs_gt_pm.o # autogenerated null render state gt-y += \ gt/gen6_renderstate.o \ diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f1f1b306e0af..e794d05d69a1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -15,6 +15,7 @@ #include "intel_rps.h" #include "intel_uncore.h" #include "intel_pm.h" +#include "sysfs_gt.h" void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) { @@ -321,6 +322,7 @@ void intel_gt_driver_register(struct intel_gt *gt) intel_rps_driver_register(>->rps); debugfs_gt_register(gt); + intel_gt_sysfs_register(gt); } static int intel_gt_init_scratch(struct intel_gt *gt, unsigned int size) @@ -641,6 +643,7 @@ void intel_gt_driver_remove(struct intel_gt *gt) void intel_gt_driver_unregister(struct intel_gt *gt) { + intel_gt_sysfs_unregister(gt); intel_rps_driver_unregister(>->rps); } diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 96890dd12b5f..7f0b4f8d9e28 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -32,6 +32,7 @@ struct intel_gt { struct drm_i915_private *i915; struct intel_uncore *uncore; struct i915_ggtt *ggtt; + struct kobject sysfs_root; struct intel_uc uc; diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.c b/drivers/gpu/drm/i915/gt/sysfs_gt.c new file mode 100644 index 000000000000..9335a92d5248 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: MIT + +/* + * Copyright © 2019 Intel Corporation + */ + +#include <linux/sysfs.h> +#include <drm/drm_device.h> +#include <linux/kobject.h> +#include <linux/printk.h> + +#include "../i915_drv.h" +#include "intel_gt.h" +#include "intel_gt_types.h" +#include "intel_rc6.h" + +#include "sysfs_gt.h" +#include "sysfs_gt_pm.h" + +static inline struct kobject *gt_get_parent_obj(struct intel_gt *gt) +{ + return >->i915->drm.primary->kdev->kobj; +} + +static ssize_t gt_info_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + return snprintf(buff, PAGE_SIZE, "0\n"); +} + +static DEVICE_ATTR_RO(gt_info); + +static struct kobj_type sysfs_gt_ktype = { + .sysfs_ops = &kobj_sysfs_ops, +}; + +void intel_gt_sysfs_register(struct intel_gt *gt) +{ + struct kobject *parent = kobject_get(gt_get_parent_obj(gt)); + int ret; + + ret = kobject_init_and_add(>->sysfs_root, + &sysfs_gt_ktype, + parent, "gt"); + if (ret) { + drm_err(>->i915->drm, "failed to initialize sysfs file\n"); + kobject_put(>->sysfs_root); + goto parent_files; + } + + ret = sysfs_create_file(>->sysfs_root, &dev_attr_gt_info.attr); + if (ret) + drm_err(>->i915->drm, "failed to create sysfs gt info files\n"); + + intel_gt_sysfs_pm_init(gt, >->sysfs_root); + +parent_files: + /* + * we need to make things right with the + * ABI compatibility. The files were originally + * generated under the parent directory. + */ + intel_gt_sysfs_pm_init(gt, parent); +} + +void intel_gt_sysfs_unregister(struct intel_gt *gt) +{ + struct kobject *parent = gt_get_parent_obj(gt); + + /* + * the name gt tells us wether sysfs_root + * object was initialized properly + */ + if (!strcmp(gt->sysfs_root.name, "gt")) + kobject_put(>->sysfs_root); + + kobject_put(parent); +} diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt.h b/drivers/gpu/drm/i915/gt/sysfs_gt.h new file mode 100644 index 000000000000..2e479aa902e5 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef SYSFS_GT_H +#define SYSFS_GT_H + +#include "intel_gt_types.h" + +struct intel_gt; + +static inline struct intel_gt *kobj_to_gt(struct kobject *kobj) +{ + return container_of(kobj, struct intel_gt, sysfs_root); +} + +void intel_gt_sysfs_register(struct intel_gt *gt); +void intel_gt_sysfs_unregister(struct intel_gt *gt); + +#endif /* SYSFS_GT_H */ diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c new file mode 100644 index 000000000000..c548eb851a70 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c @@ -0,0 +1,432 @@ +// SPDX-License-Identifier: MIT + +/* + * Copyright © 2019 Intel Corporation + */ + +#include <drm/drm_device.h> +#include <linux/sysfs.h> +#include <linux/printk.h> + +#include "../i915_drv.h" +#include "../i915_sysfs.h" +#include "intel_gt.h" +#include "intel_rc6.h" +#include "intel_rps.h" +#include "sysfs_gt.h" +#include "sysfs_gt_pm.h" + +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) +{ + struct kobject *kobj = &dev->kobj; + /* + * We are interested at knowing from where the interface + * has been called, whether it's called from gt/ or from + * the parent directory. + * From the interface position it depends also the value of + * the private data. + * If the interface is called from gt/ then private data is + * of the "struct intel_gt *" type, otherwise it's * a + * "struct drm_i915_private *" type. + */ + if (strcmp(dev->kobj.name, "gt")) { + struct drm_i915_private *i915 = kdev_minor_to_i915(dev); + + pr_warn_ratelimited(DEPRECATED + "(%s, %d) trying to access deprecated interface, " + "use the corresponding interface in gt/\n", + current->comm, task_pid_nr(current)); + return &i915->gt; + } + + return kobj_to_gt(kobj); +} + +#ifdef CONFIG_PM +static u32 get_residency(struct intel_gt *gt, i915_reg_t reg) +{ + intel_wakeref_t wakeref; + u64 res = 0; + + with_intel_runtime_pm(gt->uncore->rpm, wakeref) + res = intel_rc6_residency_us(>->rc6, reg); + + return DIV_ROUND_CLOSEST_ULL(res, 1000); +} + +static ssize_t rc6_enable_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u8 mask = 0; + + if (HAS_RC6(gt->i915)) + mask |= BIT(0); + if (HAS_RC6p(gt->i915)) + mask |= BIT(1); + if (HAS_RC6pp(gt->i915)) + mask |= BIT(2); + + return snprintf(buff, PAGE_SIZE, "%x\n", mask); +} + +static ssize_t rc6_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); +} + +static ssize_t rc6p_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency); +} + +static ssize_t rc6pp_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency); +} + +static ssize_t media_rc6_residency_ms_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6); + + return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency); +} + +static DEVICE_ATTR_RO(rc6_enable); +static DEVICE_ATTR_RO(rc6_residency_ms); +static DEVICE_ATTR_RO(rc6p_residency_ms); +static DEVICE_ATTR_RO(rc6pp_residency_ms); +static DEVICE_ATTR_RO(media_rc6_residency_ms); + +static const struct attribute *rc6_attrs[] = { + &dev_attr_rc6_enable.attr, + &dev_attr_rc6_residency_ms.attr, + NULL +}; + +static const struct attribute *rc6p_attrs[] = { + &dev_attr_rc6p_residency_ms.attr, + &dev_attr_rc6pp_residency_ms.attr, + NULL +}; + +static const struct attribute *media_rc6_attrs[] = { + &dev_attr_media_rc6_residency_ms.attr, + NULL +}; + +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) +{ + int ret = 0; + + if (HAS_RC6(gt->i915)) { + ret = sysfs_create_files(kobj, rc6_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create RC6 sysfs files\n"); + } + + if (HAS_RC6p(gt->i915)) { + ret = sysfs_create_files(kobj, rc6p_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create RC6p sysfs files\n"); + } + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { + ret = sysfs_create_files(kobj, media_rc6_attrs); + if (ret) + drm_err(>->i915->drm, + "failed to create media RC6 sysfs files\n"); + } +} +#else +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) +{ + return 0; +} +#endif /* CONFIG_PM */ + +static ssize_t gt_act_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_rps_read_actual_frequency(>->rps)); +} + +static ssize_t gt_cur_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->cur_freq)); +} + +static ssize_t gt_boost_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->boost_freq)); +} + +static ssize_t gt_boost_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + bool boost = false; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + /* 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 count; +} + +static ssize_t vlv_rpe_freq_mhz_show(struct device *dev, + struct device_attribute *attr, char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->efficient_freq)); +} + +static ssize_t gt_max_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->max_freq_softlimit)); +} + +static ssize_t gt_max_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + mutex_lock(&rps->lock); + + val = intel_freq_opcode(rps, val); + if (val < rps->min_freq || + val > rps->max_freq || + val < rps->min_freq_softlimit) { + ret = -EINVAL; + goto unlock; + } + + if (val > rps->rp0_freq) + DRM_DEBUG("User requested overclocking to %d\n", + intel_gpu_freq(rps, val)); + + rps->max_freq_softlimit = val; + + val = clamp_t(int, rps->cur_freq, + rps->min_freq_softlimit, + rps->max_freq_softlimit); + + /* + * We still need *_set_rps to process the new max_delay and + * update the interrupt limits and PMINTRMSK even though + * frequency request may be unchanged. + */ + intel_rps_set(rps, val); + +unlock: + mutex_unlock(&rps->lock); + + return ret ?: count; +} + +static ssize_t gt_min_freq_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + + return snprintf(buff, PAGE_SIZE, "%d\n", + intel_gpu_freq(rps, rps->min_freq_softlimit)); +} + +static ssize_t gt_min_freq_mhz_store(struct device *dev, + struct device_attribute *attr, + const char *buff, size_t count) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + ssize_t ret; + u32 val; + + ret = kstrtou32(buff, 0, &val); + if (ret) + return ret; + + mutex_lock(&rps->lock); + + val = intel_freq_opcode(rps, val); + if (val < rps->min_freq || + val > rps->max_freq || + val > rps->max_freq_softlimit) { + ret = -EINVAL; + goto unlock; + } + + rps->min_freq_softlimit = val; + + val = clamp_t(int, rps->cur_freq, + rps->min_freq_softlimit, + rps->max_freq_softlimit); + + /* + * We still need *_set_rps to process the new min_delay and + * update the interrupt limits and PMINTRMSK even though + * frequency request may be unchanged. + */ + intel_rps_set(rps, val); + +unlock: + mutex_unlock(&rps->lock); + + 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 *dev, + struct device_attribute *attr, + char *buff); + +static DEVICE_ATTR(gt_RP0_freq_mhz, 0444, gt_rp_mhz_show, NULL); +static DEVICE_ATTR(gt_RP1_freq_mhz, 0444, gt_rp_mhz_show, NULL); +static DEVICE_ATTR(gt_RPn_freq_mhz, 0444, gt_rp_mhz_show, NULL); + +/* For now we have a static number of RP states */ +static ssize_t gt_rp_mhz_show(struct device *dev, + struct device_attribute *attr, + char *buff) +{ + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev); + struct intel_rps *rps = >->rps; + u32 val; + + if (attr == &dev_attr_gt_RP0_freq_mhz) + val = intel_gpu_freq(rps, rps->rp0_freq); + else if (attr == &dev_attr_gt_RP1_freq_mhz) + val = intel_gpu_freq(rps, rps->rp1_freq); + else if (attr == &dev_attr_gt_RPn_freq_mhz) + val = intel_gpu_freq(rps, rps->min_freq); + else + BUG(); + + return snprintf(buff, PAGE_SIZE, "%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, +}; + +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj) +{ + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + return sysfs_create_files(kobj, vlv_attrs); + + if (INTEL_GEN(gt->i915) >= 6) + return sysfs_create_files(kobj, gen6_attrs); + + return 0; +} + +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj) +{ + int ret; + + intel_sysfs_rc6_init(gt, kobj); + + ret = intel_sysfs_rps_init(gt, kobj); + if (ret) + drm_err(>->i915->drm, "failed to create RPS sysfs files"); +} diff --git a/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h new file mode 100644 index 000000000000..758d0c3cb998 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef SYSFS_RC6_H +#define SYSFS_RC6_H + +#include <linux/kobject.h> + +#include "intel_gt_types.h" + +void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj); +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj); + +#endif /* SYSFS_RC6_H */ diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index c14d762bd652..1298977fc08b 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -38,113 +38,12 @@ #include "intel_pm.h" #include "intel_sideband.h" -static inline struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev) { struct drm_minor *minor = dev_get_drvdata(kdev); return to_i915(minor->dev); } -#ifdef CONFIG_PM -static u32 calc_residency(struct drm_i915_private *dev_priv, - i915_reg_t reg) -{ - intel_wakeref_t wakeref; - u64 res = 0; - - with_intel_runtime_pm(&dev_priv->runtime_pm, wakeref) - res = intel_rc6_residency_us(&dev_priv->gt.rc6, reg); - - return DIV_ROUND_CLOSEST_ULL(res, 1000); -} - -static ssize_t -show_rc6_mask(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - unsigned int mask; - - mask = 0; - if (HAS_RC6(dev_priv)) - mask |= BIT(0); - if (HAS_RC6p(dev_priv)) - mask |= BIT(1); - if (HAS_RC6pp(dev_priv)) - mask |= BIT(2); - - return snprintf(buf, PAGE_SIZE, "%x\n", mask); -} - -static ssize_t -show_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); -} - -static ssize_t -show_rc6p_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6p_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6p); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6p_residency); -} - -static ssize_t -show_rc6pp_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6pp_residency = calc_residency(dev_priv, GEN6_GT_GFX_RC6pp); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6pp_residency); -} - -static ssize_t -show_media_rc6_ms(struct device *kdev, struct device_attribute *attr, char *buf) -{ - struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev); - u32 rc6_residency = calc_residency(dev_priv, VLV_GT_MEDIA_RC6); - return snprintf(buf, PAGE_SIZE, "%u\n", rc6_residency); -} - -static DEVICE_ATTR(rc6_enable, S_IRUGO, show_rc6_mask, NULL); -static DEVICE_ATTR(rc6_residency_ms, S_IRUGO, show_rc6_ms, NULL); -static DEVICE_ATTR(rc6p_residency_ms, S_IRUGO, show_rc6p_ms, NULL); -static DEVICE_ATTR(rc6pp_residency_ms, S_IRUGO, show_rc6pp_ms, NULL); -static DEVICE_ATTR(media_rc6_residency_ms, S_IRUGO, show_media_rc6_ms, NULL); - -static struct attribute *rc6_attrs[] = { - &dev_attr_rc6_enable.attr, - &dev_attr_rc6_residency_ms.attr, - NULL -}; - -static const struct attribute_group rc6_attr_group = { - .name = power_group_name, - .attrs = rc6_attrs -}; - -static struct attribute *rc6p_attrs[] = { - &dev_attr_rc6p_residency_ms.attr, - &dev_attr_rc6pp_residency_ms.attr, - NULL -}; - -static const struct attribute_group rc6p_attr_group = { - .name = power_group_name, - .attrs = rc6p_attrs -}; - -static struct attribute *media_rc6_attrs[] = { - &dev_attr_media_rc6_residency_ms.attr, - NULL -}; - -static const struct attribute_group media_rc6_attr_group = { - .name = power_group_name, - .attrs = media_rc6_attrs -}; -#endif - static int l3_access_valid(struct drm_i915_private *i915, loff_t offset) { if (!HAS_L3_DPF(i915)) @@ -256,239 +155,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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->cur_freq)); -} - -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 = &i915->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->boost_freq)); -} - -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 = &dev_priv->gt.rps; - bool boost = false; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - /* 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 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 = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%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_rps *rps = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->max_freq_softlimit)); -} - -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_rps *rps = &dev_priv->gt.rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&rps->lock); - - val = intel_freq_opcode(rps, val); - if (val < rps->min_freq || - val > rps->max_freq || - val < rps->min_freq_softlimit) { - ret = -EINVAL; - goto unlock; - } - - if (val > rps->rp0_freq) - DRM_DEBUG("User requested overclocking to %d\n", - intel_gpu_freq(rps, val)); - - rps->max_freq_softlimit = val; - - val = clamp_t(int, rps->cur_freq, - rps->min_freq_softlimit, - rps->max_freq_softlimit); - - /* - * We still need *_set_rps to process the new max_delay and - * update the interrupt limits and PMINTRMSK even though - * frequency request may be unchanged. - */ - intel_rps_set(rps, val); - -unlock: - mutex_unlock(&rps->lock); - - return ret ?: count; -} - -static ssize_t gt_min_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 = &dev_priv->gt.rps; - - return snprintf(buf, PAGE_SIZE, "%d\n", - intel_gpu_freq(rps, rps->min_freq_softlimit)); -} - -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 *dev_priv = kdev_minor_to_i915(kdev); - struct intel_rps *rps = &dev_priv->gt.rps; - ssize_t ret; - u32 val; - - ret = kstrtou32(buf, 0, &val); - if (ret) - return ret; - - mutex_lock(&rps->lock); - - val = intel_freq_opcode(rps, val); - if (val < rps->min_freq || - val > rps->max_freq || - val > rps->max_freq_softlimit) { - ret = -EINVAL; - goto unlock; - } - - rps->min_freq_softlimit = val; - - val = clamp_t(int, rps->cur_freq, - rps->min_freq_softlimit, - rps->max_freq_softlimit); - - /* - * We still need *_set_rps to process the new min_delay and - * update the interrupt limits and PMINTRMSK even though - * frequency request may be unchanged. - */ - intel_rps_set(rps, val); - -unlock: - mutex_unlock(&rps->lock); - - 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 = &dev_priv->gt.rps; - u32 val; - - if (attr == &dev_attr_gt_RP0_freq_mhz) - val = intel_gpu_freq(rps, rps->rp0_freq); - else if (attr == &dev_attr_gt_RP1_freq_mhz) - val = intel_gpu_freq(rps, rps->rp1_freq); - else if (attr == &dev_attr_gt_RPn_freq_mhz) - val = intel_gpu_freq(rps, rps->min_freq); - else - BUG(); - - return snprintf(buf, PAGE_SIZE, "%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, @@ -559,29 +225,6 @@ void i915_setup_sysfs(struct drm_i915_private *dev_priv) struct device *kdev = dev_priv->drm.primary->kdev; int ret; -#ifdef CONFIG_PM - if (HAS_RC6(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &rc6_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "RC6 residency sysfs setup failed\n"); - } - if (HAS_RC6p(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &rc6p_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "RC6p residency sysfs setup failed\n"); - } - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { - ret = sysfs_merge_group(&kdev->kobj, - &media_rc6_attr_group); - if (ret) - drm_err(&dev_priv->drm, - "Media RC6 residency sysfs setup failed\n"); - } -#endif if (HAS_L3_DPF(dev_priv)) { ret = device_create_bin_file(kdev, &dpf_attrs); if (ret) @@ -597,14 +240,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 (INTEL_GEN(dev_priv) >= 6) - ret = sysfs_create_files(&kdev->kobj, gen6_attrs); - if (ret) - drm_err(&dev_priv->drm, "RPS sysfs setup failed\n"); - i915_setup_error_capture(kdev); } @@ -614,14 +249,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); -#ifdef CONFIG_PM - sysfs_unmerge_group(&kdev->kobj, &rc6_attr_group); - sysfs_unmerge_group(&kdev->kobj, &rc6p_attr_group); -#endif } diff --git a/drivers/gpu/drm/i915/i915_sysfs.h b/drivers/gpu/drm/i915/i915_sysfs.h index 41afd4366416..ad6114de81c9 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.h +++ b/drivers/gpu/drm/i915/i915_sysfs.h @@ -6,8 +6,11 @@ #ifndef __I915_SYSFS_H__ #define __I915_SYSFS_H__ +#include <linux/device.h> + struct drm_i915_private; +struct drm_i915_private *kdev_minor_to_i915(struct device *kdev); void i915_setup_sysfs(struct drm_i915_private *i915); void i915_teardown_sysfs(struct drm_i915_private *i915);
The GT has its own properties and in sysfs they should be grouped in the 'gt/' directory. Create the 'gt/' directory in sysfs and move the power management related files. The new interfaces are: gt/gt_act_freq_mhz gt/gt_boost_freq_mhz gt/gt_cur_freq_mhz gt/gt_info gt/gt_max_freq_mhz gt/gt_min_freq_mhz gt/gt_RP0_freq_mhz gt/gt_RP1_freq_mhz gt/gt_RPn_freq_mhz gt/rc6_enable gt/rc6_residency_ms The once in the root directory will be marked as deprecated, if accessed a warning message is printed. Signed-off-by: Andi Shyti <andi.shyti@intel.com> --- v4 -> v5: - removed spurious ghost 'vvv' file that was never meant to be there... sorry for spamming. v3 -> v4: - fixed Tvrtko's comments: - some renaming - some clumsy unbalanced kobject_put/get - the warning print is more descriptive and printed with limited rate - TODO: drm_print doesn't have a drm_warn_unlimited, to be added v2 -> v3: - fix some cleanups that I forgot in the previous patch - fix reference pointers to the gt structure - and many other small changes here and there. v1 -> v2: - keep the existing files as they are - use "intel_gt_*" as prefix instead of "sysfs_*" drivers/gpu/drm/i915/Makefile | 4 +- drivers/gpu/drm/i915/gt/intel_gt.c | 3 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + drivers/gpu/drm/i915/gt/sysfs_gt.c | 79 +++++ drivers/gpu/drm/i915/gt/sysfs_gt.h | 22 ++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 432 +++++++++++++++++++++++ drivers/gpu/drm/i915/gt/sysfs_gt_pm.h | 17 + drivers/gpu/drm/i915/i915_sysfs.c | 375 +------------------- drivers/gpu/drm/i915/i915_sysfs.h | 3 + 9 files changed, 561 insertions(+), 375 deletions(-) create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt.h create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c create mode 100644 drivers/gpu/drm/i915/gt/sysfs_gt_pm.h