Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/gt: Move power management debug files into a gt aware debugfs
Quoting Andi Shyti (2019-12-13 14:44:04)
> Hi Chris,
>
> > > +int intel_gt_pm_debugfs_register(struct intel_gt *gt)
> > > +{
> > > + struct dentry *root = gt->debugfs_entry;
> > > + int i;
> > > +
> > > + pr_info("ANDIII function start\n");
> > > + if (unlikely(!root))
> > > + return -ENODEV;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(gt_pm_debugfs_files); i++) {
> > > + const struct gt_pm_debugfs_files *f =
> > > _pm_debugfs_files[i];
> > > +
> > > + if (f->eval && !f->eval(gt))
> > > + continue;
> > > +
> > > + debugfs_create_file(f->name, 0444, root, gt, f->fops);
> > > + }
> > > + pr_info("ANDIII function end\n");
> > > +
> > > + return 0;
> >
> > Looking better!
>
> you liked my wonderful debugging printouts? :D
>
> > Do we even need to keep the gt->debugfs_entry around?
>
> Yes, that can be removed, indeed, I was thinking that perhaps in
> the future we might need that pointer, but yes, definitely not
> necessary.
>
> > We are not going to ever do hotplug are we and so only
> > need to populate once?
>
> for example? do you mean something like files generated in
> certain conditions, like during interrupts or files generated and
> destroyed depending on the power state?
>
> If so, you might want an interface for generating/destroying
> files, but do we have such case now to really care about?
No, I was trying to highlight that such a dynamic interface is hard to
deal with, so we don't bother unless it inherent in the HW and we can't
avoid it. i.e. we only need to populate the debugfs once on boot and
then can throw away our dentry pointers.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/gt: Move power management debug files into a gt aware debugfs
Hi Chris,
> > +int intel_gt_pm_debugfs_register(struct intel_gt *gt)
> > +{
> > + struct dentry *root = gt->debugfs_entry;
> > + int i;
> > +
> > + pr_info("ANDIII function start\n");
> > + if (unlikely(!root))
> > + return -ENODEV;
> > +
> > + for (i = 0; i < ARRAY_SIZE(gt_pm_debugfs_files); i++) {
> > + const struct gt_pm_debugfs_files *f =
> > _pm_debugfs_files[i];
> > +
> > + if (f->eval && !f->eval(gt))
> > + continue;
> > +
> > + debugfs_create_file(f->name, 0444, root, gt, f->fops);
> > + }
> > + pr_info("ANDIII function end\n");
> > +
> > + return 0;
>
> Looking better!
you liked my wonderful debugging printouts? :D
> Do we even need to keep the gt->debugfs_entry around?
Yes, that can be removed, indeed, I was thinking that perhaps in
the future we might need that pointer, but yes, definitely not
necessary.
> We are not going to ever do hotplug are we and so only
> need to populate once?
for example? do you mean something like files generated in
certain conditions, like during interrupts or files generated and
destroyed depending on the power state?
If so, you might want an interface for generating/destroying
files, but do we have such case now to really care about?
Andi
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/2] drm/i915/gt: Move power management debug files into a gt aware debugfs
From: Andi Shyti
The GT system is becoming more and more a stand-alone system in
i915 and it's fair to assign it its own debugfs directory.
rc6, rps and llc debugfs files are gt related, move them into the
gt debugfs directory.
Signed-off-by: Andi Shyti
---
drivers/gpu/drm/i915/Makefile| 2 +
drivers/gpu/drm/i915/gt/debugfs_gt.c | 19 +
drivers/gpu/drm/i915/gt/debugfs_gt.h | 26 +
drivers/gpu/drm/i915/gt/debugfs_pm.c | 626 +++
drivers/gpu/drm/i915/gt/debugfs_pm.h | 16 +
drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 +
drivers/gpu/drm/i915/i915_debugfs.c | 579 +
7 files changed, 701 insertions(+), 570 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/debugfs_gt.c
create mode 100644 drivers/gpu/drm/i915/gt/debugfs_gt.h
create mode 100644 drivers/gpu/drm/i915/gt/debugfs_pm.c
create mode 100644 drivers/gpu/drm/i915/gt/debugfs_pm.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e0fd10c0cfb8..8f6cdfd08ac8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -75,6 +75,8 @@ i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
# "Graphics Technology" (aka we talk to the gpu)
obj-y += gt/
gt-y += \
+ gt/debugfs_gt.o \
+ gt/debugfs_pm.o \
gt/intel_breadcrumbs.o \
gt/intel_context.o \
gt/intel_engine_cs.o \
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c
b/drivers/gpu/drm/i915/gt/debugfs_gt.c
new file mode 100644
index ..33abe0234d56
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt.c
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "debugfs_pm.h"
+#include "i915_trace.h"
+
+int debugfs_gt_init(struct intel_gt *gt)
+{
+ struct drm_minor *minor = gt->i915->drm.primary;
+
+ if (!minor->debugfs_root)
+ return -ENODEV;
+
+ gt->debugfs_entry = debugfs_create_dir("gt", minor->debugfs_root);
+
+ return intel_gt_pm_debugfs_register(gt);
+}
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.h
b/drivers/gpu/drm/i915/gt/debugfs_gt.h
new file mode 100644
index ..cac5a3503459
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef DEBUGFS_GT
+#define DEBUGFS_GT
+
+#include "intel_gt_types.h"
+
+#define DEFINE_GT_DEBUGFS_ATTRIBUTE(__name)\
+static int __name ## _open(struct inode *inode, struct file *file) \
+{ \
+ return single_open(file, __name ## _show, inode->i_private);\
+} \
+static const struct file_operations __name ## _fops = {
\
+ .owner = THIS_MODULE, \
+ .open = __name ## _open,\
+ .read = seq_read, \
+ .llseek = seq_lseek,\
+ .release = single_release, \
+}
+
+int debugfs_gt_init(struct intel_gt *gt);
+
+#endif /* DEBUGFS_GT */
diff --git a/drivers/gpu/drm/i915/gt/debugfs_pm.c
b/drivers/gpu/drm/i915/gt/debugfs_pm.c
new file mode 100644
index ..b5ea8848cdfc
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/debugfs_pm.c
@@ -0,0 +1,626 @@
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "debugfs_pm.h"
+#include "debugfs_gt.h"
+#include "i915_debugfs.h"
+#include "i915_trace.h"
+#include "intel_rc6.h"
+#include "intel_rps.h"
+#include "intel_sideband.h"
+
+static int frequency_info_show(struct seq_file *m, void *unused)
+{
+ struct intel_gt *gt = m->private;
+ struct drm_i915_private *i915 = gt->i915;
+ struct intel_uncore *uncore = gt->uncore;
+ struct intel_rps *rps = >rps;
+ intel_wakeref_t wakeref;
+
+ wakeref = intel_runtime_pm_get(uncore->rpm);
+
+ if (IS_GEN(i915, 5)) {
+ u16 rgvswctl = intel_uncore_read16(uncore, MEMSWCTL);
+ u16 rgvstat = intel_uncore_read16(uncore, MEMSTAT_ILK);
+
+ seq_printf(m, "Requested P-state: %d\n", (rgvswctl >> 8) & 0xf);
+ seq_printf(m, "Requested VID: %d\n", rgvswctl & 0x3f);
+ seq_printf(m, "Current VID: %d\n", (rgvstat & MEMSTAT_VID_MASK)
>>
+ MEMSTAT_VID_SHIFT);
+ seq_printf(m, "Current P-state: %d\n",
+ (rgvstat & MEMSTAT_PSTATE_MASK) >>
MEMSTAT_PSTATE_SHIFT);
+ } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
+ u32 rpmodectl, freq_sts;
+
+ rpmodectl = intel_uncore_read(uncore, GEN6_RP_CONTROL);
+ seq_printf(m, "Video Turbo Mode: %s\n",
+
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915/gt: Move power management debug files into a gt aware debugfs
Quoting Andi Shyti (2019-12-13 12:45:49)
> +int debugfs_gt_init(struct intel_gt *gt)
> +{
> + struct drm_minor *minor = gt->i915->drm.primary;
> +
> + if (!minor->debugfs_root)
> + return -ENODEV;
> +
> + gt->debugfs_entry = debugfs_create_dir("gt", minor->debugfs_root);
> +
> + return intel_gt_pm_debugfs_register(gt);
> +}
> +static const struct gt_pm_debugfs_files {
> + const char *name;
> + const struct file_operations *fops;
> + bool (*eval)(struct intel_gt *gt);
> +} gt_pm_debugfs_files[] = {
> + { "frequency_info", _info_fops, NULL },
> + { "ring_freq_table", _freq_table_fops, ring_freq_table_eval },
> + { "rps_boost_info", _boost_info_fops, NULL },
> + { "forcewake_domains", _domains_fops, NULL },
> + { "drpc_info", _info_fops, NULL },
> + { "llc", _fops, NULL },
Resort into alphabetical. We should probably cull a few.
(Thinking rps_info, rc6_info, llc_info, with a few other debug knobs
for specific use cases.)
> +};
> +
> +int intel_gt_pm_debugfs_register(struct intel_gt *gt)
> +{
> + struct dentry *root = gt->debugfs_entry;
> + int i;
> +
> + pr_info("ANDIII function start\n");
> + if (unlikely(!root))
> + return -ENODEV;
> +
> + for (i = 0; i < ARRAY_SIZE(gt_pm_debugfs_files); i++) {
> + const struct gt_pm_debugfs_files *f = _pm_debugfs_files[i];
> +
> + if (f->eval && !f->eval(gt))
> + continue;
> +
> + debugfs_create_file(f->name, 0444, root, gt, f->fops);
> + }
> + pr_info("ANDIII function end\n");
> +
> + return 0;
Looking better!
Do we even need to keep the gt->debugfs_entry around? We are not going to
ever do hotplug are we and so only need to populate once?
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
