[Intel-gfx] [RFC PATCH] drm/i915: replace platform flags with a platform enum

[Ville Syrjälä]

On Fri, Nov 18, 2016 at 04:20:32PM +0200, Jani Nikula wrote:
> The platform flags in device info are (mostly) mutually
> exclusive. Replace the flags with an enum. Add the platform enum also
> for platforms that previously didn't have a flag, and give them codename
> logging in dmesg.
> 
> Pineview remains an exception, the platform being G33 for that.
> 
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> 
> ---
> 
> Untested TGIF patch. ;)
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |  1 +
>  drivers/gpu/drm/i915/i915_drv.h          | 76 +++++++++++++++++++-------------
>  drivers/gpu/drm/i915/i915_gpu_error.c    |  1 +
>  drivers/gpu/drm/i915/i915_pci.c          | 53 +++++++++++++---------
>  drivers/gpu/drm/i915/intel_device_info.c | 40 ++++++++++++++++-
>  5 files changed, 117 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b7f42c448a44..982f0bdb768f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -77,6 +77,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
>  	const struct intel_device_info *info = INTEL_INFO(dev_priv);
>  
>  	seq_printf(m, "gen: %d\n", INTEL_GEN(dev_priv));
> +	seq_printf(m, "platform: %s\n", intel_platform_name(info->platform));
>  	seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(dev_priv));
>  #define PRINT_FLAG(x)  seq_printf(m, #x ": %s\n", yesno(info->x))
>  	DEV_INFO_FOR_EACH_FLAG(PRINT_FLAG);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index be67aeece749..b8a1b66599dc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -659,24 +659,8 @@ struct intel_csr {
>  };
>  
>  #define DEV_INFO_FOR_EACH_FLAG(func) \
> -	/* Keep is_* in chronological order */ \
>  	func(is_mobile); \
> -	func(is_i85x); \
> -	func(is_i915g); \
> -	func(is_i945gm); \
> -	func(is_g33); \
> -	func(is_g4x); \
>  	func(is_pineview); \
> -	func(is_broadwater); \
> -	func(is_crestline); \
> -	func(is_ivybridge); \
> -	func(is_valleyview); \
> -	func(is_cherryview); \
> -	func(is_haswell); \
> -	func(is_broadwell); \
> -	func(is_skylake); \
> -	func(is_broxton); \
> -	func(is_kabylake); \
>  	func(is_alpha_support); \
>  	/* Keep has_* in alphabetical order */ \
>  	func(has_64bit_reloc); \
> @@ -726,6 +710,34 @@ static inline unsigned int sseu_subslice_total(const struct sseu_dev_info *sseu)
>  	return hweight8(sseu->slice_mask) * hweight8(sseu->subslice_mask);
>  }
>  
> +/* Keep in chronological order */
> +enum intel_platform {
> +	INTEL_PLATFORM_UNINITIALIZED = 0,
> +	INTEL_I830,
> +	INTEL_I845G,
> +	INTEL_I85X,
> +	INTEL_I865G,
> +	INTEL_I915G,
> +	INTEL_I915GM,
> +	INTEL_I945G,
> +	INTEL_I945GM,
> +	INTEL_G33,
> +	INTEL_G4X,
> +	INTEL_PINEVIEW,
> +	INTEL_BROADWATER,
> +	INTEL_CRESTLINE,

The order here is rather wonky.

Assuming we want to keep roughly to the gen based order, then we'd have:

G33/BLB
PNV
965G/BW
965GM/CL
G4X/ELK
GM45/CTG

OTOH if we want to order based on the chipset side of things (which
might be closer to a chronological order I believe) we would have:

965G/BW
965GM/CL
G33/BLB
PNV
G4X/ELK
GM45/CTG

Well, pnv might have to come after elk/ctg to actually to fit into
the chronological order correctly.

This also highlights the issue we're rather inconsistent with the
number vs. codename thing. I think if we want to keep using both we
should draw a clear line where we switch. I think I would at least
flip bw/cl over to the number scheme if we want to keep to the number
scheme for the older parts.

> +	INTEL_IRONLAKE,
> +	INTEL_SANDYBRIDGE,
> +	INTEL_IVYBRIDGE,
> +	INTEL_VALLEYVIEW,
> +	INTEL_CHERRYVIEW,
> +	INTEL_HASWELL,
> +	INTEL_BROADWELL,

And as Chris noted this should be (based on either gen or
chronological order):

IVB
VLV
HSW
BDW
CHV

> +	INTEL_SKYLAKE,
> +	INTEL_BROXTON,
> +	INTEL_KABYLAKE,
> +};
> +
-- 
Ville Syrjälä
Intel OTC