From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Andi Shyti <andi@etezian.org>,
Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
Date: Tue, 11 Feb 2020 19:58:28 +0000 [thread overview]
Message-ID: <2defb42c-9fcc-17f8-0593-8e02f75f9ba4@linux.intel.com> (raw)
In-Reply-To: <20200211181027.5329-1-andi@etezian.org>
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);
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-02-11 19:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-11 18:10 [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti
2020-02-11 18:12 ` Andi Shyti
2020-02-11 19:58 ` Tvrtko Ursulin [this message]
2020-02-11 21:06 ` Andi Shyti
2020-02-12 22:24 ` Tvrtko Ursulin
2020-02-12 20:25 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev2) Patchwork
2020-02-14 8:47 ` [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files kbuild test robot
2020-02-14 8:47 ` kbuild test robot
2020-02-14 13:21 ` kbuild test robot
2020-02-14 13:21 ` kbuild test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2defb42c-9fcc-17f8-0593-8e02f75f9ba4@linux.intel.com \
--to=tvrtko.ursulin@linux.intel.com \
--cc=andi@etezian.org \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.