[PATCH i-g-t] gputop: Add memory only utilization type

[Christian Gmeiner]

Hi Tvrtko

> > From: Christian Gmeiner <cgmeiner at igalia.com>
> >
> > Add special handling for drivers that only provide memory utilization
> > fdinfo.
>
> To check I am following correctly - special in this context means avoid
> displaying empty fields, or avoid a crash, or?

- Avoid early return in print_client(..) -> no output show.
- Avoid a crash in strcmp(..).

>
> > Signed-off-by: Christian Gmeiner <cgmeiner at igalia.com>
> > ---
> >   tools/gputop.c | 20 ++++++++++++++++++++
> >   1 file changed, 20 insertions(+)
> >
> > diff --git a/tools/gputop.c b/tools/gputop.c
> > index 43b01f566..4135d84c1 100644
> > --- a/tools/gputop.c
> > +++ b/tools/gputop.c
> > @@ -35,6 +35,7 @@
> >   enum utilization_type {
> >       UTILIZATION_TYPE_ENGINE_TIME,
> >       UTILIZATION_TYPE_TOTAL_CYCLES,
> > +     UTILIZATION_TYPE_MEMORY_ONLY,
> >   };
> >
> >   static const char *bars[] = { " ", "▏", "▎", "▍", "▌", "▋", "▊", "▉", "█" };
> > @@ -141,6 +142,9 @@ engines_identical(const struct igt_drm_client *c,
> >           c->engines->max_engine_id != pc->engines->max_engine_id)
> >               return false;
> >
> > +     if (!c->engines->num_engines && !pc->engines->num_engines)
> > +             return true;
>
> strcmp() below crashes?
>

Correct.

> I would move the check you added to be first in this function. I think
> that makes most sense from the flow wise - checking it before max_engine_id.
>

Will do in v2.

> With this fixed what remains broken?
>

Show any output :)

> The loop in print_client() should ideally be skipped but even as is I
> think exits on the capacity check, no? At the moment it seems like this
> could be enough but maybe I am missing something.
>

You are correct - it works without the goto thing.

> Regards,
>
> Tvrtko
>
> > +
> >       for (i = 0; i <= c->engines->max_engine_id; i++)
> >               if (c->engines->capacity[i] != pc->engines->capacity[i] ||
> >                   !!c->engines->names[i] != !!pc->engines->names[i] ||
> > @@ -193,6 +197,8 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
> >               utilization_type = UTILIZATION_TYPE_TOTAL_CYCLES;
> >       else if (c->utilization_mask & IGT_DRM_CLIENT_UTILIZATION_ENGINE_TIME)
> >               utilization_type = UTILIZATION_TYPE_ENGINE_TIME;
> > +     else if (c->regions->num_regions)
> > +             utilization_type = UTILIZATION_TYPE_MEMORY_ONLY;
> >       else
> >               return 0;
> >
> > @@ -209,6 +215,13 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
> >              if (!c->total_total_cycles)
> >                      return 0;
> >              break;
> > +     case UTILIZATION_TYPE_MEMORY_ONLY:
> > +             for (sz = 0, i = 0; i <= c->regions->max_region_id; i++)
> > +                     sz += c->memory[i].total;
> > +
> > +             if (sz == 0)
> > +                     return 0;
> > +             break;
> >       }
> >
> >       /* Print header when moving to a different DRM card. */
> > @@ -234,6 +247,9 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
> >
> >       lines++;
> >
> > +     if (utilization_type == UTILIZATION_TYPE_MEMORY_ONLY)
> > +             goto print_name;
> > +
> >       for (i = 0; c->samples > 1 && i <= c->engines->max_engine_id; i++) {
> >               double pct;
> >
> > @@ -249,6 +265,9 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
> >                       pct = (double)c->utilization[i].delta_cycles / c->utilization[i].delta_total_cycles * 100 /
> >                               c->engines->capacity[i];
> >                       break;
> > +             case UTILIZATION_TYPE_MEMORY_ONLY:
> > +                     /* Nothing to do. */
> > +                     break;
> >               }
> >
> >               /*
> > @@ -262,6 +281,7 @@ print_client(struct igt_drm_client *c, struct igt_drm_client **prevc,
> >               len += *engine_w;
> >       }
> >
> > +print_name:
> >       printf(" %-*s\n", con_w - len - 1, c->print_name);
> >
> >       return lines;



-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy