Re: [Intel-gfx] [PATCH v2 08/12] drm/i915: finish removal of gen_mask

From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 08/12] drm/i915: finish removal of gen_mask
Date: Wed, 14 Apr 2021 10:41:45 -0700	[thread overview]
Message-ID: <20210414174145.v45vrgt3qsv7amiv@ldmartin-desk2> (raw)
In-Reply-To: <2216c8a6-003d-5240-d983-8c162872fd66@linux.intel.com>

On Wed, Apr 14, 2021 at 12:38:44PM +0100, Tvrtko Ursulin wrote:
>
>On 13/04/2021 06:09, Lucas De Marchi wrote:
>>Now that it's not used anywhere, remove it from struct
>>intel_device_info. To allow a period in which code will be converted to
>>the new macro, keep IS_GEN_RANGE() around, just redefining it to use
>>the new fields. The size advantage from IS_GEN_RANGE() using a mask is
>>not that big as it has pretty limited use througout the driver:
>>
>>    text    data     bss     dec     hex filename
>>2758497   95965    6496 2860958  2ba79e drivers/gpu/drm/i915/i915.ko.old
>>2758586   95953    6496 2861035  2ba7eb drivers/gpu/drm/i915/i915.ko.new
>
>This delta refers to this patch - I mean this point in the series? 
>Asking because it may not be 100% representative since some of the 
>previous patches have already removed some gen mask usages.

yes, it doesn't consider the other patches. These numbers are also for
v1, not v2, as I didn't update the commit mesage.

I don't think the numbers will be too different though.

>
>While I am here, I am a bit fond of the mask approach and wonder if 
>using it for all (gt/media/whatelse) new fields would still make 
>sense.
>
>Presence of the range check helpers suggests that it might, but I 
>haven't looked at how prevalent their usage ends up after the series 
>is done. So just in principle, I don't see why not still go with masks 
>since that guarantees elegant check at each range check site. It would 
>be all hidden in the macro implementation so easy.
>
>Also for historical reference, another reason why I went for masks 
>everywhere approach is that at some point we had a feature request to 
>allow compiling out platforms/gens. I *think* that was much easier to 
>do with masking and in experiments back then I was able for instance 
>to build just for Gen9+ and drop like 30% of the binary size.
>
>Oh I found the branch now.. The reason for IS_GEN(p, v) was also in 
>that series. I don't know if I ever RFC-ed or trybotted it.. google 
>suggests no and I neither can find it in my mailboxes. I could send 
>out the old patches for reference? But to be honest I have no idea if 
>this feature request (targeted driver builds) will ever resurface..

At the time I also liked having the macros. Looking back and checking if
we really took advantage of it, I lean towards a "no". Even when and if
we are interested in compiling out some platforms, I think a better
code split would be deserved rather relying on this.

Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx