diff mbox series

[v5] drm/i915/gt: move remaining debugfs interfaces into gt

Message ID 20211012221738.16029-1-andi@etezian.org (mailing list archive)
State New, archived
Headers show
Series [v5] drm/i915/gt: move remaining debugfs interfaces into gt | expand

Commit Message

Andi Shyti Oct. 12, 2021, 10:17 p.m. UTC
From: Andi Shyti <andi.shyti@linux.intel.com>

The following interfaces:

  i915_wedged
  i915_forcewake_user

are dependent on gt values. Put them inside gt/ and drop the
"i915_" prefix name. This would be the new structure:

  dri/0/gt
  |
  +-- forcewake_user
  |
  \-- reset

For backwards compatibility with existing igt (and the slight
semantic difference between operating on the i915 abi entry
points and the deep gt info):

  dri/0
  |
  +-- i915_wedged
  |
  \-- i915_forcewake_user

remain at the top level.

Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_debugfs.c    | 55 +++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_debugfs.h    |  4 ++
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c | 41 ++++++++++++++
 drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h |  4 ++
 drivers/gpu/drm/i915/i915_debugfs.c           | 43 +++------------
 5 files changed, 111 insertions(+), 36 deletions(-)

Comments

Andi Shyti Oct. 12, 2021, 10:23 p.m. UTC | #1
Hi,

sorry, just forgot to add the changelog

On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote:
> From: Andi Shyti <andi.shyti@linux.intel.com>
> 
> The following interfaces:
> 
>   i915_wedged
>   i915_forcewake_user
> 
> are dependent on gt values. Put them inside gt/ and drop the
> "i915_" prefix name. This would be the new structure:
> 
>   dri/0/gt
>   |
>   +-- forcewake_user
>   |
>   \-- reset
> 
> For backwards compatibility with existing igt (and the slight
> semantic difference between operating on the i915 abi entry
> points and the deep gt info):
> 
>   dri/0
>   |
>   +-- i915_wedged
>   |
>   \-- i915_forcewake_user
> 
> remain at the top level.
> 
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---

Changelog:
----------
v4 -> v5: https://patchwork.freedesktop.org/patch/458293/
 * rename static functions exposed to header files so that they
   can keep a coherent namespace (thanks Lucas!)
 * add Lucas r-b.

v3 -> v4: https://patchwork.freedesktop.org/patch/458225/
 * remove the unnecessary interrupt_info_show() information. They
   were already removed here by Chris:

	cf977e18610e6 ("drm/i915/gem: Spring clean debugfs")

v2 -> v3: https://patchwork.freedesktop.org/patch/458108/
 * keep the original interfaces as they were (thanks Chris) but
   implement the functionality inside the gt. The upper level
   files will call the gt functions (thanks Lucas).

v1 -> v2: https://patchwork.freedesktop.org/patch/456652/
 * keep the original interfaces intact (thanks Chris).

Andi
Lucas De Marchi Oct. 14, 2021, 12:04 a.m. UTC | #2
On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote:
>From: Andi Shyti <andi.shyti@linux.intel.com>
>
>The following interfaces:
>
>  i915_wedged
>  i915_forcewake_user
>
>are dependent on gt values. Put them inside gt/ and drop the
>"i915_" prefix name. This would be the new structure:
>
>  dri/0/gt
>  |
>  +-- forcewake_user
>  |
>  \-- reset
>
>For backwards compatibility with existing igt (and the slight
>semantic difference between operating on the i915 abi entry
>points and the deep gt info):
>
>  dri/0
>  |
>  +-- i915_wedged
>  |
>  \-- i915_forcewake_user
>
>remain at the top level.
>
>Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>Cc: Chris Wilson <chris@chris-wilson.co.uk>
>Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

do you want me to push this?

Lucas De Marchi
Andi Shyti Oct. 14, 2021, 12:11 a.m. UTC | #3
Hi Lucas,

On Wed, Oct 13, 2021 at 05:04:27PM -0700, Lucas De Marchi wrote:
> On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote:
> > From: Andi Shyti <andi.shyti@linux.intel.com>
> > 
> > The following interfaces:
> > 
> >  i915_wedged
> >  i915_forcewake_user
> > 
> > are dependent on gt values. Put them inside gt/ and drop the
> > "i915_" prefix name. This would be the new structure:
> > 
> >  dri/0/gt
> >  |
> >  +-- forcewake_user
> >  |
> >  \-- reset
> > 
> > For backwards compatibility with existing igt (and the slight
> > semantic difference between operating on the i915 abi entry
> > points and the deep gt info):
> > 
> >  dri/0
> >  |
> >  +-- i915_wedged
> >  |
> >  \-- i915_forcewake_user
> > 
> > remain at the top level.
> > 
> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> do you want me to push this?

yes, please.

Thanks,
Andi
Lucas De Marchi Oct. 15, 2021, 4:54 a.m. UTC | #4
On Thu, Oct 14, 2021 at 02:11:34AM +0200, Andi Shyti wrote:
>Hi Lucas,
>
>On Wed, Oct 13, 2021 at 05:04:27PM -0700, Lucas De Marchi wrote:
>> On Wed, Oct 13, 2021 at 12:17:38AM +0200, Andi Shyti wrote:
>> > From: Andi Shyti <andi.shyti@linux.intel.com>
>> >
>> > The following interfaces:
>> >
>> >  i915_wedged
>> >  i915_forcewake_user
>> >
>> > are dependent on gt values. Put them inside gt/ and drop the
>> > "i915_" prefix name. This would be the new structure:
>> >
>> >  dri/0/gt
>> >  |
>> >  +-- forcewake_user
>> >  |
>> >  \-- reset
>> >
>> > For backwards compatibility with existing igt (and the slight
>> > semantic difference between operating on the i915 abi entry
>> > points and the deep gt info):
>> >
>> >  dri/0
>> >  |
>> >  +-- i915_wedged
>> >  |
>> >  \-- i915_forcewake_user
>> >
>> > remain at the top level.
>> >
>> > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> do you want me to push this?
>
>yes, please.

done, thanks.

Now, about igt: eventually we need to update it to use the gt
debugfs file. Is this something you have already or is it something
we are waiting on multi-gt to land?

Lucas De Marchi

>
>Thanks,
>Andi
Andi Shyti Oct. 15, 2021, 8:51 a.m. UTC | #5
Hi Lucas,

> > > > The following interfaces:
> > > >
> > > >  i915_wedged
> > > >  i915_forcewake_user
> > > >
> > > > are dependent on gt values. Put them inside gt/ and drop the
> > > > "i915_" prefix name. This would be the new structure:
> > > >
> > > >  dri/0/gt
> > > >  |
> > > >  +-- forcewake_user
> > > >  |
> > > >  \-- reset
> > > >
> > > > For backwards compatibility with existing igt (and the slight
> > > > semantic difference between operating on the i915 abi entry
> > > > points and the deep gt info):
> > > >
> > > >  dri/0
> > > >  |
> > > >  +-- i915_wedged
> > > >  |
> > > >  \-- i915_forcewake_user
> > > >
> > > > remain at the top level.
> > > >
> > > > Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > 
> > > do you want me to push this?
> > 
> > yes, please.
> 
> done, thanks.

Thanks!

> Now, about igt: eventually we need to update it to use the gt
> debugfs file. Is this something you have already or is it something
> we are waiting on multi-gt to land?

There is some work done in igt but it's all around multitile. I
think it's better to wait for the multitile to land and then
update igt.

In any case, at the current state, igt shouldn't be affected.

Thanks again,
Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
index 1fe19ccd2794..f103664b71d4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.c
@@ -13,6 +13,59 @@ 
 #include "pxp/intel_pxp_debugfs.h"
 #include "uc/intel_uc_debugfs.h"
 
+int intel_gt_debugfs_reset_show(struct intel_gt *gt, u64 *val)
+{
+	int ret = intel_gt_terminally_wedged(gt);
+
+	switch (ret) {
+	case -EIO:
+		*val = 1;
+		return 0;
+	case 0:
+		*val = 0;
+		return 0;
+	default:
+		return ret;
+	}
+}
+
+int intel_gt_debugfs_reset_store(struct intel_gt *gt, u64 val)
+{
+	/* Flush any previous reset before applying for a new one */
+	wait_event(gt->reset.queue,
+		   !test_bit(I915_RESET_BACKOFF, &gt->reset.flags));
+
+	intel_gt_handle_error(gt, val, I915_ERROR_CAPTURE,
+			      "Manually reset engine mask to %llx", val);
+	return 0;
+}
+
+/*
+ * keep the interface clean where the first parameter
+ * is a 'struct intel_gt *' instead of 'void *'
+ */
+static int __intel_gt_debugfs_reset_show(void *data, u64 *val)
+{
+	return intel_gt_debugfs_reset_show(data, val);
+}
+
+static int __intel_gt_debugfs_reset_store(void *data, u64 val)
+{
+	return intel_gt_debugfs_reset_store(data, val);
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(reset_fops, __intel_gt_debugfs_reset_show,
+			__intel_gt_debugfs_reset_store, "%llu\n");
+
+static void gt_debugfs_register(struct intel_gt *gt, struct dentry *root)
+{
+	static const struct intel_gt_debugfs_file files[] = {
+		{ "reset", &reset_fops, NULL },
+	};
+
+	intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt);
+}
+
 void intel_gt_debugfs_register(struct intel_gt *gt)
 {
 	struct dentry *root;
@@ -24,6 +77,8 @@  void intel_gt_debugfs_register(struct intel_gt *gt)
 	if (IS_ERR(root))
 		return;
 
+	gt_debugfs_register(gt, root);
+
 	intel_gt_engines_debugfs_register(gt, root);
 	intel_gt_pm_debugfs_register(gt, root);
 	intel_sseu_debugfs_register(gt, root);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h
index 8b6fca09897c..e307ceb99031 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_debugfs.h
@@ -35,4 +35,8 @@  void intel_gt_debugfs_register_files(struct dentry *root,
 				     const struct intel_gt_debugfs_file *files,
 				     unsigned long count, void *data);
 
+/* functions that need to be accessed by the upper level non-gt interfaces */
+int intel_gt_debugfs_reset_show(struct intel_gt *gt, u64 *val);
+int intel_gt_debugfs_reset_store(struct intel_gt *gt, u64 val);
+
 #endif /* INTEL_GT_DEBUGFS_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
index 5f84ad602642..0bc1454f38dd 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.c
@@ -19,6 +19,46 @@ 
 #include "intel_sideband.h"
 #include "intel_uncore.h"
 
+int intel_gt_pm_debugfs_forcewake_user_open(struct intel_gt *gt)
+{
+	atomic_inc(&gt->user_wakeref);
+	intel_gt_pm_get(gt);
+	if (GRAPHICS_VER(gt->i915) >= 6)
+		intel_uncore_forcewake_user_get(gt->uncore);
+
+	return 0;
+}
+
+int intel_gt_pm_debugfs_forcewake_user_release(struct intel_gt *gt)
+{
+	if (GRAPHICS_VER(gt->i915) >= 6)
+		intel_uncore_forcewake_user_put(gt->uncore);
+	intel_gt_pm_put(gt);
+	atomic_dec(&gt->user_wakeref);
+
+	return 0;
+}
+
+static int forcewake_user_open(struct inode *inode, struct file *file)
+{
+	struct intel_gt *gt = inode->i_private;
+
+	return intel_gt_pm_debugfs_forcewake_user_open(gt);
+}
+
+static int forcewake_user_release(struct inode *inode, struct file *file)
+{
+	struct intel_gt *gt = inode->i_private;
+
+	return intel_gt_pm_debugfs_forcewake_user_release(gt);
+}
+
+static const struct file_operations forcewake_user_fops = {
+	.owner = THIS_MODULE,
+	.open = forcewake_user_open,
+	.release = forcewake_user_release,
+};
+
 static int fw_domains_show(struct seq_file *m, void *data)
 {
 	struct intel_gt *gt = m->private;
@@ -627,6 +667,7 @@  void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root)
 		{ "drpc", &drpc_fops, NULL },
 		{ "frequency", &frequency_fops, NULL },
 		{ "forcewake", &fw_domains_fops, NULL },
+		{ "forcewake_user", &forcewake_user_fops, NULL},
 		{ "llc", &llc_fops, llc_eval },
 		{ "rps_boost", &rps_boost_fops, rps_eval },
 	};
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h
index 2b824289582b..a8457887ec65 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm_debugfs.h
@@ -13,4 +13,8 @@  struct drm_printer;
 void intel_gt_pm_debugfs_register(struct intel_gt *gt, struct dentry *root);
 void intel_gt_pm_frequency_dump(struct intel_gt *gt, struct drm_printer *m);
 
+/* functions that need to be accessed by the upper level non-gt interfaces */
+int intel_gt_pm_debugfs_forcewake_user_open(struct intel_gt *gt);
+int intel_gt_pm_debugfs_forcewake_user_release(struct intel_gt *gt);
+
 #endif /* INTEL_GT_PM_DEBUGFS_H */
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fdbd46ff59e0..636cc3cf88be 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -35,6 +35,7 @@ 
 #include "gt/intel_gt.h"
 #include "gt/intel_gt_buffer_pool.h"
 #include "gt/intel_gt_clock_utils.h"
+#include "gt/intel_gt_debugfs.h"
 #include "gt/intel_gt_pm.h"
 #include "gt/intel_gt_pm_debugfs.h"
 #include "gt/intel_gt_requests.h"
@@ -554,36 +555,18 @@  static int i915_wa_registers(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static int
-i915_wedged_get(void *data, u64 *val)
+static int i915_wedged_get(void *data, u64 *val)
 {
 	struct drm_i915_private *i915 = data;
-	int ret = intel_gt_terminally_wedged(&i915->gt);
 
-	switch (ret) {
-	case -EIO:
-		*val = 1;
-		return 0;
-	case 0:
-		*val = 0;
-		return 0;
-	default:
-		return ret;
-	}
+	return intel_gt_debugfs_reset_show(&i915->gt, val);
 }
 
-static int
-i915_wedged_set(void *data, u64 val)
+static int i915_wedged_set(void *data, u64 val)
 {
 	struct drm_i915_private *i915 = data;
 
-	/* Flush any previous reset before applying for a new one */
-	wait_event(i915->gt.reset.queue,
-		   !test_bit(I915_RESET_BACKOFF, &i915->gt.reset.flags));
-
-	intel_gt_handle_error(&i915->gt, val, I915_ERROR_CAPTURE,
-			      "Manually set wedged engine mask = %llx", val);
-	return 0;
+	return intel_gt_debugfs_reset_store(&i915->gt, val);
 }
 
 DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
@@ -728,27 +711,15 @@  static int i915_sseu_status(struct seq_file *m, void *unused)
 static int i915_forcewake_open(struct inode *inode, struct file *file)
 {
 	struct drm_i915_private *i915 = inode->i_private;
-	struct intel_gt *gt = &i915->gt;
-
-	atomic_inc(&gt->user_wakeref);
-	intel_gt_pm_get(gt);
-	if (GRAPHICS_VER(i915) >= 6)
-		intel_uncore_forcewake_user_get(gt->uncore);
 
-	return 0;
+	return intel_gt_pm_debugfs_forcewake_user_open(&i915->gt);
 }
 
 static int i915_forcewake_release(struct inode *inode, struct file *file)
 {
 	struct drm_i915_private *i915 = inode->i_private;
-	struct intel_gt *gt = &i915->gt;
 
-	if (GRAPHICS_VER(i915) >= 6)
-		intel_uncore_forcewake_user_put(&i915->uncore);
-	intel_gt_pm_put(gt);
-	atomic_dec(&gt->user_wakeref);
-
-	return 0;
+	return intel_gt_pm_debugfs_forcewake_user_release(&i915->gt);
 }
 
 static const struct file_operations i915_forcewake_fops = {