Re: [PATCH 4/5] drm/i915: add helper IS_GEN_* macros

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

On 01/11/2018 08:35, Lucas De Marchi wrote:
> This will allow us to make all gen comparisons simple gen comparisons
> with the same kind of macro rather than resorting to a mix of IS_GEN(),
> INTEL_GEN() >=, >, <=, <.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8d22353a3da6..eb636dfe5228 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2370,6 +2370,11 @@ intel_info(const struct drm_i915_private *dev_priv)
>   #define IS_GEN(dev_priv, ...) \
>   	CONCATENATE(_IS_GEN_ARG, COUNT_ARGS(__VA_ARGS__))((dev_priv), ##__VA_ARGS__)
>   
> +#define IS_GEN_GT(dev_priv, g)	(g > (dev_priv)->info.gen)

Interesting :), I have at some point floated a similar patch and there 
was overwhelming dislike of the GT suffix, since we have GT2 GT3 etc.. 
as established acronyms for GPU configurations.

Furthermore, I am pretty sure I've done it using the gen bitmask so 
hypothetical checks like IS_GEN_LT(5) || IS_GEN_RANGE(8, 9) could still 
be compile time optimized to a single conditional. That would also now 
become more difficult due removal of GEN_FOREVER.

To summarise, I don't see a advantage of hiding this in macros if we are 
not getting the above benefit.

Regards,

Tvrtko

> +#define IS_GEN_GE(dev_priv, g)	(g >= (dev_priv)->info.gen)
> +#define IS_GEN_LT(dev_priv, g)	(g < (dev_priv)->info.gen)
> +#define IS_GEN_LE(dev_priv, g)	(g <= (dev_priv)->info.gen)
> +
>   /*
>    * Return true if revision is in range [since,until] inclusive.
>    *
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx