Re: [PATCH] drm/i915: Drop platform_mask

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 15/03/2019 07:27, Lucas De Marchi wrote:
> On Fri, Mar 15, 2019 at 12:13 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 15/03/2019 06:56, Tvrtko Ursulin wrote:
>>>
>>> On 15/03/2019 00:52, Chris Wilson wrote:
>>>> Quoting José Roberto de Souza (2019-03-15 00:42:35)
>>>>> We don't have any platform that is composed by 2 or more platforms so
>>>>> we don't need a mask, lets drop it and remove the actual limit of 32
>>>>> platforms.
>>>
>>> Platform mask was a nifty trick to compile tests like IS_SKYLAKE ||
>>> IS_BROADWELL etc into a single conditional.
>>>
>>>> gcc doesn't entirely agree, this is a net loss here (i.e. code size
>>>> increases).
>>>
>>> Perhaps the size re-gain of dropping the platform mask could be checked
>>> against the size gain of making the mask 64 bit.
> 
> current:
>    text       data        bss        dec        hex    filename
> 1836533      40454       4176    1881163     1cb44b
> drivers/gpu/drm/i915/i915.o.old
> 
> 64 bit bitmask:
> 1836757      40454       4176    1881387     1cb52b drivers/gpu/drm/i915/i915.o
> 
> no platform_mask:
> 1837591      40454       4176    1882221     1cb86d
> drivers/gpu/drm/i915/i915.o
> 
> 
> So current situation to "just use a number" we are talking about 1k
> here, which for me looks acceptable. Alternative is the 64-bit
> bitmask, with ~200 bytes.

Maybe yeah, but there was also the second part about per SKU builds.

Let me dig out the sub platform patch and modify it to split out the 
mask into two dwords and see how that would look.

Regards,

Tvrtko

> 
> Lucas De Marchi
> 
> 
> 
>>
>> One possible alternative could be splitting the 64-bit platform mask
>> into two 32-bit dwords. Like:
>>
>>     u32 platform_mask[2];
>>
>>     #define IS_PLATFORM(p) (platform_mask[p / 32] & BIT(p % 32))
>>
>> And a bit of jigging the enum space so that we don't end up with
>> something often tested together on the dword boundary.
>>
>> In fact I had a prototype many months ago which went a step further and
>> interleaved platform with gen mask, in some sized chunks, so even
>> IS_PLATFORM || IS_GEN checks could be merged. This included the
>> sub-platform thing as well with the ULT/ULX/LP stuff I think.. maybe I
>> need to dig this out to see how it worked.
>>
>> Regards,
>>
>> Tvrtko
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> --
> Lucas De Marchi
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx