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

[Tvrtko Ursulin]

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.

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..

Regards,

Tvrtko

> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.c          |  2 --
>   drivers/gpu/drm/i915/i915_drv.h          | 13 ++++---------
>   drivers/gpu/drm/i915/i915_pci.c          |  1 -
>   drivers/gpu/drm/i915/intel_device_info.h |  2 --
>   4 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 305557e1942a..825b45cb3543 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -768,8 +768,6 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	memcpy(device_info, match_info, sizeof(*device_info));
>   	RUNTIME_INFO(i915)->device_id = pdev->device;
>   
> -	BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask));
> -
>   	return i915;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cb59eb0f6c5b..b984a340b21f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1241,6 +1241,10 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>    * GRAPHICS_VER(), MEDIA_VER and DISPLAY_VER()
>    */
>   #define INTEL_GEN(dev_priv)		(INTEL_INFO(dev_priv)->gen)
> +/*
> + * Deprecated: use IS_GRAPHICS_VER()
> + */
> +#define IS_GEN_RANGE(dev_priv, s, e)	IS_GRAPHICS_VER(dev_priv, (s), (e))
>   
>   #define GRAPHICS_VER(i915)		(INTEL_INFO(i915)->graphics_ver)
>   #define IS_GRAPHICS_VER(i915, from, until) \
> @@ -1257,15 +1261,6 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
>   #define REVID_FOREVER		0xff
>   #define INTEL_REVID(dev_priv)	(to_pci_dev((dev_priv)->drm.dev)->revision)
>   
> -#define INTEL_GEN_MASK(s, e) ( \
> -	BUILD_BUG_ON_ZERO(!__builtin_constant_p(s)) + \
> -	BUILD_BUG_ON_ZERO(!__builtin_constant_p(e)) + \
> -	GENMASK((e) - 1, (s) - 1))
> -
> -/* Returns true if Gen is in inclusive range [Start, End] */
> -#define IS_GEN_RANGE(dev_priv, s, e) \
> -	(!!(INTEL_INFO(dev_priv)->gen_mask & INTEL_GEN_MASK((s), (e))))
> -
>   #define IS_GEN(dev_priv, n) \
>   	(BUILD_BUG_ON_ZERO(!__builtin_constant_p(n)) + \
>   	 INTEL_INFO(dev_priv)->gen == (n))
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 97ab73276334..3b9cd1af0f28 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -37,7 +37,6 @@
>   
>   #define PLATFORM(x) .platform = (x)
>   #define GEN(x) \
> -	.gen_mask = BIT((x) - 1), \
>   	.gen = (x), \
>   	.graphics_ver = (x), \
>   	.media_ver = (x), \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 405883a8cc84..b8f7b996f140 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -160,8 +160,6 @@ enum intel_ppgtt_type {
>   	func(supports_tv);
>   
>   struct intel_device_info {
> -	u16 gen_mask;
> -
>   	u8 graphics_ver;
>   	u8 media_ver;
>   
>