On 21/11/2016 09:27, Jani Nikula wrote:
On Fri, 18 Nov 2016, Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
On 18/11/2016 14:20, Jani Nikula wrote:
The platform flags in device info are (mostly) mutually
exclusive. Replace the flags with an enum. Add the platform enum also
for platforms that previously didn't have a flag, and give them codename
logging in dmesg.
It just saddens me a bit that it prevents the compiler optimisation of
our IS_THIS || IS_THAT || IS_TAT ugliness. :I On the basis on that I
cannot quite make myself to support it, although it conceptually does
make more sense.
Personally I think making more sense conceptually trumps possible
compiler optimizations, especially when the code paths where this
actually matters are extremely rare. (Can you even point me at an
example where this makes a difference?)
Without thinking deeply, all instances like IS_HASWELL || IS_BROADWELL,
IS_SKYLAKE || IS_KABYLAKE, or IS_VALLEYVIEW || IS_CHERRYVIEW. Quick grep
shows that there is at least around 170 instances of those. There might
be more, I don't know. At least the ones I listed all currently
translate to a single conditional in the generated code.
I am not saying these are all on performance critical paths or anything,
just that on balance, for me the sweetness of the optimisation cancels
out the ugliness of the code (which is often unavoidable). Losing that
and ending up with compare ladders in the code would just be a bit sad.
That said, does doing something silly like this make a difference:
enum intel_platform {
INTEL_PLATFORM_UNINITIALIZED = 0,
INTEL_I830 = BIT(0),
INTEL_I845G = BIT(1),
INTEL_I85X = BIT(2),
INTEL_I865G = BIT(3),
INTEL_I915G = BIT(4),
INTEL_I915GM = BIT(5),
INTEL_I945G = BIT(6),
INTEL_I945GM = BIT(7),
INTEL_G33 = BIT(8),
INTEL_G4X = BIT(9),
INTEL_PINEVIEW = BIT(10),
INTEL_BROADWATER = BIT(11),
INTEL_CRESTLINE = BIT(12),
INTEL_IRONLAKE = BIT(13),
INTEL_SANDYBRIDGE = BIT(14),
INTEL_IVYBRIDGE = BIT(15),
INTEL_VALLEYVIEW = BIT(16),
INTEL_CHERRYVIEW = BIT(17),
INTEL_HASWELL = BIT(18),
INTEL_BROADWELL = BIT(19),
INTEL_SKYLAKE = BIT(20),
INTEL_BROXTON = BIT(21),
INTEL_KABYLAKE = BIT(22),
};
I could live with that. Probably best to compare the code size before
and after just to verify there are no surprises.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
