Re: [PATCH v4 3/6] drm/i915/debugfs: add rcs topology entry

[Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>]

On 15/01/18 17:42, Tvrtko Ursulin wrote:
> On 15/01/2018 14:41, Lionel Landwerlin wrote:
> > While the end goal is to make this information available to userspace
> > through a new ioctl, there is no reason we can't display it in a human
> > readable fashion through debugfs.
> >
> > slice0: 3 subslice(s) (0x7):
> >     subslice0: 8 EUs (0xff)
> >     subslice1: 8 EUs (0xff)
> >     subslice2: 8 EUs (0xff)
> >     subslice3: 0 EUs (0x0)
> > slice1: 3 subslice(s) (0x7):
> >     subslice0: 8 EUs (0xff)
> >     subslice1: 8 EUs (0xff)
> >     subslice2: 8 EUs (0xff)
> >     subslice3: 0 EUs (0x0)
> > slice2: 3 subslice(s) (0x7):
> >     subslice0: 8 EUs (0xff)
> >     subslice1: 8 EUs (0xff)
> >     subslice2: 8 EUs (0xff)
> >     subslice3: 0 EUs (0x0)
> >
> > v2: Reformat debugfs printing (Tvrtko)
> >      Use the new EU mask helper (Tvrtko)
> >
> > v3: Move printing code to intel_device_info.c to be shared with error
> >      state (Michal)
> >
> > Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_debugfs.c      | 11 +++++++++++
> >   drivers/gpu/drm/i915/intel_device_info.c | 24 ++++++++++++++++++++++++
> >   drivers/gpu/drm/i915/intel_device_info.h |  2 ++
> >   3 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index e41a19b7d7bb..c3a44a54a0ba 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3166,6 +3166,16 @@ static int i915_engine_info(struct seq_file 
> > *m, void *unused)
> >       return 0;
> >   }
> >   +static int i915_rcs_topology(struct seq_file *m, void *unused)
> > +{
> > +    struct drm_i915_private *dev_priv = node_to_i915(m->private);
> > +    struct drm_printer p = drm_seq_file_printer(m);
> > +
> > + intel_device_info_dump_topology(&INTEL_INFO(dev_priv)->sseu, &p);
> > +
> > +    return 0;
> > +}
> > +
> >   static int i915_shrinker_info(struct seq_file *m, void *unused)
> >   {
> >       struct drm_i915_private *i915 = node_to_i915(m->private);
> > @@ -4696,6 +4706,7 @@ static const struct drm_info_list 
> > i915_debugfs_list[] = {
> >       {"i915_dmc_info", i915_dmc_info, 0},
> >       {"i915_display_info", i915_display_info, 0},
> >       {"i915_engine_info", i915_engine_info, 0},
> > +    {"i915_rcs_topology", i915_rcs_topology, 0},
> >       {"i915_shrinker_info", i915_shrinker_info, 0},
> >       {"i915_shared_dplls_info", i915_shared_dplls_info, 0},
> >       {"i915_dp_mst_info", i915_dp_mst_info, 0},
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index ed14994527fc..dcb5127a174c 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -123,6 +123,30 @@ void intel_device_info_dump(const struct 
> > intel_device_info *info,
> >       intel_device_info_dump_flags(info, p);
> >   }
> >   +void intel_device_info_dump_topology(const struct sseu_dev_info 
> > *sseu,
> > +                     struct drm_printer *p)
> > +{
> > +    int s, ss;
> > +
> > +    if (sseu->max_slices == 0) {
> > +        drm_printf(p, "Unavailable\n");
> > +        return;
> > +    }
> > +
> > +    for (s = 0; s < sseu->max_slices; s++) {
> > +        drm_printf(p, "slice%i: %u subslice(s) (0x%hhx):\n",
> > +               s, hweight8(sseu->subslice_mask[s]),
> > +               sseu->subslice_mask[s]);
> > +
> > +        for (ss = 0; ss < sseu->max_subslices; ss++) {
> > +            drm_printf(p, "\tsubslice%i: %u EUs (0x%hhx)",
> > +                   ss, hweight8(sseu_get_eus(sseu, s, ss)),
> > +                   sseu_get_eus(sseu, s, ss));
>
> If gcc cannot reduce to one call to sseu_get_eus consider caching in a 
> local here.
>
> > +        }
> > +    }
> > +}
> > +
> > +
> >   static u16 compute_eu_total(const struct sseu_dev_info *sseu)
> >   {
> >       u16 i, total = 0;
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.h 
> > b/drivers/gpu/drm/i915/intel_device_info.h
> > index 36e0df87862d..701888162944 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.h
> > +++ b/drivers/gpu/drm/i915/intel_device_info.h
> > @@ -220,5 +220,7 @@ void intel_device_info_dump_flags(const struct 
> > intel_device_info *info,
> >                     struct drm_printer *p);
> >   void intel_device_info_dump_runtime(const struct intel_device_info 
> > *info,
> >                       struct drm_printer *p);
> > +void intel_device_info_dump_topology(const struct sseu_dev_info *sseu,
> > +                     struct drm_printer *p);
> >     #endif
>
> I wished you used %d instead of %i but OK.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Regards,
>
> Tvrtko
Sure, will apply you nits. Thanks.

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