[Intel-gfx] [RFC] drm/i915: Eliminate devid sprinkle
Chris Wilson
chris at chris-wilson.co.uk
Thu Feb 22 08:24:51 UTC 2018
Quoting Tvrtko Ursulin (2018-02-22 08:09:07)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Introduce subplatform mask to eliminate throughout the code devid checking
> sprinkle, mostly courtesy of IS_*_UL[TX] macros.
>
> Subplatform mask initialization is moved either to static tables (Ironlake
> M) or runtime device info init (Pineview, Haswell, Broadwell, Skylake,
> Kabylake, Coffeelake and Cannonlake).
>
> text data bss dec hex filename
> 1673630 59691 5064 1738385 1a8691 i915.ko.0
> 1673214 59691 5064 1737969 1a84f1 i915.ko.1
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula at intel.com>
> ---
> @@ -2624,38 +2631,19 @@ intel_info(const struct drm_i915_private *dev_priv)
> #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)
Killme. :)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 298f8996cc54..f5c9d29a7471 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -111,10 +111,11 @@ void intel_device_info_dump(const struct intel_device_info *info,
> struct drm_i915_private *dev_priv =
> container_of(info, struct drm_i915_private, info);
>
> - drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s gen=%i\n",
> + drm_printf(p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=%x) gen=%i\n",
> INTEL_DEVID(dev_priv),
> INTEL_REVID(dev_priv),
> intel_platform_name(info->platform),
> + info->subplatform_mask,
> info->gen);
Worth it.
>
> intel_device_info_dump_flags(info, p);
> @@ -458,6 +459,62 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +static void intel_device_info_subplatform_init(struct intel_device_info *info)
> +{
> + struct drm_i915_private *i915 =
> + container_of(info, struct drm_i915_private, info);
> + u16 devid = INTEL_DEVID(i915);
> +
> + if (IS_PINEVIEW(i915)) {
> -#define IS_PINEVIEW_G(dev_priv) (INTEL_DEVID(dev_priv) == 0xa001)
> -#define IS_PINEVIEW_M(dev_priv) (INTEL_DEVID(dev_priv) == 0xa011)
> + if (devid == 0xa001)
> + info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_G;
> + else if (devid == 0xa011)
> + info->subplatform_mask = INTEL_SUBPLATFORM_PINEVIEW_M;
Ok. Do we use IS_PINEVIEW_G/M? Should just be IS_PINEVIEW() &&
IS_MOBILE() but I guess after this change, IS_PINEVIEW_G/M is even
easier.
> + } else if (IS_HASWELL(i915)) {
> + if ((devid & 0xFF00) == 0x0A00)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
> + /* ULX machines are also considered ULT. */
> + if (devid == 0x0A0E || devid == 0x0A1E)
> + info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;
> + } else if (IS_BROADWELL(i915)) {
> -#define IS_BDW_ULT(dev_priv) (IS_BROADWELL(dev_priv) && \
> - ((INTEL_DEVID(dev_priv) & 0xf) == 0x6 || \
> - (INTEL_DEVID(dev_priv) & 0xf) == 0xb || \
> - (INTEL_DEVID(dev_priv) & 0xf) == 0xe))
> + if ((devid & 0xf) == 0x6 ||
> + (devid & 0xf) == 0xb ||
> + (devid & 0xf) == 0xe)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
> -#define IS_BDW_ULX(dev_priv) (IS_BROADWELL(dev_priv) && \
> - (INTEL_DEVID(dev_priv) & 0xf) == 0xe)
> + /* ULX machines are also considered ULT. */
> + if ((devid & 0xf) == 0xe)
> + info->subplatform_mask |= INTEL_SUBPLATFORM_ULX;
Ok.
> + } else if (IS_SKYLAKE(i915)) {
> -#define IS_SKL_ULT(dev_priv) (INTEL_DEVID(dev_priv) == 0x1906 || \
> - INTEL_DEVID(dev_priv) == 0x1913 || \
> - INTEL_DEVID(dev_priv) == 0x1916 || \
> - INTEL_DEVID(dev_priv) == 0x1921 || \
> - INTEL_DEVID(dev_priv) == 0x1926)
> + if (devid == 0x1906 ||
> + devid == 0x1913 ||
> + devid == 0x1916 ||
> + devid == 0x1921 ||
> + devid == 0x1926)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
> -#define IS_SKL_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x190E || \
> - INTEL_DEVID(dev_priv) == 0x1915 || \
> - INTEL_DEVID(dev_priv) == 0x191E)
> + else if (devid == 0x190E ||
> + devid == 0x1915 ||
> + devid == 0x191E)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULX;
Ok.
> + } else if (IS_KABYLAKE(i915)) {
> -#define IS_KBL_ULT(dev_priv) (INTEL_DEVID(dev_priv) == 0x5906 || \
> - INTEL_DEVID(dev_priv) == 0x5913 || \
> - INTEL_DEVID(dev_priv) == 0x5916 || \
> - INTEL_DEVID(dev_priv) == 0x5921 || \
> - INTEL_DEVID(dev_priv) == 0x5926)
> + if (devid == 0x5906 ||
> + devid == 0x5913 ||
> + devid == 0x5916 ||
> + devid == 0x5921 ||
> + devid == 0x5926)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
> -#define IS_KBL_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x590E || \
> - INTEL_DEVID(dev_priv) == 0x5915 || \
> - INTEL_DEVID(dev_priv) == 0x591E)
> + else if (devid == 0x590E ||
> + devid == 0x5915 ||
> + devid == 0x591E)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULX;
Ok.
> + } else if (IS_COFFEELAKE(i915)) {
> -#define IS_CFL_ULT(dev_priv) (IS_COFFEELAKE(dev_priv) && \
> - (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)
> + if ((devid & 0x00F0) == 0x00A0)
> + info->subplatform_mask = INTEL_SUBPLATFORM_ULT;
Ok.
> + } else if (IS_CANNONLAKE(i915)) {
> -#define IS_CNL_WITH_PORT_F(dev_priv) (IS_CANNONLAKE(dev_priv) && \
> - (INTEL_DEVID(dev_priv) & 0x0004) == 0x0004)
> + if ((devid & 0x0004) == 0x0004)
> + info->subplatform_mask = INTEL_SUBPLATFORM_PORTF;
Odd, but ok. (Wondering if this needs it own bit. It doesn't, ok.)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 71fdfb0451ef..7b6211061fba 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -74,6 +74,20 @@ enum intel_platform {
> INTEL_MAX_PLATFORMS
> };
>
> +/* Subplatform flags share the same namespace per parent platform. */
> +
> +#define INTEL_SUBPLATFORM_BITS (2)
Enough space to do the same for GT (4 bits?) on top?
Looks good to me. I'm moaning at mkwrite_device_info() but that can
stand for now ;)
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris
More information about the Intel-gfx
mailing list