[PATCH] drm/i915: Skip logging impossible slices

[PATCH] drm/i915: Skip logging impossible slices

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
printing impossible and empty slices for a platform.

Also compact slice total and slice mask into one log line.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 4babfc6ee45b..68aa9746d0e1 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
 {
 	int s;
 
-	drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
-	drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
+	drm_printf(p, "slice total: %u, mask=%04x\n",
+		   hweight8(sseu->slice_mask), sseu->slice_mask);
 	drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
-	for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
-		drm_printf(p, "slice%d %u subslices mask=%04x\n",
+	for (s = 0; s < sseu->max_slices; s++) {
+		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
 			   s, hweight8(sseu->subslice_mask[s]),
 			   sseu->subslice_mask[s]);
 	}
-- 
2.14.1

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

Re: [PATCH] drm/i915: Skip logging impossible slices

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
> printing impossible and empty slices for a platform.
> 
> Also compact slice total and slice mask into one log line.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 4babfc6ee45b..68aa9746d0e1 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>  {
>         int s;
>  
> -       drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
> -       drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
> +       drm_printf(p, "slice total: %u, mask=%04x\n",
> +                  hweight8(sseu->slice_mask), sseu->slice_mask);
>         drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
> -       for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
> -               drm_printf(p, "slice%d %u subslices mask=%04x\n",
> +       for (s = 0; s < sseu->max_slices; s++) {
> +               drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
>                            s, hweight8(sseu->subslice_mask[s]),
>                            sseu->subslice_mask[s]);

Just idly testing the waters...

In yaml, this would be
  "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"

How do we feel about that in our debug output? And gradually use that
style by default? (I'm planning on converting the error state
wholesale...)
-chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Skip logging impossible slices

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

On 21/03/18 10:32, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
> printing impossible and empty slices for a platform.
>
> Also compact slice total and slice mask into one log line.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 4babfc6ee45b..68aa9746d0e1 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>   {
>   	int s;
>   
> -	drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
> -	drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
> +	drm_printf(p, "slice total: %u, mask=%04x\n",
> +		   hweight8(sseu->slice_mask), sseu->slice_mask);
>   	drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
> -	for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
> -		drm_printf(p, "slice%d %u subslices mask=%04x\n",
> +	for (s = 0; s < sseu->max_slices; s++) {
> +		drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
>   			   s, hweight8(sseu->subslice_mask[s]),
>   			   sseu->subslice_mask[s]);
>   	}


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

Re: [PATCH] drm/i915: Skip logging impossible slices

[Chris Wilson]

Quoting Chris Wilson (2018-03-21 10:41:37)
> Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
> > printing impossible and empty slices for a platform.
> > 
> > Also compact slice total and slice mask into one log line.
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 4babfc6ee45b..68aa9746d0e1 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
> >  {
> >         int s;
> >  
> > -       drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
> > -       drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
> > +       drm_printf(p, "slice total: %u, mask=%04x\n",
> > +                  hweight8(sseu->slice_mask), sseu->slice_mask);
> >         drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
> > -       for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
> > -               drm_printf(p, "slice%d %u subslices mask=%04x\n",
> > +       for (s = 0; s < sseu->max_slices; s++) {
> > +               drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
> >                            s, hweight8(sseu->subslice_mask[s]),
> >                            sseu->subslice_mask[s]);
> 
> Just idly testing the waters...
> 
> In yaml, this would be
>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"

Or if we keep the node name the same for easier parsing:

	"<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Skip logging impossible slices

[Patchwork]

== Series Details ==

Series: drm/i915: Skip logging impossible slices
URL   : https://patchwork.freedesktop.org/series/40367/
State : success

== Summary ==

Series 40367v1 drm/i915: Skip logging impossible slices
https://patchwork.freedesktop.org/api/1.0/series/40367/revisions/1/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (fi-cfl-s2) fdo#105641
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#105641 https://bugs.freedesktop.org/show_bug.cgi?id=105641
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:433s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:379s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:541s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:510s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:509s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:514s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:508s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:409s
fi-cfl-s2        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:580s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:511s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:523s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:413s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:285  pass:224  dwarn:0   dfail:0   fail:1   skip:60  time:419s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:469s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:432s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:652s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:441s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:529s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:506s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:429s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:586s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:399s

9d737cebc219c821989021a3115424165ff7b052 drm-tip: 2018y-03m-20d-14h-56m-05s UTC integration manifest
1787d01b7782 drm/i915: Skip logging impossible slices

== Logs ==

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

Re: [PATCH] drm/i915: Skip logging impossible slices

[Jani Nikula]

On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Chris Wilson (2018-03-21 10:41:37)
>> Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
>> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > 
>> > Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
>> > printing impossible and empty slices for a platform.
>> > 
>> > Also compact slice total and slice mask into one log line.
>> > 
>> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> > index 4babfc6ee45b..68aa9746d0e1 100644
>> > --- a/drivers/gpu/drm/i915/intel_device_info.c
>> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> > @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>> >  {
>> >         int s;
>> >  
>> > -       drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
>> > -       drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
>> > +       drm_printf(p, "slice total: %u, mask=%04x\n",
>> > +                  hweight8(sseu->slice_mask), sseu->slice_mask);
>> >         drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
>> > -       for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
>> > -               drm_printf(p, "slice%d %u subslices mask=%04x\n",
>> > +       for (s = 0; s < sseu->max_slices; s++) {
>> > +               drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
>> >                            s, hweight8(sseu->subslice_mask[s]),
>> >                            sseu->subslice_mask[s]);
>> 
>> Just idly testing the waters...
>> 
>> In yaml, this would be
>>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"
>
> Or if we keep the node name the same for easier parsing:
>
> 	"<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"

I'm not against doing this, especially for gpu dumps.

Wouldn't json be easier to generate and parse? Or do you prefer the
slightly better human readability of yaml?

I think it would be pretty straighforward to write drm printer helpers
for printing valid json without having to actually manually print the
colons and braces etc. And the struct drm_printer could even have checks
for ensuring you don't burp verbatim stuff to a printer that's supposed
to be json.

Any considerations for the transition? Massive wholesale patch bomb
conversion? Yikes.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Skip logging impossible slices

[Chris Wilson]

Quoting Jani Nikula (2018-03-21 11:47:06)
> 
> On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Chris Wilson (2018-03-21 10:41:37)
> >> Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
> >> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> > 
> >> > Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
> >> > printing impossible and empty slices for a platform.
> >> > 
> >> > Also compact slice total and slice mask into one log line.
> >> > 
> >> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
> >> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >> > 
> >> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> >> > index 4babfc6ee45b..68aa9746d0e1 100644
> >> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> >> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> >> > @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
> >> >  {
> >> >         int s;
> >> >  
> >> > -       drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
> >> > -       drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
> >> > +       drm_printf(p, "slice total: %u, mask=%04x\n",
> >> > +                  hweight8(sseu->slice_mask), sseu->slice_mask);
> >> >         drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
> >> > -       for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
> >> > -               drm_printf(p, "slice%d %u subslices mask=%04x\n",
> >> > +       for (s = 0; s < sseu->max_slices; s++) {
> >> > +               drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
> >> >                            s, hweight8(sseu->subslice_mask[s]),
> >> >                            sseu->subslice_mask[s]);
> >> 
> >> Just idly testing the waters...
> >> 
> >> In yaml, this would be
> >>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"
> >
> > Or if we keep the node name the same for easier parsing:
> >
> >       "<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"
> 
> I'm not against doing this, especially for gpu dumps.
> 
> Wouldn't json be easier to generate and parse? Or do you prefer the
> slightly better human readability of yaml?

I think for any of the debug output preferring to keep it as readable as
possible is essential. libyaml isn't that hard to use, even for a
beginner like myself.

> I think it would be pretty straighforward to write drm printer helpers
> for printing valid json without having to actually manually print the
> colons and braces etc. And the struct drm_printer could even have checks
> for ensuring you don't burp verbatim stuff to a printer that's supposed
> to be json.

About the biggest challenge is tracking indent; which drm_printer
already does iirc. Still, I think we want to move this into lib/
 
> Any considerations for the transition? Massive wholesale patch bomb
> conversion? Yikes.

I think it's only worth converting bits and pieces that we are trying to
parse. So quite a few debugfs are candidates, and the error-state being
a prime example as we want to make it more amenable and flexible for
future post-mortem capture depending on what userspace needs. (I might
even go as far as all future debugfs should come with a parser for igt.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Skip logging impossible slices

[Jani Nikula]

On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2018-03-21 11:47:06)
>> 
>> On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > Quoting Chris Wilson (2018-03-21 10:41:37)
>> >> Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
>> >> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> >> > 
>> >> > Log up to sseu->max_slices instead basing on ARRAY_SIZE since to avoid
>> >> > printing impossible and empty slices for a platform.
>> >> > 
>> >> > Also compact slice total and slice mask into one log line.
>> >> > 
>> >> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> >> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/intel_device_info.c | 8 ++++----
>> >> >  1 file changed, 4 insertions(+), 4 deletions(-)
>> >> > 
>> >> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> >> > index 4babfc6ee45b..68aa9746d0e1 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_device_info.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> >> > @@ -83,11 +83,11 @@ static void sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p)
>> >> >  {
>> >> >         int s;
>> >> >  
>> >> > -       drm_printf(p, "slice mask: %04x\n", sseu->slice_mask);
>> >> > -       drm_printf(p, "slice total: %u\n", hweight8(sseu->slice_mask));
>> >> > +       drm_printf(p, "slice total: %u, mask=%04x\n",
>> >> > +                  hweight8(sseu->slice_mask), sseu->slice_mask);
>> >> >         drm_printf(p, "subslice total: %u\n", sseu_subslice_total(sseu));
>> >> > -       for (s = 0; s < ARRAY_SIZE(sseu->subslice_mask); s++) {
>> >> > -               drm_printf(p, "slice%d %u subslices mask=%04x\n",
>> >> > +       for (s = 0; s < sseu->max_slices; s++) {
>> >> > +               drm_printf(p, "slice%d: %u subslices, mask=%04x\n",
>> >> >                            s, hweight8(sseu->subslice_mask[s]),
>> >> >                            sseu->subslice_mask[s]);
>> >> 
>> >> Just idly testing the waters...
>> >> 
>> >> In yaml, this would be
>> >>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"
>> >
>> > Or if we keep the node name the same for easier parsing:
>> >
>> >       "<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"
>> 
>> I'm not against doing this, especially for gpu dumps.
>> 
>> Wouldn't json be easier to generate and parse? Or do you prefer the
>> slightly better human readability of yaml?
>
> I think for any of the debug output preferring to keep it as readable as
> possible is essential. libyaml isn't that hard to use, even for a
> beginner like myself.

Fair enough; I'm only familiar with json, and that's my natural reason
to lean towards it.

>> I think it would be pretty straighforward to write drm printer helpers
>> for printing valid json without having to actually manually print the
>> colons and braces etc. And the struct drm_printer could even have checks
>> for ensuring you don't burp verbatim stuff to a printer that's supposed
>> to be json.
>
> About the biggest challenge is tracking indent; which drm_printer
> already does iirc. Still, I think we want to move this into lib/

I agree having this in lib is preferrable. But apparently that requires
moving the whole abstracted drm_printer style printing there first, and
then hoping to get yaml/json added on top. There's bound to be some
friction there.

>> Any considerations for the transition? Massive wholesale patch bomb
>> conversion? Yikes.
>
> I think it's only worth converting bits and pieces that we are trying to
> parse. So quite a few debugfs are candidates, and the error-state being
> a prime example as we want to make it more amenable and flexible for
> future post-mortem capture depending on what userspace needs. (I might
> even go as far as all future debugfs should come with a parser for igt.)

Oh, I did mean mass conversion on a per-debugfs-file basis, i.e. error
state in one go. But I guess there's no way around that, anything else
is just pain spread over a longer period.

I'd definitely go as far as saying any debugfs file that's supposed to
be in a serialized format should have a corresponding parser in igt, and
written *before* merging the kernel change.

There's also some discipline required wrt backward/forward compatibility
to actually reap the benefits of the format. Can't just change the
contents nilly willy with that either. It's not a magic bullet.


BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915: Skip logging impossible slices

[Patchwork]

== Series Details ==

Series: drm/i915: Skip logging impossible slices
URL   : https://patchwork.freedesktop.org/series/40367/
State : warning

== Summary ==

---- Possible new issues:

Test kms_setmode:
        Subgroup clone-exclusive-crtc:
                pass       -> DMESG-WARN (shard-hsw)

---- Known issues:

Test kms_vblank:
        Subgroup pipe-b-ts-continuation-dpms-suspend:
                incomplete -> PASS       (shard-hsw) fdo#105054

fdo#105054 https://bugs.freedesktop.org/show_bug.cgi?id=105054

shard-apl        total:3478 pass:1815 dwarn:1   dfail:0   fail:6   skip:1655 time:13139s
shard-hsw        total:3478 pass:1767 dwarn:2   dfail:0   fail:1   skip:1707 time:11858s
shard-snb        total:3478 pass:1358 dwarn:1   dfail:0   fail:2   skip:2117 time:7304s
Blacklisted hosts:
shard-kbl        total:3478 pass:1939 dwarn:1   dfail:0   fail:10  skip:1528 time:9952s

== Logs ==

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

Re: [PATCH] drm/i915: Skip logging impossible slices

[Joonas Lahtinen]

Quoting Jani Nikula (2018-03-21 14:16:37)
> On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Quoting Jani Nikula (2018-03-21 11:47:06)
> >> 
> >> > Quoting Chris Wilson (2018-03-21 10:41:37)
> >> >> 
> >> >> Just idly testing the waters...
> >> >> 
> >> >> In yaml, this would be
> >> >>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"
> >> >
> >> > Or if we keep the node name the same for easier parsing:
> >> >
> >> >       "<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"
> >> 
> >> I'm not against doing this, especially for gpu dumps.
> >> 
> >> Wouldn't json be easier to generate and parse? Or do you prefer the
> >> slightly better human readability of yaml?
> >
> > I think for any of the debug output preferring to keep it as readable as
> > possible is essential. libyaml isn't that hard to use, even for a
> > beginner like myself.
> 
> Fair enough; I'm only familiar with json, and that's my natural reason
> to lean towards it.

JSON gets irritating to look at by plain eye. So I would agree that YAML
would serve better when developers are likely to scroll through the file
contents manually, too.

Then some more random thoughts:

This discussion will of course bring closer the can of forms that when
we've formalized the debugfs format, does somebody expect it to stay
more stable than not?

Having a counterpart in IGT and making that always match kernel, would
be the obvious way to keep things sane.

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

Re: [PATCH] drm/i915: Skip logging impossible slices

[Chris Wilson]

Quoting Joonas Lahtinen (2018-03-23 12:29:54)
> Quoting Jani Nikula (2018-03-21 14:16:37)
> > On Wed, 21 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > Quoting Jani Nikula (2018-03-21 11:47:06)
> > >> 
> > >> > Quoting Chris Wilson (2018-03-21 10:41:37)
> > >> >> 
> > >> >> Just idly testing the waters...
> > >> >> 
> > >> >> In yaml, this would be
> > >> >>   "<indent>- slice%d: { subslices: %u, mask: 0x%04x }\n"
> > >> >
> > >> > Or if we keep the node name the same for easier parsing:
> > >> >
> > >> >       "<indent>- slice: { id:%u, subslices:%u, mask:0x%04x }\n"
> > >> 
> > >> I'm not against doing this, especially for gpu dumps.
> > >> 
> > >> Wouldn't json be easier to generate and parse? Or do you prefer the
> > >> slightly better human readability of yaml?
> > >
> > > I think for any of the debug output preferring to keep it as readable as
> > > possible is essential. libyaml isn't that hard to use, even for a
> > > beginner like myself.
> > 
> > Fair enough; I'm only familiar with json, and that's my natural reason
> > to lean towards it.
> 
> JSON gets irritating to look at by plain eye. So I would agree that YAML
> would serve better when developers are likely to scroll through the file
> contents manually, too.
> 
> Then some more random thoughts:
> 
> This discussion will of course bring closer the can of forms that when
> we've formalized the debugfs format, does somebody expect it to stay
> more stable than not?

The starting point is that we do want to transition to an
easier-to-extend file format for sysfs (the idea of it being
single-valued is long gone :) There having a stable interface is without
question, so getting the transition right is the challenge and then
maintaining the hierarchy such that it is backwards and forward
compatible with new content.

> Having a counterpart in IGT and making that always match kernel, would
> be the obvious way to keep things sane.

Imo, if we aren't parsing debugfs in igt, then they can be dead code
eliminated. So I expect yamlification to snow ball once we've got the
first few done and a good feel for how to design kernel/igt API to suite
us. Then it's just a matter of throwing each debugfs through a yamllint
:)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx