On Wed, 21 Mar 2018, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> Quoting Jani Nikula (2018-03-21 11:47:06)
>>
>> On Wed, 21 Mar 2018, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
>> > Quoting Chris Wilson (2018-03-21 10:41:37)
>> >> Quoting Tvrtko Ursulin (2018-03-21 10:32:28)
>> >> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>> >> >
>> >> > 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@xxxxxxxxx>
>> >> > Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
>> >> > ---
>> >> > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
