Re: [PATCH i-g-t 4/8] tools/gputop/i915_gputop: add i915 PMU engine monitoring

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi, Krzysztof,
thanks for review.

On Mon Sep 28, 2026 at 1:01 PM CEST, Krzysztof Karas wrote:
> Hi Sebastian,
>
> The implementaion looks OK, I am wondering about the duplicated
> bits and whether it is worth it to make funcitons forcibly stand
> out from the Xe implementation, e.g. sometimes you have tiny bit
> of difference in formatting, or there is one more helper
> variable in one implementation, but not the other, etc.
>
> I'd say it would be worth considering extracting common parts
> into a shared file, but not necessarily in this patch series.
Agreed on not doing it in this series, and honestly, I'd lean towards
not doing it at all unless we actually get more backend. Each vendor
backend implements the same interface on its own terms, by design, so
each one can do things however makes sense for that hardware. Merging
the common bits now, with only two backends to generalize from, risks
guessing wrong about what's actually shared, and we'd
end up with an abstraction that doesn't fit either one cleanly.

It also helps that gputop is a small standalone tool, not core library
code, so a bit of duplication here is cheap. If another backend shows
up we can revisit this.


-- 
Best regards,
Sebastian