[Intel-gfx] [PATCH v2] drm/i915: Don't use enums for hardware engine id

[Chris Wilson]

On Wed, Mar 01, 2017 at 08:26:15PM +0000, Michal Wajdeczko wrote:
> Generally we are using macros for any hardware identifiers as these
> may change between Gens. Do the same with hardware engine ids.
> 
> v2: move hw engine defs to i915_reg.h (Chris)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  6 ++++++
>  drivers/gpu/drm/i915/intel_engine_cs.c  |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 33 +++++++++++++++++----------------
>  3 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 93dcbbc..456cb7d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -77,7 +77,13 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define _MASKED_BIT_ENABLE(a)	({ typeof(a) _a = (a); _MASKED_FIELD(_a, _a); })
>  #define _MASKED_BIT_DISABLE(a)	(_MASKED_FIELD((a), 0))
>  
> +/* Engine ID */
>  
> +#define RCS_HW		0
> +#define VCS_HW		1
> +#define BCS_HW		2
> +#define VECS_HW		3
> +#define VCS2_HW		4

Works for me. I dream of i915_reg.h being broken up as well.

>  
>  /* PCI config space */
>  
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index c4d4698..a238304 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -29,7 +29,7 @@
>  static const struct engine_info {
>  	const char *name;
>  	unsigned exec_id;
> -	enum intel_engine_hw_id hw_id;
> +	unsigned hw_id;
>  	u32 mmio_base;
>  	unsigned irq_shift;
>  	int (*init_legacy)(struct intel_engine_cs *engine);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 3dd6eee..b5767c5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -186,26 +186,27 @@ struct i915_ctx_workarounds {
>  struct drm_i915_gem_request;
>  struct intel_render_state;
>  
> +
> +/*
> + * Engine IDs definitions.
> + * Keep instances of the same type engine together.
> + */
> +enum intel_engine_id {
> +	RCS = 0,
> +	BCS,
> +	VCS,
> +	VCS2,
> +#define _VCS(n) (VCS + (n))
> +	VECS
> +};
> +
>  struct intel_engine_cs {
>  	struct drm_i915_private *i915;
>  	const char	*name;
> -	enum intel_engine_id {
> -		RCS = 0,
> -		BCS,
> -		VCS,
> -		VCS2,	/* Keep instances of the same type engine together. */
> -		VECS
> -	} id;
> -#define _VCS(n) (VCS + (n))
> +	enum intel_engine_id id;
>  	unsigned int exec_id;
> -	enum intel_engine_hw_id {
> -		RCS_HW = 0,
> -		VCS_HW,
> -		BCS_HW,
> -		VECS_HW,
> -		VCS2_HW
> -	} hw_id;
> -	enum intel_engine_hw_id guc_id; /* XXX same as hw_id? */
> +	unsigned int hw_id;
> +	unsigned int guc_id;

Bah. There goes the reminder that guc_id is not special and we just want
to get an architect to confirm that they won't differ!.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre