Re: [PATCH 2/3] drm/i915: Generate all IS_<platform> macros

From: Jani Nikula <jani.nikula@intel.com>
To: Tvrtko Ursulin <tursulin@ursulin.net>, Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: Generate all IS_<platform> macros
Date: Thu, 08 Dec 2016 12:46:26 +0200	[thread overview]
Message-ID: <87inquwvel.fsf@intel.com> (raw)
In-Reply-To: <1481190599-1217-3-git-send-email-tvrtko.ursulin@linux.intel.com>

On Thu, 08 Dec 2016, Tvrtko Ursulin <tursulin@ursulin.net> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Instead of listing them individually we can generate them
> using the new i915_platforms.h header.
>
> Also convert them to a static inline function which
> interestingly makes the code smaller as well.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

NAK. Absolutely opposed to this.

A large part of my work involves digging through the source tree, and a
crucial part of that is looking up definitions and references, both for
macros and functions. Not having the macro/function definitions breaks
that workflow. Losing that, source code archeology gets *much*
harder. The losses are much greater than the gains.

BR,
Jani.

> ---
>  drivers/gpu/drm/i915/i915_drv.h | 39 ++++++++++++++++-----------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea06d3ff59da..fb8f4b7cd1ae 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2579,36 +2579,29 @@ intel_info(const struct drm_i915_private *dev_priv)
>  #define IS_REVID(p, since, until) \
>  	(INTEL_REVID(p) >= (since) && INTEL_REVID(p) <= (until))
>  
> -#define IS_I830(dev_priv)	((dev_priv)->info.platform == INTEL_I830)
> -#define IS_I845G(dev_priv)	((dev_priv)->info.platform == INTEL_I845G)
> -#define IS_I85X(dev_priv)	((dev_priv)->info.platform == INTEL_I85X)
> -#define IS_I865G(dev_priv)	((dev_priv)->info.platform == INTEL_I865G)
> -#define IS_I915G(dev_priv)	((dev_priv)->info.platform == INTEL_I915G)
> -#define IS_I915GM(dev_priv)	((dev_priv)->info.platform == INTEL_I915GM)
> -#define IS_I945G(dev_priv)	((dev_priv)->info.platform == INTEL_I945G)
> -#define IS_I945GM(dev_priv)	((dev_priv)->info.platform == INTEL_I945GM)
> -#define IS_I965G(dev_priv)	((dev_priv)->info.platform == INTEL_I965G)
> -#define IS_I965GM(dev_priv)	((dev_priv)->info.platform == INTEL_I965GM)
> -#define IS_G45(dev_priv)	((dev_priv)->info.platform == INTEL_G45)
> -#define IS_GM45(dev_priv)	((dev_priv)->info.platform == INTEL_GM45)
> +/*
> + * This block generates the IS_<platform> helper functions in the format of:
> + *
> + * static inline bool IS_SKYLAKE(const struct drm_i915_private *dev_priv);
> + * ...
> + *
> + * One for each platform listed in i915_platforms.h is generated.
> + */
> +#define i915_platform(name, id) \
> +static inline bool IS_##name(const struct drm_i915_private *dev_priv) \
> +{ \
> +	return (dev_priv)->info.platform == INTEL_##name; \
> +}
> +#include "i915_platforms.h"
> +#undef i915_platform
> +
>  #define IS_G4X(dev_priv)	(IS_G45(dev_priv) || IS_GM45(dev_priv))
>  #define IS_PINEVIEW_G(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa001)
>  #define IS_PINEVIEW_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0xa011)
> -#define IS_PINEVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_PINEVIEW)
> -#define IS_G33(dev_priv)	((dev_priv)->info.platform == INTEL_G33)
>  #define IS_IRONLAKE_M(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0046)
> -#define IS_IVYBRIDGE(dev_priv)	((dev_priv)->info.platform == INTEL_IVYBRIDGE)
>  #define IS_IVB_GT1(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0156 || \
>  				 INTEL_DEVID(dev_priv) == 0x0152 || \
>  				 INTEL_DEVID(dev_priv) == 0x015a)
> -#define IS_VALLEYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_VALLEYVIEW)
> -#define IS_CHERRYVIEW(dev_priv)	((dev_priv)->info.platform == INTEL_CHERRYVIEW)
> -#define IS_HASWELL(dev_priv)	((dev_priv)->info.platform == INTEL_HASWELL)
> -#define IS_BROADWELL(dev_priv)	((dev_priv)->info.platform == INTEL_BROADWELL)
> -#define IS_SKYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_SKYLAKE)
> -#define IS_BROXTON(dev_priv)	((dev_priv)->info.platform == INTEL_BROXTON)
> -#define IS_KABYLAKE(dev_priv)	((dev_priv)->info.platform == INTEL_KABYLAKE)
> -#define IS_GEMINILAKE(dev_priv)	((dev_priv)->info.platform == INTEL_GEMINILAKE)
>  #define IS_MOBILE(dev_priv)	((dev_priv)->info.is_mobile)
>  #define IS_HSW_EARLY_SDV(dev_priv) (IS_HASWELL(dev_priv) && \
>  				    (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0C00)

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