| Message ID | 20200211181027.5329-1-andi@etezian.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/gt: make a gt sysfs group and move power management files | expand |
On 11/02/2020 18:10, Andi Shyti wrote: > From: Andi Shyti <andi.shyti@intel.com> > > 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. > > Signed-off-by: Andi Shyti <andi.shyti@intel.com> > --- > Hi, > > this second version takes into account the compatibility with the > existing API. Version 1 was breaking it while this second version > generates the files twice in redundancy. > > Thanks Chris and Jani for the review, > Andi > > Changelog: > ========== > 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 | 67 ++++ > drivers/gpu/drm/i915/gt/sysfs_gt.h | 15 + > drivers/gpu/drm/i915/gt/sysfs_gt_pm.c | 456 +++++++++++++++++++++++ > 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, 566 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 49eed50ef0a4..3b81c8a96c06 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -106,7 +106,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..cdf659a7c74f 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 *kobj; > > 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..141518c101ed > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c > @@ -0,0 +1,67 @@ > +// 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_pm.h" > + > +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 const struct attribute *gt_attrs[] = { > + &dev_attr_gt_info.attr, > + NULL > +}; > + > +void intel_gt_sysfs_register(struct intel_gt *gt) > +{ > + struct kobject *parent = kobject_get(>->i915->drm.primary->kdev->kobj); Does this needs a kobject_put to balance out somewhere? > + int ret; > + > + gt->kobj = kobject_create_and_add("gt", parent); > + if (!gt->kobj) { > + pr_err("failed to initialize sysfs file\n"); > + return; > + } > + > + dev_set_drvdata(kobj_to_dev(gt->kobj), gt); > + > + ret = sysfs_create_files(gt->kobj, gt_attrs); > + if (ret) > + pr_err("failed to create sysfs gt info files\n"); I can't remember which log helper takes in the device and prefixes that string but I think it could be useful here, since the error is otherwise eaten. > + > + intel_gt_sysfs_pm_init(gt, gt->kobj); > + > + /* > + * 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) > +{ > + if (!gt->kobj) > + return; > + > + intel_gt_sysfs_pm_remove(gt, gt->kobj); > + sysfs_remove_files(gt->kobj, gt_attrs); Why is this asymmetrical to creation? I mean you call intel_gt_sysfs_pm_init two times with different roots, so why not intel_gt_sysfs_pm_remove also twice with different roots? Will the code do the right thing like this? Who removes the files in legacy root? And is kobject_put(gt->kobj) need somewhere to tear the gt root down? > +} > 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..07e08cd599b8 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#ifndef SYSFS_GT_H > +#define SYSFS_GT_H > + > +struct intel_gt; > + > +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..f3b373b9e953 > --- /dev/null > +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c > @@ -0,0 +1,456 @@ > +// SPDX-License-Identifier: MIT > + > +/* > + * Copyright © 2019 Intel Corporation > + */ > + > +#include <linux/sysfs.h> > +#include <drm/drm_device.h> > +#include <linux/printk.h> > + > +#include "../i915_drv.h" > +#include "../i915_sysfs.h" > +#include "intel_gt.h" > +#include "intel_gt_types.h" > +#include "intel_rc6.h" > +#include "intel_rps.h" > +#include "sysfs_gt_pm.h" > + > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) > +{ > + struct kobject *kobj = &dev->kobj; > + struct drm_i915_private *i915; > + > + /* > + * 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(kobj->name, "gt")) { > + pr_warn("the interface is obsolete, use gt/\n"); I think the message will need to be a bit more verbose since it is intended for users. I don't have any suggestions straight away apart from googling to find similar examples from the past and other subsystems. > + i915 = kdev_minor_to_i915(dev); > + return &i915->gt; > + } > + > + return dev_get_drvdata(dev); > +} > + > +#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); The rest of code is unchanged apart from this same line in all show/store vfuncs? Regards, Tvrtko > + 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 struct attribute *rc6_attrs[] = { > + &dev_attr_rc6_enable.attr, > + &dev_attr_rc6_residency_ms.attr, > + NULL > +}; > + > +static struct attribute *rc6p_attrs[] = { > + &dev_attr_rc6p_residency_ms.attr, > + &dev_attr_rc6pp_residency_ms.attr, > + NULL > +}; > + > +static struct attribute *media_rc6_attrs[] = { > + &dev_attr_media_rc6_residency_ms.attr, > + NULL > +}; > + > +static const struct attribute_group rc6_attribute_group = { > + .attrs = rc6_attrs, > +}; > + > +static const struct attribute_group rc6p_attribute_group = { > + .attrs = rc6p_attrs, > +}; > + > +static const struct attribute_group media_rc6_attribute_group = { > + .attrs = media_rc6_attrs, > +}; > + > +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) > +{ > + int ret = 0; > + > + if (HAS_RC6(gt->i915)) { > + ret = sysfs_create_group(kobj, &rc6_attribute_group); > + if (ret) > + pr_err("failed to create RC6 sysfs files\n"); > + } > + > + if (HAS_RC6p(gt->i915)) { > + ret = sysfs_merge_group(kobj, &rc6p_attribute_group); > + if (ret) > + pr_err("failed to create RC6p sysfs files\n"); > + } > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { > + ret = sysfs_merge_group(kobj, &media_rc6_attribute_group); > + if (ret) > + pr_err("failed to create media RC6 sysfs files\n"); > + } > +} > +#else > +static int 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) > + pr_err("failed to create RPS sysfs files"); > +} > + > +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj) > +{ > +#ifdef CONFIG_PM > + if (HAS_RC6p(gt->i915)) > + sysfs_unmerge_group(kobj, &rc6p_attribute_group); > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + sysfs_unmerge_group(kobj, &media_rc6_attribute_group); > + if (HAS_RC6(gt->i915)) > + sysfs_remove_group(kobj, &rc6_attribute_group); > +#endif > + > + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) > + sysfs_remove_files(kobj, vlv_attrs); > + else > + sysfs_remove_files(kobj, gen6_attrs); > +} > 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); > >
Hi Tvrtko, > > +void intel_gt_sysfs_register(struct intel_gt *gt) > > +{ > > + struct kobject *parent = kobject_get(>->i915->drm.primary->kdev->kobj); > > Does this needs a kobject_put to balance out somewhere? Yes, I forgot the two kobject_put that are needed. > > + int ret; > > + > > + gt->kobj = kobject_create_and_add("gt", parent); > > + if (!gt->kobj) { > > + pr_err("failed to initialize sysfs file\n"); > > + return; > > + } > > + > > + dev_set_drvdata(kobj_to_dev(gt->kobj), gt); > > + > > + ret = sysfs_create_files(gt->kobj, gt_attrs); > > + if (ret) > > + pr_err("failed to create sysfs gt info files\n"); > > I can't remember which log helper takes in the device and prefixes that > string but I think it could be useful here, since the error is otherwise > eaten. pr_* is used a lot in gt/. Playing with the dev pointer I can use "dev_err(dev,...)" > > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > > +{ > > + if (!gt->kobj) > > + return; > > + > > + intel_gt_sysfs_pm_remove(gt, gt->kobj); > > + sysfs_remove_files(gt->kobj, gt_attrs); > > Why is this asymmetrical to creation? Because in V1 gt_attrs and whatever was created in sysfs_gt_pm was in the same group, but it desn't matter. > I mean you call intel_gt_sysfs_pm_init > two times with different roots, so why not intel_gt_sysfs_pm_remove also > twice with different roots? Because I forgot them in the cleanups :) > > + /* > > + * 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(kobj->name, "gt")) { > > + pr_warn("the interface is obsolete, use gt/\n"); > > I think the message will need to be a bit more verbose since it is intended > for users. I don't have any suggestions straight away apart from googling to > find similar examples from the past and other subsystems. Yes, I couldn't come up with a better message in 80 characters, and I can use dev_warn instead of pr_warn. > > +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); > > The rest of code is unchanged apart from this same line in all show/store > vfuncs? yes, more or less. Andi
On 11/02/2020 21:06, Andi Shyti wrote: > Hi Tvrtko, > >>> +void intel_gt_sysfs_register(struct intel_gt *gt) >>> +{ >>> + struct kobject *parent = kobject_get(>->i915->drm.primary->kdev->kobj); >> >> Does this needs a kobject_put to balance out somewhere? > > Yes, I forgot the two kobject_put that are needed. > >>> + int ret; >>> + >>> + gt->kobj = kobject_create_and_add("gt", parent); >>> + if (!gt->kobj) { >>> + pr_err("failed to initialize sysfs file\n"); >>> + return; >>> + } >>> + >>> + dev_set_drvdata(kobj_to_dev(gt->kobj), gt); >>> + >>> + ret = sysfs_create_files(gt->kobj, gt_attrs); >>> + if (ret) >>> + pr_err("failed to create sysfs gt info files\n"); >> >> I can't remember which log helper takes in the device and prefixes that >> string but I think it could be useful here, since the error is otherwise >> eaten. > > pr_* is used a lot in gt/. Playing with the dev pointer I > can use "dev_err(dev,...)" > >>> +void intel_gt_sysfs_unregister(struct intel_gt *gt) >>> +{ >>> + if (!gt->kobj) >>> + return; >>> + >>> + intel_gt_sysfs_pm_remove(gt, gt->kobj); >>> + sysfs_remove_files(gt->kobj, gt_attrs); >> >> Why is this asymmetrical to creation? > > Because in V1 gt_attrs and whatever was created in sysfs_gt_pm > was in the same group, but it desn't matter. > >> I mean you call intel_gt_sysfs_pm_init >> two times with different roots, so why not intel_gt_sysfs_pm_remove also >> twice with different roots? > > Because I forgot them in the cleanups :) Next version incoming soon? :) >>> + /* >>> + * 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(kobj->name, "gt")) { >>> + pr_warn("the interface is obsolete, use gt/\n"); >> >> I think the message will need to be a bit more verbose since it is intended >> for users. I don't have any suggestions straight away apart from googling to >> find similar examples from the past and other subsystems. > > Yes, I couldn't come up with a better message in 80 characters, > and I can use dev_warn instead of pr_warn. Maybe we even need to log this once. Well we may need to log the offending process name/pid. Not sure if we can manage once on top of that.. sounds too hard. So maybe just name/pid. > >>> +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); >> >> The rest of code is unchanged apart from this same line in all show/store >> vfuncs? > > yes, more or less. Cool, just so I know what I don't have to cross-reference too much. Regards, Tvrtko
Hi Andi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next v5.6-rc1 next-20200213]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Andi-Shyti/drm-i915-gt-make-a-gt-sysfs-group-and-move-power-management-files/20200214-142110
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-defconfig (attached as .config)
compiler: gcc-7 (Debian 7.5.0-4) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/sysfs_gt_pm.c: In function 'intel_gt_sysfs_get_drvdata':
>> drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:26:49: warning: "/*" within comment [-Wcomment]
* has been called, whether it's called from gt/* or from
vim +26 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c
18
19 struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev)
20 {
21 struct kobject *kobj = &dev->kobj;
22 struct drm_i915_private *i915;
23
24 /*
25 * We are interested at knowing from where the interface
> 26 * has been called, whether it's called from gt/* or from
27 * the parent directory.
28 * From the interface position it depends also the value of
29 * the private data.
30 * If the interface is called from gt/ then private data is
31 * of the "struct intel_gt *" type, otherwise it's * a
32 * "struct drm_i915_private *" type.
33 */
34 if (strcmp(kobj->name, "gt")) {
35 pr_warn("the interface is obsolete, use gt/\n");
36 i915 = kdev_minor_to_i915(dev);
37 return &i915->gt;
38 }
39
40 return dev_get_drvdata(dev);
41 }
42
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Andi,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next v5.6-rc1 next-20200214]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Andi-Shyti/drm-i915-gt-make-a-gt-sysfs-group-and-move-power-management-files/20200214-142110
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: i386-randconfig-h003-20200213 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-4) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/sysfs_gt_pm.c: In function 'intel_gt_sysfs_get_drvdata':
>> drivers/gpu/drm/i915/gt/sysfs_gt_pm.c:26:49: error: "/*" within comment [-Werror=comment]
* has been called, whether it's called from gt/* or from
cc1: all warnings being treated as errors
vim +26 drivers/gpu/drm/i915/gt/sysfs_gt_pm.c
18
19 struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev)
20 {
21 struct kobject *kobj = &dev->kobj;
22 struct drm_i915_private *i915;
23
24 /*
25 * We are interested at knowing from where the interface
> 26 * has been called, whether it's called from gt/* or from
27 * the parent directory.
28 * From the interface position it depends also the value of
29 * the private data.
30 * If the interface is called from gt/ then private data is
31 * of the "struct intel_gt *" type, otherwise it's * a
32 * "struct drm_i915_private *" type.
33 */
34 if (strcmp(kobj->name, "gt")) {
35 pr_warn("the interface is obsolete, use gt/\n");
36 i915 = kdev_minor_to_i915(dev);
37 return &i915->gt;
38 }
39
40 return dev_get_drvdata(dev);
41 }
42
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 49eed50ef0a4..3b81c8a96c06 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -106,7 +106,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..cdf659a7c74f 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 *kobj; 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..141518c101ed --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c @@ -0,0 +1,67 @@ +// 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_pm.h" + +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 const struct attribute *gt_attrs[] = { + &dev_attr_gt_info.attr, + NULL +}; + +void intel_gt_sysfs_register(struct intel_gt *gt) +{ + struct kobject *parent = kobject_get(>->i915->drm.primary->kdev->kobj); + int ret; + + gt->kobj = kobject_create_and_add("gt", parent); + if (!gt->kobj) { + pr_err("failed to initialize sysfs file\n"); + return; + } + + dev_set_drvdata(kobj_to_dev(gt->kobj), gt); + + ret = sysfs_create_files(gt->kobj, gt_attrs); + if (ret) + pr_err("failed to create sysfs gt info files\n"); + + intel_gt_sysfs_pm_init(gt, gt->kobj); + + /* + * 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) +{ + if (!gt->kobj) + return; + + intel_gt_sysfs_pm_remove(gt, gt->kobj); + sysfs_remove_files(gt->kobj, gt_attrs); +} 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..07e08cd599b8 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2019 Intel Corporation + */ + +#ifndef SYSFS_GT_H +#define SYSFS_GT_H + +struct intel_gt; + +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..f3b373b9e953 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c @@ -0,0 +1,456 @@ +// SPDX-License-Identifier: MIT + +/* + * Copyright © 2019 Intel Corporation + */ + +#include <linux/sysfs.h> +#include <drm/drm_device.h> +#include <linux/printk.h> + +#include "../i915_drv.h" +#include "../i915_sysfs.h" +#include "intel_gt.h" +#include "intel_gt_types.h" +#include "intel_rc6.h" +#include "intel_rps.h" +#include "sysfs_gt_pm.h" + +struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev) +{ + struct kobject *kobj = &dev->kobj; + struct drm_i915_private *i915; + + /* + * 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(kobj->name, "gt")) { + pr_warn("the interface is obsolete, use gt/\n"); + i915 = kdev_minor_to_i915(dev); + return &i915->gt; + } + + return dev_get_drvdata(dev); +} + +#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 struct attribute *rc6_attrs[] = { + &dev_attr_rc6_enable.attr, + &dev_attr_rc6_residency_ms.attr, + NULL +}; + +static struct attribute *rc6p_attrs[] = { + &dev_attr_rc6p_residency_ms.attr, + &dev_attr_rc6pp_residency_ms.attr, + NULL +}; + +static struct attribute *media_rc6_attrs[] = { + &dev_attr_media_rc6_residency_ms.attr, + NULL +}; + +static const struct attribute_group rc6_attribute_group = { + .attrs = rc6_attrs, +}; + +static const struct attribute_group rc6p_attribute_group = { + .attrs = rc6p_attrs, +}; + +static const struct attribute_group media_rc6_attribute_group = { + .attrs = media_rc6_attrs, +}; + +static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj) +{ + int ret = 0; + + if (HAS_RC6(gt->i915)) { + ret = sysfs_create_group(kobj, &rc6_attribute_group); + if (ret) + pr_err("failed to create RC6 sysfs files\n"); + } + + if (HAS_RC6p(gt->i915)) { + ret = sysfs_merge_group(kobj, &rc6p_attribute_group); + if (ret) + pr_err("failed to create RC6p sysfs files\n"); + } + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) { + ret = sysfs_merge_group(kobj, &media_rc6_attribute_group); + if (ret) + pr_err("failed to create media RC6 sysfs files\n"); + } +} +#else +static int 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) + pr_err("failed to create RPS sysfs files"); +} + +void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj) +{ +#ifdef CONFIG_PM + if (HAS_RC6p(gt->i915)) + sysfs_unmerge_group(kobj, &rc6p_attribute_group); + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + sysfs_unmerge_group(kobj, &media_rc6_attribute_group); + if (HAS_RC6(gt->i915)) + sysfs_remove_group(kobj, &rc6_attribute_group); +#endif + + if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) + sysfs_remove_files(kobj, vlv_attrs); + else + sysfs_remove_files(kobj, gen6_attrs); +} 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);