Re: [Intel-gfx] [PATCH 1/3] i915/perf: Store a mask of valid OA formats for a platform

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Umesh Nerlige Ramappa (2021-02-02 20:10:44)
> On Tue, Feb 02, 2021 at 08:24:15AM +0000, Chris Wilson wrote:
> >Ok, this looks as compact and readable as writing it as a bunch of
> >tables. I presume there's a reason you didn't just use generation rather
> >than platform.
> >
> >switch (gen) {
> >case 7:
> >       haswell();
> >       break;
> >case 8 .. 11:
> >       broadwell();
> >       break;
> >case 12:
> >       tigerlake();
> >       break;
> >}
> >if you wanted to stick with a switch rather than an if-else tree for the
> >ranges.
> 
> only haswell is supported on gen7 and gen12 may define new formats that 
> are platform specific.
> 
> How about a mix? -
> 
> if (gen == 7 && haswell)
>         haswell();
> else if (gen >= 8 && gen <= 11)
>         broadwell;
> else
>         gen12_formats();
> 
> gen12_formats can choose to use the switch if formats vary between 
> platforms.

I didn't mind the platform switch too much, so no need to change at the
moment. I just worry that it's more typing to maintain :)

What I thought you were going to do (from the subject) were tables with
a platform_mask for applicability,  but that I feell would be just as
much typing, now and in the future.


I thought support started at Haswell, so the other gen7 were not a
concern? But yes, if we look at how we end up doing it else where it's a
mix of gen and platform

if (gen >= 12)
	gen12_formats;
else if (gen >= 8)
	gen8_formats;
else if (IS_HSW)
	hsw_formats;
else
	MISSING_CASE(gen)

At the end of the day, you're the person who is typing this, so it's up
to you how much effort you want to spend now to save later. :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx