* [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
@ 2020-02-11 18:10 Andi Shyti
2020-02-11 18:12 ` Andi Shyti
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Andi Shyti @ 2020-02-11 18:10 UTC (permalink / raw)
To: Intel GFX
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);
+ 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);
--
2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
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
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Andi Shyti @ 2020-02-11 18:12 UTC (permalink / raw)
To: Andi Shyti; +Cc: Intel GFX
On Tue, Feb 11, 2020 at 08:10:27PM +0200, 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_*"
As a side note of this patch, the current interface needs to be
declared as obsolete in the ABI, for now it prints a warning
whenever the old (or better existing) interface is called.
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
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
2020-02-11 21:06 ` Andi Shyti
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
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-02-11 19:58 UTC (permalink / raw)
To: Andi Shyti, Intel GFX
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
2020-02-11 19:58 ` Tvrtko Ursulin
@ 2020-02-11 21:06 ` Andi Shyti
2020-02-12 22:24 ` Tvrtko Ursulin
0 siblings, 1 reply; 8+ messages in thread
From: Andi Shyti @ 2020-02-11 21:06 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel GFX
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gt: make a gt sysfs group and move power management files (rev2)
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
@ 2020-02-12 20:25 ` 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 13:21 ` kbuild test robot
4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2020-02-12 20:25 UTC (permalink / raw)
To: Andi Shyti; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gt: make a gt sysfs group and move power management files (rev2)
URL : https://patchwork.freedesktop.org/series/73190/
State : failure
== Summary ==
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
CC [M] drivers/gpu/drm/i915/gt/sysfs_gt_pm.o
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
scripts/Makefile.build:267: recipe for target 'drivers/gpu/drm/i915/gt/sysfs_gt_pm.o' failed
make[4]: *** [drivers/gpu/drm/i915/gt/sysfs_gt_pm.o] Error 1
scripts/Makefile.build:505: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:505: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:505: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1681: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
2020-02-11 21:06 ` Andi Shyti
@ 2020-02-12 22:24 ` Tvrtko Ursulin
0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2020-02-12 22:24 UTC (permalink / raw)
To: Andi Shyti; +Cc: Intel GFX
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
2020-02-11 18:10 [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti
` (2 preceding siblings ...)
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 ` kbuild test robot
2020-02-14 13:21 ` kbuild test robot
4 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2020-02-14 8:47 UTC (permalink / raw)
To: Andi Shyti; +Cc: Intel GFX, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29028 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files
2020-02-11 18:10 [Intel-gfx] [PATCH v2] drm/i915/gt: make a gt sysfs group and move power management files Andi Shyti
` (3 preceding siblings ...)
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 13:21 ` kbuild test robot
4 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2020-02-14 13:21 UTC (permalink / raw)
To: Andi Shyti; +Cc: Intel GFX, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33140 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-02-14 13:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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 13:21 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).