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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 08/12] drm/i915: finish removal of gen_mask
Date: Tue, 13 Apr 2021 12:40:15 +0300	[thread overview]
Message-ID: <87o8eih69s.fsf@intel.com> (raw)
In-Reply-To: <20210413051002.92589-9-lucas.demarchi@intel.com>

On Mon, 12 Apr 2021, Lucas De Marchi <lucas.demarchi@intel.com> 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
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@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()
> + */

Nitpick, I think this should also mention IS_MEDIA_VER() and
DISPLAY_VER() to not have people blindly use IS_GRAPHICS_VER().

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> +#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;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx