Re: [PATCH 5/5] drm/i915: Warn on obsolete revision checks

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 08 Jun 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> If we are doing revision checks against a preproduction
> range, when there is already a product, it is a sign
> that there is code to be removed.
>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_chipset.h | 30 +++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_chipset.h b/drivers/gpu/drm/i915/intel_chipset.h
> index 946c889c0118..bc9ff02dc8df 100644
> --- a/drivers/gpu/drm/i915/intel_chipset.h
> +++ b/drivers/gpu/drm/i915/intel_chipset.h
> @@ -131,6 +131,12 @@
>  #define IS_PREPRODUCTION_HW(dev_priv)   (INTEL_REVID(dev_priv) < FIRST_PRODUCT_REVID(INTEL_INFO(dev_priv)))
>  #define IS_PLATFORM_SUPPORT_ALPHA(intel_info) (FIRST_PRODUCT_REVID(intel_info) == PRODUCT_REVID_UNKNOWN)
>  
> +#define BUILD_BUG_ON_REVID_LT(revid, production_revid) ({ \
> +		BUILD_BUG_ON((production_revid) != PRODUCT_REVID_UNKNOWN && \
> +			     (revid) < (production_revid)); \
> +		1; \
> +	})
> +
>  #define SKL_REVID_A0		0x0
>  #define SKL_REVID_B0		0x1
>  #define SKL_REVID_C0		0x2
> @@ -141,7 +147,9 @@
>  #define SKL_REVID_PRODUCT	SKL_REVID_G0
>  #define SKL_REVID_H0		0x7
>  
> -#define IS_SKL_REVID(p, since, until) (IS_SKYLAKE(p) && IS_REVID(p, since, until))
> +#define IS_SKL_REVID(p, since, until) \
> +	(BUILD_BUG_ON_REVID_LT(until, SKL_REVID_PRODUCT) && \
> +	 IS_SKYLAKE(p) && IS_REVID(p, since, until))
>  
>  #define BXT_REVID_A0		0x0
>  #define BXT_REVID_A1		0x1
> @@ -151,7 +159,8 @@
>  #define BXT_REVID_PRODUCT	BXT_REVID_C0
>  
>  #define IS_BXT_REVID(dev_priv, since, until) \
> -	(IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until))
> +	(BUILD_BUG_ON_REVID_LT(until, BXT_REVID_PRODUCT) && \
> +	 IS_BROXTON(dev_priv) && IS_REVID(dev_priv, since, until))
>  
>  #define KBL_REVID_A0		0x0
>  #define KBL_REVID_B0		0x1
> @@ -161,29 +170,36 @@
>  #define KBL_REVID_E0		0x4
>  
>  #define IS_KBL_REVID(dev_priv, since, until) \
> -	(IS_KABYLAKE(dev_priv) && IS_REVID(dev_priv, since, until))
> +	(BUILD_BUG_ON_REVID_LT(until, KBL_REVID_PRODUCT) && \
> +	 IS_KABYLAKE(dev_priv) && IS_REVID(dev_priv, since, until))
>  
>  #define GLK_REVID_A0		0x0
>  #define GLK_REVID_A1		0x1
> +#define GLK_REVID_PRODUCT	PRODUCT_REVID_UNKNOWN
>  
> -#define IS_GLK_REVID(dev_priv, since, until) \
> -	(IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
> +#define IS_GLK_REVID(dev_priv, since, until)		    \
> +	(BUILD_BUG_ON_REVID_LT(until, GLK_REVID_PRODUCT) && \
> +	 IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
>  
>  #define CNL_REVID_A0		0x0
>  #define CNL_REVID_B0		0x1
>  #define CNL_REVID_C0		0x2
> +#define CNL_REVID_PRODUCT	PRODUCT_REVID_UNKNOWN
>  
>  #define IS_CNL_REVID(p, since, until) \
> -	(IS_CANNONLAKE(p) && IS_REVID(p, since, until))
> +	(BUILD_BUG_ON_REVID_LT(until, CNL_REVID_PRODUCT) && \
> +	 IS_CANNONLAKE(p) && IS_REVID(p, since, until))
>  
>  #define ICL_REVID_A0		0x0
>  #define ICL_REVID_A2		0x1
>  #define ICL_REVID_B0		0x3
>  #define ICL_REVID_B2		0x4
>  #define ICL_REVID_C0		0x5
> +#define ICL_REVID_PRODUCT	PRODUCT_REVID_UNKNOWN
>  
>  #define IS_ICL_REVID(p, since, until) \
> -	(IS_ICELAKE(p) && IS_REVID(p, since, until))
> +	(BUILD_BUG_ON_REVID_LT(until, ICL_REVID_PRODUCT) && \
> +	IS_ICELAKE(p) && IS_REVID(p, since, until))

The trouble with this is that it ties the macros *_REVID_PRODUCT to
having to get rid of all the pre-production revid checks. We'll
typically know the product revid *before* we can drop all the
checks. And we can't drop all the checks because there'll be plenty of
hardware to phase out and replace, and it doesn't happen overnight. But
we'd still like to start complaining about the pre-pro hardware use as
soon as possible.

BR,
Jani.

>  
>  /*
>   * The genX designation typically refers to the render engine, so render

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