[Intel-gfx] [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 at 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 at 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 at intel.com>
> >> > Cc: Lionel Landwerlin <lionel.g.landwerlin at 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