intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
@ 2020-03-06 23:03 Andi Shyti
  2020-03-07  2:26 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Andi Shyti @ 2020-03-06 23:03 UTC (permalink / raw)
  To: Intel GFX

From: Andi Shyti <andi.shyti@intel.com>

When registering debugfs files the intel gt debugfs library
forces a 'struct *gt' private data on the caller.

To be open to different usages make the new
"intel_gt_debugfs_register_files()"[*] function more generic by
converting the 'struct *gt' pointer to a 'void *' type.

I take the chance to rename the functions by using "intel_gt_" as
prefix instead of "debugfs_", so that "debugfs_gt_register_files()"
becomes "intel_gt_debugfs_register_files()".

Signed-off-by: Andi Shyti <andi.shyti@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
Hi,

Thanks Daniele for the review.

Andi

Changelog:
v4:
 - removed the wrapper which turns out it's not needed anymore.
v3:
 - removed unused gt parameter from the
   __intel_gt_debugfs_register_files()
v2:
 - the eval function is made generic as suggested by Daniele.

 drivers/gpu/drm/i915/gt/debugfs_engines.c |  2 +-
 drivers/gpu/drm/i915/gt/debugfs_gt.c      | 11 +++++------
 drivers/gpu/drm/i915/gt/debugfs_gt.h      |  9 ++++-----
 drivers/gpu/drm/i915/gt/debugfs_gt_pm.c   | 14 +++++++++-----
 4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/debugfs_engines.c b/drivers/gpu/drm/i915/gt/debugfs_engines.c
index 6a5e9ab20b94..5e3725e62241 100644
--- a/drivers/gpu/drm/i915/gt/debugfs_engines.c
+++ b/drivers/gpu/drm/i915/gt/debugfs_engines.c
@@ -32,5 +32,5 @@ void debugfs_engines_register(struct intel_gt *gt, struct dentry *root)
 		{ "engines", &engines_fops },
 	};
 
-	debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files));
+	intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt);
 }
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.c b/drivers/gpu/drm/i915/gt/debugfs_gt.c
index 75255aaacaed..de73b63d6ba7 100644
--- a/drivers/gpu/drm/i915/gt/debugfs_gt.c
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt.c
@@ -26,15 +26,14 @@ void debugfs_gt_register(struct intel_gt *gt)
 	debugfs_gt_pm_register(gt, root);
 }
 
-void debugfs_gt_register_files(struct intel_gt *gt,
-			       struct dentry *root,
-			       const struct debugfs_gt_file *files,
-			       unsigned long count)
+void intel_gt_debugfs_register_files(struct dentry *root,
+				     const struct debugfs_gt_file *files,
+				     unsigned long count, void *data)
 {
 	while (count--) {
-		if (!files->eval || files->eval(gt))
+		if (!files->eval || files->eval(data))
 			debugfs_create_file(files->name,
-					    0444, root, gt,
+					    0444, root, data,
 					    files->fops);
 
 		files++;
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt.h b/drivers/gpu/drm/i915/gt/debugfs_gt.h
index 4ea0f06cda8f..f77540f727e9 100644
--- a/drivers/gpu/drm/i915/gt/debugfs_gt.h
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt.h
@@ -28,12 +28,11 @@ void debugfs_gt_register(struct intel_gt *gt);
 struct debugfs_gt_file {
 	const char *name;
 	const struct file_operations *fops;
-	bool (*eval)(const struct intel_gt *gt);
+	bool (*eval)(void *data);
 };
 
-void debugfs_gt_register_files(struct intel_gt *gt,
-			       struct dentry *root,
-			       const struct debugfs_gt_file *files,
-			       unsigned long count);
+void intel_gt_debugfs_register_files(struct dentry *root,
+				     const struct debugfs_gt_file *files,
+				     unsigned long count, void *data);
 
 #endif /* DEBUGFS_GT_H */
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
index 059c9e5c002e..dc024944873a 100644
--- a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
@@ -506,9 +506,11 @@ static int llc_show(struct seq_file *m, void *data)
 	return 0;
 }
 
-static bool llc_eval(const struct intel_gt *gt)
+static bool llc_eval(void *data)
 {
-	return HAS_LLC(gt->i915);
+	struct intel_gt *gt = data;
+
+	return gt && HAS_LLC(gt->i915);
 }
 
 DEFINE_GT_DEBUGFS_ATTRIBUTE(llc);
@@ -580,9 +582,11 @@ static int rps_boost_show(struct seq_file *m, void *data)
 	return 0;
 }
 
-static bool rps_eval(const struct intel_gt *gt)
+static bool rps_eval(void *data)
 {
-	return HAS_RPS(gt->i915);
+	struct intel_gt *gt = data;
+
+	return gt && HAS_RPS(gt->i915);
 }
 
 DEFINE_GT_DEBUGFS_ATTRIBUTE(rps_boost);
@@ -597,5 +601,5 @@ void debugfs_gt_pm_register(struct intel_gt *gt, struct dentry *root)
 		{ "rps_boost", &rps_boost_fops, rps_eval },
 	};
 
-	debugfs_gt_register_files(gt, root, files, ARRAY_SIZE(files));
+	intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt);
 }
-- 
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] 13+ messages in thread

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: allow setting generic data pointer (rev4)
  2020-03-06 23:03 [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Andi Shyti
@ 2020-03-07  2:26 ` Patchwork
  2020-03-07  2:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2020-03-07  2:26 UTC (permalink / raw)
  To: Andi Shyti; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: allow setting generic data pointer (rev4)
URL   : https://patchwork.freedesktop.org/series/74360/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/display/intel_dpll_mgr.h:285: warning: Function parameter or member 'get_freq' not described in 'intel_shared_dpll_funcs'

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: allow setting generic data pointer (rev4)
  2020-03-06 23:03 [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Andi Shyti
  2020-03-07  2:26 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
@ 2020-03-07  2:40 ` Patchwork
  2020-03-07 12:07 ` [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Chris Wilson
  2020-03-07 21:10 ` [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2020-03-07  2:40 UTC (permalink / raw)
  To: Andi Shyti; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: allow setting generic data pointer (rev4)
URL   : https://patchwork.freedesktop.org/series/74360/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8088 -> Patchwork_16869
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/index.html

Known issues
------------

  Here are the changes found in Patchwork_16869 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gem_contexts:
    - fi-cml-s:           [PASS][1] -> [DMESG-FAIL][2] ([i915#877])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-cml-s/igt@i915_selftest@live@gem_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-cml-s/igt@i915_selftest@live@gem_contexts.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][3] -> [FAIL][4] ([i915#262])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@vgem_basic@setversion:
    - fi-tgl-y:           [PASS][5] -> [DMESG-WARN][6] ([CI#94] / [i915#402]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-tgl-y/igt@vgem_basic@setversion.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-tgl-y/igt@vgem_basic@setversion.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@execlists:
    - fi-bsw-kefka:       [DMESG-FAIL][7] ([i915#1314]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-bsw-kefka/igt@i915_selftest@live@execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-bsw-kefka/igt@i915_selftest@live@execlists.html

  * igt@kms_addfb_basic@bo-too-small-due-to-tiling:
    - fi-tgl-y:           [DMESG-WARN][9] ([CI#94] / [i915#402]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-tgl-y/igt@kms_addfb_basic@bo-too-small-due-to-tiling.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-tgl-y/igt@kms_addfb_basic@bo-too-small-due-to-tiling.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [FAIL][11] ([fdo#109635] / [i915#217]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111096] / [i915#323]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [CI#94]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/94
  [fdo#109635]: https://bugs.freedesktop.org/show_bug.cgi?id=109635
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [i915#1314]: https://gitlab.freedesktop.org/drm/intel/issues/1314
  [i915#217]: https://gitlab.freedesktop.org/drm/intel/issues/217
  [i915#262]: https://gitlab.freedesktop.org/drm/intel/issues/262
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877


Participating hosts (46 -> 43)
------------------------------

  Additional (1): fi-bwr-2160 
  Missing    (4): fi-skl-6770hq fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8088 -> Patchwork_16869

  CI-20190529: 20190529
  CI_DRM_8088: 91dc8b179da374160a6bbdbd6987a512a10fbc02 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5498: 1bb7a25a09fe3e653d310e8bdfbdde4a1934b326 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16869: 5c203f8d2312d96d4fd0f912635ddfc455722866 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5c203f8d2312 drm/i915/gt: allow setting generic data pointer

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-06 23:03 [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Andi Shyti
  2020-03-07  2:26 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
  2020-03-07  2:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-03-07 12:07 ` Chris Wilson
  2020-03-07 12:55   ` Andi Shyti
  2020-03-07 21:10 ` [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
  3 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2020-03-07 12:07 UTC (permalink / raw)
  To: Andi Shyti, Intel GFX

Quoting Andi Shyti (2020-03-06 23:03:44)
> -void debugfs_gt_register_files(struct intel_gt *gt,
> -                              struct dentry *root,
> -                              const struct debugfs_gt_file *files,
> -                              unsigned long count)
> +void intel_gt_debugfs_register_files(struct dentry *root,
> +                                    const struct debugfs_gt_file *files,
> +                                    unsigned long count, void *data)
>  {
>         while (count--) {
> -               if (!files->eval || files->eval(gt))
> +               if (!files->eval || files->eval(data))
>                         debugfs_create_file(files->name,
> -                                           0444, root, gt,
> +                                           0444, root, data,
>                                             files->fops);
>  

And now we are not a intel_gt routine, you'll want to move again :)
i915_debugfs_utils.c ? :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-07 12:07 ` [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Chris Wilson
@ 2020-03-07 12:55   ` Andi Shyti
  2020-03-07 17:35     ` Chris Wilson
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Shyti @ 2020-03-07 12:55 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel GFX

Hi Chris,

On Sat, Mar 07, 2020 at 12:07:22PM +0000, Chris Wilson wrote:
> Quoting Andi Shyti (2020-03-06 23:03:44)
> > -void debugfs_gt_register_files(struct intel_gt *gt,
> > -                              struct dentry *root,
> > -                              const struct debugfs_gt_file *files,
> > -                              unsigned long count)
> > +void intel_gt_debugfs_register_files(struct dentry *root,
> > +                                    const struct debugfs_gt_file *files,
> > +                                    unsigned long count, void *data)
> >  {
> >         while (count--) {
> > -               if (!files->eval || files->eval(gt))
> > +               if (!files->eval || files->eval(data))
> >                         debugfs_create_file(files->name,
> > -                                           0444, root, gt,
> > +                                           0444, root, data,
> >                                             files->fops);
> >  
> 
> And now we are not a intel_gt routine, you'll want to move again :)
> i915_debugfs_utils.c ? :)

Actually, this is what it came to and this was the first
discussion I had with Daniele and that's also why I was loyal to
th "_gt_" wrappers until the end. But I had to agree that this
was becoming more a limitation.

The biggest difference left, which by the way is the real
distinguishing factor other than the *gt pointer, is that we
create files under gt directory, instead of having the root
imposed by the drm (even though the caller can eventually choose
different roots).

We could perhaps store the root pointer in the intel_gt
structure so that this function stays de facto an intel_gt
routine and the caller doesn't need to care where the files will
be generated. This is what we planned to do with sysfs as well.

What do you think?

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-07 12:55   ` Andi Shyti
@ 2020-03-07 17:35     ` Chris Wilson
  2020-03-07 22:19       ` Andi Shyti
  0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2020-03-07 17:35 UTC (permalink / raw)
  To: Andi Shyti; +Cc: Intel GFX

Quoting Andi Shyti (2020-03-07 12:55:31)
> Hi Chris,
> 
> On Sat, Mar 07, 2020 at 12:07:22PM +0000, Chris Wilson wrote:
> > Quoting Andi Shyti (2020-03-06 23:03:44)
> > > -void debugfs_gt_register_files(struct intel_gt *gt,
> > > -                              struct dentry *root,
> > > -                              const struct debugfs_gt_file *files,
> > > -                              unsigned long count)
> > > +void intel_gt_debugfs_register_files(struct dentry *root,
> > > +                                    const struct debugfs_gt_file *files,
> > > +                                    unsigned long count, void *data)
> > >  {
> > >         while (count--) {
> > > -               if (!files->eval || files->eval(gt))
> > > +               if (!files->eval || files->eval(data))
> > >                         debugfs_create_file(files->name,
> > > -                                           0444, root, gt,
> > > +                                           0444, root, data,
> > >                                             files->fops);
> > >  
> > 
> > And now we are not a intel_gt routine, you'll want to move again :)
> > i915_debugfs_utils.c ? :)
> 
> Actually, this is what it came to and this was the first
> discussion I had with Daniele and that's also why I was loyal to
> th "_gt_" wrappers until the end. But I had to agree that this
> was becoming more a limitation.
> 
> The biggest difference left, which by the way is the real
> distinguishing factor other than the *gt pointer, is that we
> create files under gt directory, instead of having the root
> imposed by the drm (even though the caller can eventually choose
> different roots).
> 
> We could perhaps store the root pointer in the intel_gt
> structure so that this function stays de facto an intel_gt
> routine and the caller doesn't need to care where the files will
> be generated. This is what we planned to do with sysfs as well.
> 
> What do you think?

I thought we were passing along the root. If not I think we should, more
of a debugfs constructor context?

The main thing of course is not to overengineer and do the minimal
necessary for the immediate users we have. We can always extend and
refactor for a third user, etc, etc.

So if this works for gt + children, go for it and worry about tomorrow,
tomorrow. Trusting our good practice for "a stitch in time saves nine".
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: allow setting generic data pointer (rev4)
  2020-03-06 23:03 [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Andi Shyti
                   ` (2 preceding siblings ...)
  2020-03-07 12:07 ` [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Chris Wilson
@ 2020-03-07 21:10 ` Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2020-03-07 21:10 UTC (permalink / raw)
  To: Andi Shyti; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: allow setting generic data pointer (rev4)
URL   : https://patchwork.freedesktop.org/series/74360/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8088_full -> Patchwork_16869_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_16869_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([i915#180]) +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-apl4/igt@gem_ctx_isolation@rcs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-apl1/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#110841])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb7/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@implicit-read-write-bsd1:
    - shard-iclb:         [PASS][7] -> [SKIP][8] ([fdo#109276] / [i915#677])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb2/igt@gem_exec_schedule@implicit-read-write-bsd1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb3/igt@gem_exec_schedule@implicit-read-write-bsd1.html

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#112146]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb6/igt@gem_exec_schedule@preempt-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb1/igt@gem_exec_schedule@preempt-bsd.html

  * igt@gem_exec_whisper@basic-fds-all:
    - shard-glk:          [PASS][11] -> [DMESG-WARN][12] ([i915#118] / [i915#95])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-glk1/igt@gem_exec_whisper@basic-fds-all.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-glk2/igt@gem_exec_whisper@basic-fds-all.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][13] -> [FAIL][14] ([i915#644])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-glk7/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-glk1/igt@gem_ppgtt@flink-and-close-vma-leak.html
    - shard-apl:          [PASS][15] -> [FAIL][16] ([i915#644])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-apl6/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-apl2/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][17] -> [INCOMPLETE][18] ([i915#716])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl2/igt@gen9_exec_parse@allowed-single.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl6/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([i915#454])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb1/igt@i915_pm_dc@dc6-psr.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb8/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][21] -> [INCOMPLETE][22] ([i915#221])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl10/igt@kms_flip@flip-vs-suspend-interruptible.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#34])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl2/igt@kms_flip@plain-flip-fb-recreate.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl8/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-skl:          [PASS][25] -> [INCOMPLETE][26] ([i915#69])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl10/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [PASS][27] -> [INCOMPLETE][28] ([fdo#103665])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          [PASS][29] -> [DMESG-WARN][30] ([i915#180]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][31] -> [FAIL][32] ([fdo#108145] / [i915#265])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][33] -> [SKIP][34] ([fdo#109441]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@perf_pmu@busy-vcs1:
    - shard-iclb:         [PASS][35] -> [SKIP][36] ([fdo#112080]) +13 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb1/igt@perf_pmu@busy-vcs1.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb8/igt@perf_pmu@busy-vcs1.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][37] -> [SKIP][38] ([fdo#109276]) +17 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb4/igt@prime_vgem@fence-wait-bsd2.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb3/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_busy@extended-parallel-vcs1:
    - shard-iclb:         [SKIP][39] ([fdo#112080]) -> [PASS][40] +12 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb5/igt@gem_busy@extended-parallel-vcs1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb4/igt@gem_busy@extended-parallel-vcs1.html

  * igt@gem_ctx_persistence@close-replace-race:
    - shard-kbl:          [INCOMPLETE][41] ([fdo#103665]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl6/igt@gem_ctx_persistence@close-replace-race.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl1/igt@gem_ctx_persistence@close-replace-race.html
    - shard-iclb:         [TIMEOUT][43] -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb6/igt@gem_ctx_persistence@close-replace-race.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb1/igt@gem_ctx_persistence@close-replace-race.html

  * igt@gem_exec_schedule@implicit-both-bsd2:
    - shard-iclb:         [SKIP][45] ([fdo#109276] / [i915#677]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb5/igt@gem_exec_schedule@implicit-both-bsd2.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb4/igt@gem_exec_schedule@implicit-both-bsd2.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][47] ([fdo#109276]) -> [PASS][48] +20 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb6/igt@gem_exec_schedule@independent-bsd2.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb1/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_exec_schedule@pi-distinct-iova-bsd:
    - shard-iclb:         [SKIP][49] ([i915#677]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb2/igt@gem_exec_schedule@pi-distinct-iova-bsd.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb3/igt@gem_exec_schedule@pi-distinct-iova-bsd.html

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-iclb:         [SKIP][51] ([fdo#112146]) -> [PASS][52] +7 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb1/igt@gem_exec_schedule@preempt-other-chain-bsd.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-kbl:          [FAIL][53] ([i915#644]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl3/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl7/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-iclb:         [FAIL][55] ([i915#447]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb3/igt@i915_pm_dc@dc5-dpms.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb8/igt@i915_pm_dc@dc5-dpms.html

  * igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding:
    - shard-skl:          [FAIL][57] ([i915#54]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl5/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][59] ([i915#180]) -> [PASS][60] +3 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][61] ([i915#72]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-skl:          [FAIL][63] ([IGT#5]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][65] ([i915#46]) -> [PASS][66]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl2/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][67] ([i915#79]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          [FAIL][69] ([i915#34]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl1/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl5/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][71] ([i915#1188]) -> [PASS][72] +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl10/igt@kms_hdr@bpc-switch-dpms.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          [DMESG-WARN][73] ([i915#180]) -> [PASS][74] +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-iclb:         [INCOMPLETE][75] ([i915#250]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][77] ([fdo#108145]) -> [PASS][78] +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-glk:          [FAIL][79] ([i915#899]) -> [PASS][80] +1 similar issue
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-glk2/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-glk9/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][81] ([fdo#109642] / [fdo#111068]) -> [PASS][82]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb5/igt@kms_psr2_su@frontbuffer.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][83] ([fdo#109441]) -> [PASS][84] +2 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-iclb5/igt@kms_psr@psr2_primary_page_flip.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][85] ([i915#31]) -> [PASS][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-kbl2/igt@kms_setmode@basic.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-kbl2/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [SKIP][87] ([i915#468]) -> [FAIL][88] ([i915#454])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-tglb1/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-snb:          [SKIP][89] ([fdo#109271]) -> [INCOMPLETE][90] ([i915#82])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8088/shard-snb6/igt@i915_pm_rpm@system-suspend-execbuf.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/shard-snb6/igt@i915_pm_rpm@system-suspend-execbuf.html

  
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#221]: https://gitlab.freedesktop.org/drm/intel/issues/221
  [i915#250]: https://gitlab.freedesktop.org/drm/intel/issues/250
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#447]: https://gitlab.freedesktop.org/drm/intel/issues/447
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#46]: https://gitlab.freedesktop.org/drm/intel/issues/46
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8088 -> Patchwork_16869

  CI-20190529: 20190529
  CI_DRM_8088: 91dc8b179da374160a6bbdbd6987a512a10fbc02 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5498: 1bb7a25a09fe3e653d310e8bdfbdde4a1934b326 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16869: 5c203f8d2312d96d4fd0f912635ddfc455722866 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16869/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-07 17:35     ` Chris Wilson
@ 2020-03-07 22:19       ` Andi Shyti
  2020-03-09 22:05         ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Shyti @ 2020-03-07 22:19 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel GFX

Hi Chris,

> > > Quoting Andi Shyti (2020-03-06 23:03:44)
> > > > -void debugfs_gt_register_files(struct intel_gt *gt,
> > > > -                              struct dentry *root,
> > > > -                              const struct debugfs_gt_file *files,
> > > > -                              unsigned long count)
> > > > +void intel_gt_debugfs_register_files(struct dentry *root,
> > > > +                                    const struct debugfs_gt_file *files,
> > > > +                                    unsigned long count, void *data)
> > > >  {
> > > >         while (count--) {
> > > > -               if (!files->eval || files->eval(gt))
> > > > +               if (!files->eval || files->eval(data))
> > > >                         debugfs_create_file(files->name,
> > > > -                                           0444, root, gt,
> > > > +                                           0444, root, data,
> > > >                                             files->fops);
> > > >  
> > > 
> > > And now we are not a intel_gt routine, you'll want to move again :)
> > > i915_debugfs_utils.c ? :)
> > 
> > Actually, this is what it came to and this was the first
> > discussion I had with Daniele and that's also why I was loyal to
> > th "_gt_" wrappers until the end. But I had to agree that this
> > was becoming more a limitation.
> > 
> > The biggest difference left, which by the way is the real
> > distinguishing factor other than the *gt pointer, is that we
> > create files under gt directory, instead of having the root
> > imposed by the drm (even though the caller can eventually choose
> > different roots).
> > 
> > We could perhaps store the root pointer in the intel_gt
> > structure so that this function stays de facto an intel_gt
> > routine and the caller doesn't need to care where the files will
> > be generated. This is what we planned to do with sysfs as well.
> > 
> > What do you think?
> 
> I thought we were passing along the root. If not I think we should, more
> of a debugfs constructor context?

What do you mean with debugfs constructor context? Is it a
gt->debugfs_root pointer like the gt->sysfs_root?

> The main thing of course is not to overengineer and do the minimal
> necessary for the immediate users we have. We can always extend and
> refactor for a third user, etc, etc.
> 
> So if this works for gt + children, go for it and worry about tomorrow,
> tomorrow. Trusting our good practice for "a stitch in time saves nine".

this came after Daniele's guc patches where he preferred to
define his own functions instead of using this one that is meant
to be used in that situation.

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-07 22:19       ` Andi Shyti
@ 2020-03-09 22:05         ` Daniele Ceraolo Spurio
  2020-03-09 22:38           ` Andi Shyti
  0 siblings, 1 reply; 13+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-09 22:05 UTC (permalink / raw)
  To: Andi Shyti, Chris Wilson; +Cc: Intel GFX



On 3/7/20 2:19 PM, Andi Shyti wrote:
> Hi Chris,
> 
>>>> Quoting Andi Shyti (2020-03-06 23:03:44)
>>>>> -void debugfs_gt_register_files(struct intel_gt *gt,
>>>>> -                              struct dentry *root,
>>>>> -                              const struct debugfs_gt_file *files,
>>>>> -                              unsigned long count)
>>>>> +void intel_gt_debugfs_register_files(struct dentry *root,
>>>>> +                                    const struct debugfs_gt_file *files,
>>>>> +                                    unsigned long count, void *data)
>>>>>   {
>>>>>          while (count--) {
>>>>> -               if (!files->eval || files->eval(gt))
>>>>> +               if (!files->eval || files->eval(data))
>>>>>                          debugfs_create_file(files->name,
>>>>> -                                           0444, root, gt,
>>>>> +                                           0444, root, data,
>>>>>                                              files->fops);
>>>>>   
>>>>
>>>> And now we are not a intel_gt routine, you'll want to move again :)
>>>> i915_debugfs_utils.c ? :)
>>>
>>> Actually, this is what it came to and this was the first
>>> discussion I had with Daniele and that's also why I was loyal to
>>> th "_gt_" wrappers until the end. But I had to agree that this
>>> was becoming more a limitation.
>>>
>>> The biggest difference left, which by the way is the real
>>> distinguishing factor other than the *gt pointer, is that we
>>> create files under gt directory, instead of having the root
>>> imposed by the drm (even though the caller can eventually choose
>>> different roots).
>>>
>>> We could perhaps store the root pointer in the intel_gt
>>> structure so that this function stays de facto an intel_gt
>>> routine and the caller doesn't need to care where the files will
>>> be generated. This is what we planned to do with sysfs as well.
>>>
>>> What do you think?
>>
>> I thought we were passing along the root. If not I think we should, more
>> of a debugfs constructor context?
> 
> What do you mean with debugfs constructor context? Is it a
> gt->debugfs_root pointer like the gt->sysfs_root?
> 

Getting the root pointer internally from gt wouldn't work well for 
subfolders, like the gt/uc/ folder I want to add for GuC/HuC files. I 
think extracting this generic helper to a common file, possibly as a 
follow-up step, isn't a bad idea, also considering that there is at 
least 1 more use-case in i915_debugfs_register(). Maybe we can 
generalize as something like:

struct i915_debugfs_files {
	const char *name;
	const struct file_operations *fops;
	bool (*eval)(void *data);
}

void i915_debugfs_register_files(struct dentry *root,
				 const struct i915_debugfs_files *files,
				 unsigned long count, void *data)
{
  	while (count--) {
		umode_t mode = files->fops->write ? 0644 : 0444;
		if (!files->eval || files->eval(data))
  			debugfs_create_file(files->name,
					    mode, root, data,
  					    files->fops);
	}
}

Daniele

>> The main thing of course is not to overengineer and do the minimal
>> necessary for the immediate users we have. We can always extend and
>> refactor for a third user, etc, etc.
>>
>> So if this works for gt + children, go for it and worry about tomorrow,
>> tomorrow. Trusting our good practice for "a stitch in time saves nine".
> 
> this came after Daniele's guc patches where he preferred to
> define his own functions instead of using this one that is meant
> to be used in that situation.
> 
> Andi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-09 22:05         ` Daniele Ceraolo Spurio
@ 2020-03-09 22:38           ` Andi Shyti
  2020-03-09 23:11             ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Shyti @ 2020-03-09 22:38 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: Intel GFX

Hi Daniele,

> > > > > Quoting Andi Shyti (2020-03-06 23:03:44)
> > > > > > -void debugfs_gt_register_files(struct intel_gt *gt,
> > > > > > -                              struct dentry *root,
> > > > > > -                              const struct debugfs_gt_file *files,
> > > > > > -                              unsigned long count)
> > > > > > +void intel_gt_debugfs_register_files(struct dentry *root,
> > > > > > +                                    const struct debugfs_gt_file *files,
> > > > > > +                                    unsigned long count, void *data)
> > > > > >   {
> > > > > >          while (count--) {
> > > > > > -               if (!files->eval || files->eval(gt))
> > > > > > +               if (!files->eval || files->eval(data))
> > > > > >                          debugfs_create_file(files->name,
> > > > > > -                                           0444, root, gt,
> > > > > > +                                           0444, root, data,
> > > > > >                                              files->fops);
> > > > > 
> > > > > And now we are not a intel_gt routine, you'll want to move again :)
> > > > > i915_debugfs_utils.c ? :)
> > > > 
> > > > Actually, this is what it came to and this was the first
> > > > discussion I had with Daniele and that's also why I was loyal to
> > > > th "_gt_" wrappers until the end. But I had to agree that this
> > > > was becoming more a limitation.
> > > > 
> > > > The biggest difference left, which by the way is the real
> > > > distinguishing factor other than the *gt pointer, is that we
> > > > create files under gt directory, instead of having the root
> > > > imposed by the drm (even though the caller can eventually choose
> > > > different roots).
> > > > 
> > > > We could perhaps store the root pointer in the intel_gt
> > > > structure so that this function stays de facto an intel_gt
> > > > routine and the caller doesn't need to care where the files will
> > > > be generated. This is what we planned to do with sysfs as well.
> > > > 
> > > > What do you think?
> > > 
> > > I thought we were passing along the root. If not I think we should, more
> > > of a debugfs constructor context?
> > 
> > What do you mean with debugfs constructor context? Is it a
> > gt->debugfs_root pointer like the gt->sysfs_root?
> > 

> Getting the root pointer internally from gt wouldn't work well for
> subfolders, like the gt/uc/ folder I want to add for GuC/HuC files.

this was not my idea, actually I was thinking the opposite.

When in this case you call "intel_gt_debugfs_register_files", you
would provide "gt" pointer where the funcion extracts and handles
by its own the debugfs_root. The caller doesn't need to care
about it.

Another idea could be to use contexts, e.g. guc or pm or whatever
comes to mind, and the intel_gt_debugfs handles everything
including subdirectories.

> I think extracting this generic helper to a common file, possibly as a follow-up
> step, isn't a bad idea, also considering that there is at least 1 more
> use-case in i915_debugfs_register(). Maybe we can generalize as something
> like:
> 
> struct i915_debugfs_files {
> 	const char *name;
> 	const struct file_operations *fops;
> 	bool (*eval)(void *data);
> }
> 
> void i915_debugfs_register_files(struct dentry *root,
> 				 const struct i915_debugfs_files *files,
> 				 unsigned long count, void *data)
> {
>  	while (count--) {
> 		umode_t mode = files->fops->write ? 0644 : 0444;
> 		if (!files->eval || files->eval(data))
>  			debugfs_create_file(files->name,
> 					    mode, root, data,
>  					    files->fops);
> 	}
> }

apart from the mode, isn't this the same as the latest patch you
actually reviewed?

> void i915_debugfs_register_files(struct dentry *root,

based on my proposal, root would point, in your case, to the
"guc/" directory that will be created under the "gt/". NULL if
you want the file to be created in the main "gt/" directory.

While if we want to go by context, we could do something like:

struct i915_debugfs_files {                                    
      const char *name;                                        
      const struct file_operations *fops;                      
      bool (*eval)(void *data);                                
      enum intel_gt_context context;
}

and the gt handles everything.

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-09 22:38           ` Andi Shyti
@ 2020-03-09 23:11             ` Daniele Ceraolo Spurio
  2020-03-11  8:24               ` Andi Shyti
  0 siblings, 1 reply; 13+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-09 23:11 UTC (permalink / raw)
  To: Andi Shyti; +Cc: Intel GFX



On 3/9/20 3:38 PM, Andi Shyti wrote:
> Hi Daniele,
> 
>>>>>> Quoting Andi Shyti (2020-03-06 23:03:44)
>>>>>>> -void debugfs_gt_register_files(struct intel_gt *gt,
>>>>>>> -                              struct dentry *root,
>>>>>>> -                              const struct debugfs_gt_file *files,
>>>>>>> -                              unsigned long count)
>>>>>>> +void intel_gt_debugfs_register_files(struct dentry *root,
>>>>>>> +                                    const struct debugfs_gt_file *files,
>>>>>>> +                                    unsigned long count, void *data)
>>>>>>>    {
>>>>>>>           while (count--) {
>>>>>>> -               if (!files->eval || files->eval(gt))
>>>>>>> +               if (!files->eval || files->eval(data))
>>>>>>>                           debugfs_create_file(files->name,
>>>>>>> -                                           0444, root, gt,
>>>>>>> +                                           0444, root, data,
>>>>>>>                                               files->fops);
>>>>>>
>>>>>> And now we are not a intel_gt routine, you'll want to move again :)
>>>>>> i915_debugfs_utils.c ? :)
>>>>>
>>>>> Actually, this is what it came to and this was the first
>>>>> discussion I had with Daniele and that's also why I was loyal to
>>>>> th "_gt_" wrappers until the end. But I had to agree that this
>>>>> was becoming more a limitation.
>>>>>
>>>>> The biggest difference left, which by the way is the real
>>>>> distinguishing factor other than the *gt pointer, is that we
>>>>> create files under gt directory, instead of having the root
>>>>> imposed by the drm (even though the caller can eventually choose
>>>>> different roots).
>>>>>
>>>>> We could perhaps store the root pointer in the intel_gt
>>>>> structure so that this function stays de facto an intel_gt
>>>>> routine and the caller doesn't need to care where the files will
>>>>> be generated. This is what we planned to do with sysfs as well.
>>>>>
>>>>> What do you think?
>>>>
>>>> I thought we were passing along the root. If not I think we should, more
>>>> of a debugfs constructor context?
>>>
>>> What do you mean with debugfs constructor context? Is it a
>>> gt->debugfs_root pointer like the gt->sysfs_root?
>>>
> 
>> Getting the root pointer internally from gt wouldn't work well for
>> subfolders, like the gt/uc/ folder I want to add for GuC/HuC files.
> 
> this was not my idea, actually I was thinking the opposite.
> 
> When in this case you call "intel_gt_debugfs_register_files", you
> would provide "gt" pointer where the funcion extracts and handles
> by its own the debugfs_root. The caller doesn't need to care
> about it.
> 
> Another idea could be to use contexts, e.g. guc or pm or whatever
> comes to mind, and the intel_gt_debugfs handles everything
> including subdirectories.
> 
>> I think extracting this generic helper to a common file, possibly as a follow-up
>> step, isn't a bad idea, also considering that there is at least 1 more
>> use-case in i915_debugfs_register(). Maybe we can generalize as something
>> like:
>>
>> struct i915_debugfs_files {
>> 	const char *name;
>> 	const struct file_operations *fops;
>> 	bool (*eval)(void *data);
>> }
>>
>> void i915_debugfs_register_files(struct dentry *root,
>> 				 const struct i915_debugfs_files *files,
>> 				 unsigned long count, void *data)
>> {
>>   	while (count--) {
>> 		umode_t mode = files->fops->write ? 0644 : 0444;
>> 		if (!files->eval || files->eval(data))
>>   			debugfs_create_file(files->name,
>> 					    mode, root, data,
>>   					    files->fops);
>> 	}
>> }
> 
> apart from the mode, isn't this the same as the latest patch you
> actually reviewed?
> 

Yes, but by adding the mode and making the naming generic we can re-use 
it outside the GT code, e.g. in i915_debugfs_connector_add() and to 
replace the loop in i915_debugfs_register(). I was reconnecting to 
Chris' proposal of having a common function in i915_debugfs_utils.c (or 
even just in i915_debugfs.c ?).

>> void i915_debugfs_register_files(struct dentry *root,
> 
> based on my proposal, root would point, in your case, to the
> "guc/" directory that will be created under the "gt/". NULL if
> you want the file to be created in the main "gt/" directory.
> 

If I'm understanding correctly, you're proposing to pass both struct 
intel_gt *gt and struct dentry *root, with the latter being set only if 
we want a folder different that gt/ ? What would that gain us compared 
to just passing the desired root every time like we currently do?

> While if we want to go by context, we could do something like:
> 
> struct i915_debugfs_files {
>        const char *name;
>        const struct file_operations *fops;
>        bool (*eval)(void *data);
>        enum intel_gt_context context;

This seems overkill, also because you'd have to save all the roots 
inside of the gt struct to allow accessing them from within the 
register_files function.

> }
> 
> and the gt handles everything.

Maybe I'm misunderstanding your proposal, but it feels like you're 
trying to find a use for a gt variable we don't really need just to keep 
this as a gt routine.

Daniele

> 
> Andi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-09 23:11             ` Daniele Ceraolo Spurio
@ 2020-03-11  8:24               ` Andi Shyti
  2020-03-12  0:37                 ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Shyti @ 2020-03-11  8:24 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio; +Cc: Intel GFX

Hi Daniele,

> > > > > > > Quoting Andi Shyti (2020-03-06 23:03:44)
> > > > > > > > -void debugfs_gt_register_files(struct intel_gt *gt,
> > > > > > > > -                              struct dentry *root,
> > > > > > > > -                              const struct debugfs_gt_file *files,
> > > > > > > > -                              unsigned long count)
> > > > > > > > +void intel_gt_debugfs_register_files(struct dentry *root,
> > > > > > > > +                                    const struct debugfs_gt_file *files,
> > > > > > > > +                                    unsigned long count, void *data)
> > > > > > > >    {
> > > > > > > >           while (count--) {
> > > > > > > > -               if (!files->eval || files->eval(gt))
> > > > > > > > +               if (!files->eval || files->eval(data))
> > > > > > > >                           debugfs_create_file(files->name,
> > > > > > > > -                                           0444, root, gt,
> > > > > > > > +                                           0444, root, data,
> > > > > > > >                                               files->fops);
> > > > > > > 
> > > > > > > And now we are not a intel_gt routine, you'll want to move again :)
> > > > > > > i915_debugfs_utils.c ? :)
> > > > > > 
> > > > > > Actually, this is what it came to and this was the first
> > > > > > discussion I had with Daniele and that's also why I was loyal to
> > > > > > th "_gt_" wrappers until the end. But I had to agree that this
> > > > > > was becoming more a limitation.
> > > > > > 
> > > > > > The biggest difference left, which by the way is the real
> > > > > > distinguishing factor other than the *gt pointer, is that we
> > > > > > create files under gt directory, instead of having the root
> > > > > > imposed by the drm (even though the caller can eventually choose
> > > > > > different roots).
> > > > > > 
> > > > > > We could perhaps store the root pointer in the intel_gt
> > > > > > structure so that this function stays de facto an intel_gt
> > > > > > routine and the caller doesn't need to care where the files will
> > > > > > be generated. This is what we planned to do with sysfs as well.
> > > > > > 
> > > > > > What do you think?
> > > > > 
> > > > > I thought we were passing along the root. If not I think we should, more
> > > > > of a debugfs constructor context?
> > > > 
> > > > What do you mean with debugfs constructor context? Is it a
> > > > gt->debugfs_root pointer like the gt->sysfs_root?
> > > > 
> > 
> > > Getting the root pointer internally from gt wouldn't work well for
> > > subfolders, like the gt/uc/ folder I want to add for GuC/HuC files.
> > 
> > this was not my idea, actually I was thinking the opposite.
> > 
> > When in this case you call "intel_gt_debugfs_register_files", you
> > would provide "gt" pointer where the funcion extracts and handles
> > by its own the debugfs_root. The caller doesn't need to care
> > about it.
> > 
> > Another idea could be to use contexts, e.g. guc or pm or whatever
> > comes to mind, and the intel_gt_debugfs handles everything
> > including subdirectories.
> > 
> > > I think extracting this generic helper to a common file, possibly as a follow-up
> > > step, isn't a bad idea, also considering that there is at least 1 more
> > > use-case in i915_debugfs_register(). Maybe we can generalize as something
> > > like:
> > > 
> > > struct i915_debugfs_files {
> > > 	const char *name;
> > > 	const struct file_operations *fops;
> > > 	bool (*eval)(void *data);
> > > }
> > > 
> > > void i915_debugfs_register_files(struct dentry *root,
> > > 				 const struct i915_debugfs_files *files,
> > > 				 unsigned long count, void *data)
> > > {
> > >   	while (count--) {
> > > 		umode_t mode = files->fops->write ? 0644 : 0444;
> > > 		if (!files->eval || files->eval(data))
> > >   			debugfs_create_file(files->name,
> > > 					    mode, root, data,
> > >   					    files->fops);
> > > 	}
> > > }
> > 
> > apart from the mode, isn't this the same as the latest patch you
> > actually reviewed?
> > 
> 
> Yes, but by adding the mode and making the naming generic we can re-use it
> outside the GT code, e.g. in i915_debugfs_connector_add() and to replace the
> loop in i915_debugfs_register(). I was reconnecting to Chris' proposal of
> having a common function in i915_debugfs_utils.c (or even just in
> i915_debugfs.c ?).

that's where we are coming from, we just moved this in :)

> > > void i915_debugfs_register_files(struct dentry *root,
> > 
> > based on my proposal, root would point, in your case, to the
> > "guc/" directory that will be created under the "gt/". NULL if
> > you want the file to be created in the main "gt/" directory.
> > 
> 
> If I'm understanding correctly, you're proposing to pass both struct
> intel_gt *gt and struct dentry *root, with the latter being set only if we
> want a folder different that gt/ ? What would that gain us compared to just
> passing the desired root every time like we currently do?
> 
> > While if we want to go by context, we could do something like:
> > 
> > struct i915_debugfs_files {
> >        const char *name;
> >        const struct file_operations *fops;
> >        bool (*eval)(void *data);
> >        enum intel_gt_context context;
> 
> This seems overkill, also because you'd have to save all the roots inside of
> the gt struct to allow accessing them from within the register_files
> function.
> 
> > }
> > 
> > and the gt handles everything.
> 
> Maybe I'm misunderstanding your proposal, but it feels like you're trying to
> find a use for a gt variable we don't really need just to keep this as a gt
> routine.

I'm just thinking aloud here also to avoid moving things back and
forth so easily.

I don't want to overdo things, but the way I see it, and the way
it was thought originally, is to have a hierarchical organization
of the debugfs, so taht "uc" and other gt systems cannot create
files above gt.

This patch is not just becoming gt independent, but it's i915
independent and, at this point, we can send it directly under
inode.c.

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer
  2020-03-11  8:24               ` Andi Shyti
@ 2020-03-12  0:37                 ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 13+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-03-12  0:37 UTC (permalink / raw)
  To: Andi Shyti; +Cc: Intel GFX



On 3/11/20 1:24 AM, Andi Shyti wrote:
> Hi Daniele,
> 
>>>>>>>> Quoting Andi Shyti (2020-03-06 23:03:44)
>>>>>>>>> -void debugfs_gt_register_files(struct intel_gt *gt,
>>>>>>>>> -                              struct dentry *root,
>>>>>>>>> -                              const struct debugfs_gt_file *files,
>>>>>>>>> -                              unsigned long count)
>>>>>>>>> +void intel_gt_debugfs_register_files(struct dentry *root,
>>>>>>>>> +                                    const struct debugfs_gt_file *files,
>>>>>>>>> +                                    unsigned long count, void *data)
>>>>>>>>>     {
>>>>>>>>>            while (count--) {
>>>>>>>>> -               if (!files->eval || files->eval(gt))
>>>>>>>>> +               if (!files->eval || files->eval(data))
>>>>>>>>>                            debugfs_create_file(files->name,
>>>>>>>>> -                                           0444, root, gt,
>>>>>>>>> +                                           0444, root, data,
>>>>>>>>>                                                files->fops);
>>>>>>>>
>>>>>>>> And now we are not a intel_gt routine, you'll want to move again :)
>>>>>>>> i915_debugfs_utils.c ? :)
>>>>>>>
>>>>>>> Actually, this is what it came to and this was the first
>>>>>>> discussion I had with Daniele and that's also why I was loyal to
>>>>>>> th "_gt_" wrappers until the end. But I had to agree that this
>>>>>>> was becoming more a limitation.
>>>>>>>
>>>>>>> The biggest difference left, which by the way is the real
>>>>>>> distinguishing factor other than the *gt pointer, is that we
>>>>>>> create files under gt directory, instead of having the root
>>>>>>> imposed by the drm (even though the caller can eventually choose
>>>>>>> different roots).
>>>>>>>
>>>>>>> We could perhaps store the root pointer in the intel_gt
>>>>>>> structure so that this function stays de facto an intel_gt
>>>>>>> routine and the caller doesn't need to care where the files will
>>>>>>> be generated. This is what we planned to do with sysfs as well.
>>>>>>>
>>>>>>> What do you think?
>>>>>>
>>>>>> I thought we were passing along the root. If not I think we should, more
>>>>>> of a debugfs constructor context?
>>>>>
>>>>> What do you mean with debugfs constructor context? Is it a
>>>>> gt->debugfs_root pointer like the gt->sysfs_root?
>>>>>
>>>
>>>> Getting the root pointer internally from gt wouldn't work well for
>>>> subfolders, like the gt/uc/ folder I want to add for GuC/HuC files.
>>>
>>> this was not my idea, actually I was thinking the opposite.
>>>
>>> When in this case you call "intel_gt_debugfs_register_files", you
>>> would provide "gt" pointer where the funcion extracts and handles
>>> by its own the debugfs_root. The caller doesn't need to care
>>> about it.
>>>
>>> Another idea could be to use contexts, e.g. guc or pm or whatever
>>> comes to mind, and the intel_gt_debugfs handles everything
>>> including subdirectories.
>>>
>>>> I think extracting this generic helper to a common file, possibly as a follow-up
>>>> step, isn't a bad idea, also considering that there is at least 1 more
>>>> use-case in i915_debugfs_register(). Maybe we can generalize as something
>>>> like:
>>>>
>>>> struct i915_debugfs_files {
>>>> 	const char *name;
>>>> 	const struct file_operations *fops;
>>>> 	bool (*eval)(void *data);
>>>> }
>>>>
>>>> void i915_debugfs_register_files(struct dentry *root,
>>>> 				 const struct i915_debugfs_files *files,
>>>> 				 unsigned long count, void *data)
>>>> {
>>>>    	while (count--) {
>>>> 		umode_t mode = files->fops->write ? 0644 : 0444;
>>>> 		if (!files->eval || files->eval(data))
>>>>    			debugfs_create_file(files->name,
>>>> 					    mode, root, data,
>>>>    					    files->fops);
>>>> 	}
>>>> }
>>>
>>> apart from the mode, isn't this the same as the latest patch you
>>> actually reviewed?
>>>
>>
>> Yes, but by adding the mode and making the naming generic we can re-use it
>> outside the GT code, e.g. in i915_debugfs_connector_add() and to replace the
>> loop in i915_debugfs_register(). I was reconnecting to Chris' proposal of
>> having a common function in i915_debugfs_utils.c (or even just in
>> i915_debugfs.c ?).
> 
> that's where we are coming from, we just moved this in :)
> 
>>>> void i915_debugfs_register_files(struct dentry *root,
>>>
>>> based on my proposal, root would point, in your case, to the
>>> "guc/" directory that will be created under the "gt/". NULL if
>>> you want the file to be created in the main "gt/" directory.
>>>
>>
>> If I'm understanding correctly, you're proposing to pass both struct
>> intel_gt *gt and struct dentry *root, with the latter being set only if we
>> want a folder different that gt/ ? What would that gain us compared to just
>> passing the desired root every time like we currently do?
>>
>>> While if we want to go by context, we could do something like:
>>>
>>> struct i915_debugfs_files {
>>>         const char *name;
>>>         const struct file_operations *fops;
>>>         bool (*eval)(void *data);
>>>         enum intel_gt_context context;
>>
>> This seems overkill, also because you'd have to save all the roots inside of
>> the gt struct to allow accessing them from within the register_files
>> function.
>>
>>> }
>>>
>>> and the gt handles everything.
>>
>> Maybe I'm misunderstanding your proposal, but it feels like you're trying to
>> find a use for a gt variable we don't really need just to keep this as a gt
>> routine.
> 
> I'm just thinking aloud here also to avoid moving things back and
> forth so easily.
> 

As I've mentioned above I wasn't suggesting moving this around now, just 
pointing out that there are other parts of the driver that open-code 
what we do in the gt and therefore there is space for code re-use in the 
future. I'm ok with merging this as-is in the meantime.

> I don't want to overdo things, but the way I see it, and the way
> it was thought originally, is to have a hierarchical organization
> of the debugfs, so taht "uc" and other gt systems cannot create
> files above gt.

But this hierarchy is currently implemented by passing the correct root 
pointer in and not via the gt pointer, so from the function POV you 
nothing stops you from passing in a root above gt/. The only thing the 
gt pointer is used for are data and eval().

> 
> This patch is not just becoming gt independent, but it's i915
> independent and, at this point, we can send it directly under
> inode.c.
> 

Fair point :)
I'm going to send my patches rebased on top of this one to better show 
what I mean when I say I don't use the gt variable locally at all. If we 
decide we still want it I'll throw in a guc/huc_to_gt to take it out.

Daniele

> Andi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2020-03-12  0:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 23:03 [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Andi Shyti
2020-03-07  2:26 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork
2020-03-07  2:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-03-07 12:07 ` [Intel-gfx] [PATCH v4] drm/i915/gt: allow setting generic data pointer Chris Wilson
2020-03-07 12:55   ` Andi Shyti
2020-03-07 17:35     ` Chris Wilson
2020-03-07 22:19       ` Andi Shyti
2020-03-09 22:05         ` Daniele Ceraolo Spurio
2020-03-09 22:38           ` Andi Shyti
2020-03-09 23:11             ` Daniele Ceraolo Spurio
2020-03-11  8:24               ` Andi Shyti
2020-03-12  0:37                 ` Daniele Ceraolo Spurio
2020-03-07 21:10 ` [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: allow setting generic data pointer (rev4) Patchwork

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