[Intel-gfx] [PATCH v3 2/3] drm/i915: replace IS_GEN<N> with IS_GEN(..., N)

[Tvrtko Ursulin]

On 07/12/2018 20:57, Lucas De Marchi wrote:
> On Fri, Dec 07, 2018 at 11:30:28AM +0000, Tvrtko Ursulin wrote:
>>
>> On 07/12/2018 01:17, Lucas De Marchi wrote:
>>> On Thu, Dec 6, 2018 at 4:37 AM Tvrtko Ursulin
>>> <tvrtko.ursulin at linux.intel.com> wrote:
>>>>
>>>>
>>>> On 06/12/2018 06:11, Lucas De Marchi wrote:
>>>>> Define IS_GEN() similarly to our IS_GEN_RANGE(). but use gen instead of
>>>>> gen_mask to do the comparison. Now callers can pass then gen as a parameter,
>>>>
>>>> Since you are calling it out here, I assume there is some good reason to
>>>> replace gen_mask with gen?
>>>
>>> Because in this version we don't have the commit removing gen from the
>>> device info.
>>
>> You had that? Totally don't remember.. what was the goal of that?
> 
> Derive the same info from mask. gen = ffs(gen_mask) + 1, or something
> like that.
> 
> Checking again I had actually removed only the macro INTEL_GEN, not the
> struct member. Probably because we use that than I thought we would.
> 
>>
>>> Checking gen instead of gen_mask is both simpler and generates smaller
>>> code (although
>>> the difference is negligible, ~100 bytes)
>>
>> Ok fair, and easy enough to change back once per SKU work rekindles.
> 
> why would you need to change it back for per-SKU work? The compiler
> won't do anything smarter because of using the bitfield (provided this
> series is applied, which already merges IS_GEN8() || IS_GEN9() and the
> like).

I think in my prototype I built a possible platform and gen mask from 
Kconfig options, and then applied that mask in the single __IS_GEN and 
__IS_PLATFORM macros, over the device and argument masks. That way DCE 
easily eliminates all unused branches. With the numerical gen it is 
still possible I suppose just possibly less elegant. So yeah, I think 
it's fine, was just wondering why since the commit message did not explain.

Regards,

Tvrtko