Re: [PATCH] drm/i915/gt: make a gt sysfs group and move power management files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 14/02/2020 11:03, Andi Shyti wrote:
The GT has its own properties and in sysfs they should be grouped
in the 'gt/' directory.

Create the 'gt/' directory in sysfs and move the power management
related files.

Can you paste the new and legacy paths in the commit message?


Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxx>
---
Hi,

this version has some more substantial differences, nothing that
changes the wanted behavior, though.

Thanks Chris, Jani and Tvrtko for the reviews,
Andi

Changelog:
==========

v2 -> v3:
  - fix some cleanups that I forgot in the previous patch
  - fix reference pointers to the gt structure
  - and many other small changes here and there.
v1 -> v2:
  - keep the existing files as they are
  - use "intel_gt_*" as prefix instead of "sysfs_*"

  drivers/gpu/drm/i915/Makefile            |   4 +-
  drivers/gpu/drm/i915/gt/intel_gt.c       |   3 +
  drivers/gpu/drm/i915/gt/intel_gt_types.h |   1 +
  drivers/gpu/drm/i915/gt/sysfs_gt.c       |  85 +++++
  drivers/gpu/drm/i915/gt/sysfs_gt.h       |  22 ++
  drivers/gpu/drm/i915/gt/sysfs_gt_pm.c    | 446 +++++++++++++++++++++++
  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, 581 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(&gt->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(&gt->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..552a27cc0622 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;

sysfs_root or something like would perhaps be more descriptive?

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..4ca140fc215f
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/sysfs_gt.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include <linux/sysfs.h>
+#include <drm/drm_device.h>
+#include <linux/kobject.h>
+#include <linux/printk.h>
+
+#include "../i915_drv.h"
+#include "intel_gt.h"
+#include "intel_gt_types.h"
+#include "intel_rc6.h"
+
+#include "sysfs_gt.h"
+#include "sysfs_gt_pm.h"
+
+static inline struct kobject *gt_to_parent_obj(struct intel_gt *gt)
+{
+	return kobject_get(&gt->i915->drm.primary->kdev->kobj);

It's a bit surprising X_to_Y helper get a reference as well, no? gt_get_parent_obj perhaps? But where is this released?

+}
+
+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 void sysfs_gt_kobj_release(struct kobject *kobj)
+{
+	struct intel_gt *gt = kobj_to_gt(kobj);
+
+	drm_info(&gt->i915->drm, "releasing interface\n");

Debugging remnants.

+}
+
+static struct kobj_type sysfs_gt_ktype = {
+	.release   = sysfs_gt_kobj_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+};
+
+void intel_gt_sysfs_register(struct intel_gt *gt)
+{
+	struct kobject *kparent = gt_to_parent_obj(gt);
+	int ret;
+
+	ret = kobject_init_and_add(&gt->kobj, &sysfs_gt_ktype, kparent, "gt");
+	if (ret) {
+		drm_err(&gt->i915->drm, "failed to initialize sysfs file\n");
+		kobject_put(&gt->kobj);

So you want gt->kobj to be embedded struct and you want to then override the release vfunc so it is not freed, but what is the specific reason you want it embedded?

+		goto parent_files;
+	}
+
+	ret = sysfs_create_file(&gt->kobj, &dev_attr_gt_info.attr);
+	if (ret)
+		drm_err(&gt->i915->drm, "failed to create sysfs gt info files\n");
+
+	intel_gt_sysfs_pm_init(gt, &gt->kobj);
+
+parent_files:
+	/*
+	 * we need to make things right with the
+	 * ABI compatibility. The files were originally
+	 * generated under the parent directory.
+	 */
+	intel_gt_sysfs_pm_init(gt, kparent);
+}
+
+void intel_gt_sysfs_unregister(struct intel_gt *gt)
+{
+	struct kobject *root = gt_to_parent_obj(gt);
+
+	if (&gt->kobj) {

This is always true.

+		sysfs_remove_file(&gt->kobj, &dev_attr_gt_info.attr);
+		intel_gt_sysfs_pm_remove(gt, &gt->kobj);
+		kobject_put(&gt->kobj);

I think kobject_put is enough to tear down the whole hierarchy so you could simplify this.

+	}
+
+	intel_gt_sysfs_pm_remove(gt, root);
+	kobject_put(root);

Maybe stick to the same terminology regarding root and parent.

Get/put on the parent looks unbalanced. Both register and unregister take a reference and only unregister releases it. But do you even need a reference?

+}
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..efff6b884b18
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/sysfs_gt.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef SYSFS_GT_H
+#define SYSFS_GT_H
+
+#include "intel_gt_types.h"
+
+struct intel_gt;
+
+static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
+{
+	return container_of(kobj, struct intel_gt, kobj);
+}
+
+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..a3746485569f
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/sysfs_gt_pm.c
@@ -0,0 +1,446 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include <drm/drm_device.h>
+#include <linux/sysfs.h>
+#include <linux/printk.h>
+
+#include "../i915_drv.h"
+#include "../i915_sysfs.h"
+#include "intel_gt.h"
+#include "intel_rc6.h"
+#include "intel_rps.h"
+#include "sysfs_gt.h"
+#include "sysfs_gt_pm.h"
+
+struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev)
+{
+	struct kobject *kobj = &dev->kobj;
+	/*
+	 * We are interested at knowing from where the interface
+	 * has been called, whether it's called from gt/ or from
+	 * the parent directory.
+	 * From the interface position it depends also the value of
+	 * the private data.
+	 * If the interface is called from gt/ then private data is
+	 * of the "struct intel_gt *" type, otherwise it's * a
+	 * "struct drm_i915_private *" type.
+	 */
+	if (strcmp(dev->kobj.name, "gt")) {
+		struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
+
+		drm_warn(&i915->drm, "the interface is obsolete, use gt/\n");

Can you log current->name & pid?

I am also thinking is a level down from warn would be better. Notice sounds intuitively correct to me.

I am also tempted by the _once alternative, but then it makes less sense to include name & pid.

Regards,

Tvrtko

+		return &i915->gt;
+	}
+
+	return kobj_to_gt(kobj);
+}
+
+#ifdef CONFIG_PM
+static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
+{
+	intel_wakeref_t wakeref;
+	u64 res = 0;
+
+	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+		res = intel_rc6_residency_us(&gt->rc6, reg);
+
+	return DIV_ROUND_CLOSEST_ULL(res, 1000);
+}
+
+static ssize_t rc6_enable_show(struct device *dev,
+			       struct device_attribute *attr,
+			       char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+	u8 mask = 0;
+
+	if (HAS_RC6(gt->i915))
+		mask |= BIT(0);
+	if (HAS_RC6p(gt->i915))
+		mask |= BIT(1);
+	if (HAS_RC6pp(gt->i915))
+		mask |= BIT(2);
+
+	return snprintf(buff, PAGE_SIZE, "%x\n", mask);
+}
+
+static ssize_t rc6_residency_ms_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+	u32 rc6_residency = get_residency(gt, GEN6_GT_GFX_RC6);
+
+	return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency);
+}
+
+static ssize_t rc6p_residency_ms_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+	u32 rc6p_residency = get_residency(gt, GEN6_GT_GFX_RC6p);
+
+	return snprintf(buff, PAGE_SIZE, "%u\n", rc6p_residency);
+}
+
+static ssize_t rc6pp_residency_ms_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+	u32 rc6pp_residency = get_residency(gt, GEN6_GT_GFX_RC6pp);
+
+	return snprintf(buff, PAGE_SIZE, "%u\n", rc6pp_residency);
+}
+
+static ssize_t media_rc6_residency_ms_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+	u32 rc6_residency = get_residency(gt, VLV_GT_MEDIA_RC6);
+
+	return snprintf(buff, PAGE_SIZE, "%u\n", rc6_residency);
+}
+
+static DEVICE_ATTR_RO(rc6_enable);
+static DEVICE_ATTR_RO(rc6_residency_ms);
+static DEVICE_ATTR_RO(rc6p_residency_ms);
+static DEVICE_ATTR_RO(rc6pp_residency_ms);
+static DEVICE_ATTR_RO(media_rc6_residency_ms);
+
+static const struct attribute *rc6_attrs[] = {
+	&dev_attr_rc6_enable.attr,
+	&dev_attr_rc6_residency_ms.attr,
+	NULL
+};
+
+static const struct attribute *rc6p_attrs[] = {
+	&dev_attr_rc6p_residency_ms.attr,
+	&dev_attr_rc6pp_residency_ms.attr,
+	NULL
+};
+
+static const struct attribute *media_rc6_attrs[] = {
+	&dev_attr_media_rc6_residency_ms.attr,
+	NULL
+};
+
+static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
+{
+	int ret = 0;
+
+	if (HAS_RC6(gt->i915)) {
+		ret = sysfs_create_files(kobj, rc6_attrs);
+		if (ret)
+			drm_err(&gt->i915->drm,
+				"failed to create RC6 sysfs files\n");
+	}
+
+	if (HAS_RC6p(gt->i915)) {
+		ret = sysfs_create_files(kobj, rc6p_attrs);
+		if (ret)
+			drm_err(&gt->i915->drm,
+				"failed to create RC6p sysfs files\n");
+	}
+
+	if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915)) {
+		ret = sysfs_create_files(kobj, media_rc6_attrs);
+		if (ret)
+			drm_err(&gt->i915->drm,
+				"failed to create media RC6 sysfs files\n");
+	}
+}
+#else
+static void intel_sysfs_rc6_init(struct intel_gt *gt, struct kobject *kobj)
+{
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static ssize_t gt_act_freq_mhz_show(struct device *dev,
+				    struct device_attribute *attr, char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev);
+
+	return snprintf(buff, PAGE_SIZE, "%d\n",
+			intel_rps_read_actual_frequency(&gt->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 = &gt->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 = &gt->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 = &gt->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 = &gt->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 = &gt->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 = &gt->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 = &gt->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 = &gt->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 = &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(buff, PAGE_SIZE, "%d\n", val);
+}
+
+static const struct attribute * const gen6_attrs[] = {
+	&dev_attr_gt_act_freq_mhz.attr,
+	&dev_attr_gt_cur_freq_mhz.attr,
+	&dev_attr_gt_boost_freq_mhz.attr,
+	&dev_attr_gt_max_freq_mhz.attr,
+	&dev_attr_gt_min_freq_mhz.attr,
+	&dev_attr_gt_RP0_freq_mhz.attr,
+	&dev_attr_gt_RP1_freq_mhz.attr,
+	&dev_attr_gt_RPn_freq_mhz.attr,
+	NULL,
+};
+
+static const struct attribute * const vlv_attrs[] = {
+	&dev_attr_gt_act_freq_mhz.attr,
+	&dev_attr_gt_cur_freq_mhz.attr,
+	&dev_attr_gt_boost_freq_mhz.attr,
+	&dev_attr_gt_max_freq_mhz.attr,
+	&dev_attr_gt_min_freq_mhz.attr,
+	&dev_attr_gt_RP0_freq_mhz.attr,
+	&dev_attr_gt_RP1_freq_mhz.attr,
+	&dev_attr_gt_RPn_freq_mhz.attr,
+	&dev_attr_vlv_rpe_freq_mhz.attr,
+	NULL,
+};
+
+static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
+{
+	if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
+		return sysfs_create_files(kobj, vlv_attrs);
+
+	if (INTEL_GEN(gt->i915) >= 6)
+		return sysfs_create_files(kobj, gen6_attrs);
+
+	return 0;
+}
+
+void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
+{
+	int ret;
+
+	intel_sysfs_rc6_init(gt, kobj);
+
+	ret = intel_sysfs_rps_init(gt, kobj);
+	if (ret)
+		drm_err(&gt->i915->drm, "failed to create RPS sysfs files");
+}
+
+void intel_gt_sysfs_pm_remove(struct intel_gt *gt, struct kobject *kobj)
+{
+#ifdef CONFIG_PM
+	if (HAS_RC6(gt->i915))
+		sysfs_remove_files(kobj, rc6_attrs);
+	if (HAS_RC6p(gt->i915))
+		sysfs_remove_files(kobj, rc6p_attrs);
+	if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
+		sysfs_remove_files(kobj, media_rc6_attrs);
+#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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux