dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
@ 2021-10-12 22:17 Andi Shyti
  2021-10-12 22:23 ` Andi Shyti
  2021-10-14  0:04 ` [Intel-gfx] " Lucas De Marchi
  0 siblings, 2 replies; 6+ messages in thread
From: Andi Shyti @ 2021-10-12 22:17 UTC (permalink / raw)
  To: Intel GFX, DRI Devel
  Cc: Tvrtko Ursulin, Chris Wilson, Lucas De Marchi, Andi Shyti, Andi Shyti

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(-)

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 = {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
  2021-10-12 22:17 [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt Andi Shyti
@ 2021-10-12 22:23 ` Andi Shyti
  2021-10-14  0:04 ` [Intel-gfx] " Lucas De Marchi
  1 sibling, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2021-10-12 22:23 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Intel GFX, DRI Devel, Tvrtko Ursulin, Chris Wilson,
	Lucas De Marchi, Andi Shyti

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
  2021-10-12 22:17 [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt Andi Shyti
  2021-10-12 22:23 ` Andi Shyti
@ 2021-10-14  0:04 ` Lucas De Marchi
  2021-10-14  0:11   ` Andi Shyti
  1 sibling, 1 reply; 6+ messages in thread
From: Lucas De Marchi @ 2021-10-14  0:04 UTC (permalink / raw)
  To: Andi Shyti; +Cc: Intel GFX, DRI Devel, Tvrtko Ursulin, Chris Wilson, Andi Shyti

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
  2021-10-14  0:04 ` [Intel-gfx] " Lucas De Marchi
@ 2021-10-14  0:11   ` Andi Shyti
  2021-10-15  4:54     ` Lucas De Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Andi Shyti @ 2021-10-14  0:11 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Andi Shyti, Intel GFX, DRI Devel, Tvrtko Ursulin, Chris Wilson,
	Andi Shyti

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
  2021-10-14  0:11   ` Andi Shyti
@ 2021-10-15  4:54     ` Lucas De Marchi
  2021-10-15  8:51       ` Andi Shyti
  0 siblings, 1 reply; 6+ messages in thread
From: Lucas De Marchi @ 2021-10-15  4:54 UTC (permalink / raw)
  To: Andi Shyti; +Cc: Andi Shyti, Intel GFX, DRI Devel, Tvrtko Ursulin, Chris Wilson

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-gfx] [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt
  2021-10-15  4:54     ` Lucas De Marchi
@ 2021-10-15  8:51       ` Andi Shyti
  0 siblings, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2021-10-15  8:51 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Andi Shyti, Andi Shyti, Intel GFX, DRI Devel, Tvrtko Ursulin,
	Chris Wilson

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-15  8:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 22:17 [PATCH v5] drm/i915/gt: move remaining debugfs interfaces into gt Andi Shyti
2021-10-12 22:23 ` Andi Shyti
2021-10-14  0:04 ` [Intel-gfx] " Lucas De Marchi
2021-10-14  0:11   ` Andi Shyti
2021-10-15  4:54     ` Lucas De Marchi
2021-10-15  8:51       ` Andi Shyti

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).