Re: [igt-dev] [PATCH i-g-t 8/8] gputop: Basic vendor agnostic GPU top tool

Rob Clark
2023-05-12T14:18:52Z

On Thu, Apr 6, 2023 at 7:33 AM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
>
> On 06/04/2023 15:21, Rob Clark wrote:
> > On Thu, Apr 6, 2023 at 4:08 AM Tvrtko Ursulin
> > <tvrtko.ursulin@linux.intel.com> wrote:
> >>
> >>
> >> On 05/04/2023 18:57, Rob Clark wrote:
> >>> On Tue, Jan 31, 2023 at 3:33 AM Tvrtko Ursulin
> >>> <tvrtko.ursulin@linux.intel.com> wrote:
> >>>>
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> Rudimentary vendor agnostic example of how lib_igt_drm_clients can be used
> >>>> to display a sorted by card and usage list of processes using GPUs.
> >>>>
> >>>> Borrows a bit of code from intel_gpu_top but for now omits the fancy
> >>>> features like interactive functionality, card selection, client
> >>>> aggregation, sort modes, JSON output and pretty engine names. Also no
> >>>> support for global GPU or system metrics.
> >>>>
> >>>> On the other hand it shows clients from all DRM cards which
> >>>> intel_gpu_top does not do.
> >>>>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> Cc: Rob Clark <robdclark@chromium.org>
> >>>> Cc: Christian König <ckoenig.leichtzumerken@gmail.com>
> >>>> Acked-by: Christian König <christian.koenig@amd.com>
> >>>
> >>> Reviewed-by: Rob Clark <robdclark@chromium.org>
> >>
> >> Presumably for 8/8 only?
> >>
> >> The rest of the series does not apply any more by now. I need to rebase..
> >
> > I didn't look closely at the rest of the series (was kinda assuming
> > that was mostly just moving things around).. but I see you rebased it
> > so I can take a look.
>
> There's a lot in there - first patch is extracting some code into a
> library, with the corresponding renames, but then there are six patches
> of tweaks and feature additions which finally make gputop possible.
>
> Hopefully you can penetrate the concepts. It was all at least Valgrind
> clean back in the day I first did it.
>

by now I've read (and rebased locally) the series, and even added a
couple things on top.. so r-b for the series, we should get this
landed

BR,
-R