[Intel-gfx] [RFC] drm/i915: store all mmio bases in intel_engines

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2018-03-07 19:45:15)
> The mmio bases we're currently storing in the intel_engines array are
> only valid for a subset of gens, so we need to ignore them and use
> different values in some cases. Instead of doing that, we can have a
> table of [starting gen, mmio base] pairs for each engine in
> intel_engines and select the correct one based on the gen we're running
> on in a consistent way.
> 
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c  | 77 +++++++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  1 -
>  2 files changed, 49 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 4ba139c27fba..1dd92cac8d67 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -81,12 +81,16 @@ static const struct engine_class_info intel_engine_classes[] = {
>         },
>  };
>  
> +#define MAX_MMIO_BASES 3
>  struct engine_info {
>         unsigned int hw_id;
>         unsigned int uabi_id;
>         u8 class;
>         u8 instance;
> -       u32 mmio_base;
> +       struct engine_mmio_base {
> +               u32 gen : 8;
> +               u32 base : 24;
> +       } mmio_bases[MAX_MMIO_BASES];

Needs a note to mention the array must be in reverse gen order.

I would even add a selftest just to verify the arrays.

>         unsigned irq_shift;
>  };
>  
> @@ -96,7 +100,9 @@ static const struct engine_info intel_engines[] = {
>                 .uabi_id = I915_EXEC_RENDER,
>                 .class = RENDER_CLASS,
>                 .instance = 0,
> -               .mmio_base = RENDER_RING_BASE,
> +               .mmio_bases = {
> +                       { .gen = 1, .base = RENDER_RING_BASE }

Even gen0 (i740) has the render ring :)

> +static u32 __engine_mmio_base(struct drm_i915_private *i915,
> +                             const struct engine_mmio_base* bases)
> +{
> +       int i;
> +
> +       for (i = 0; i < MAX_MMIO_BASES; i++)
> +               if (INTEL_GEN(i915) >= bases[i].gen)
> +                       break;
> +
> +       GEM_BUG_ON(i == MAX_MMIO_BASES);
> +       GEM_BUG_ON(!bases[i].base);
> +
> +       return bases[i].base;
> +}
> +
>  static int
>  intel_engine_setup(struct drm_i915_private *dev_priv,
>                    enum intel_engine_id id)
> @@ -257,25 +296,7 @@ intel_engine_setup(struct drm_i915_private *dev_priv,
>                          class_info->name, info->instance) >=
>                 sizeof(engine->name));
>         engine->hw_id = engine->guc_id = info->hw_id;
> -       if (INTEL_GEN(dev_priv) >= 11) {
> -               switch (engine->id) {
> -               case VCS:
> -                       engine->mmio_base = GEN11_BSD_RING_BASE;
> -                       break;
> -               case VCS2:
> -                       engine->mmio_base = GEN11_BSD2_RING_BASE;
> -                       break;
> -               case VECS:
> -                       engine->mmio_base = GEN11_VEBOX_RING_BASE;
> -                       break;
> -               default:
> -                       /* take the original value for all other engines  */
> -                       engine->mmio_base = info->mmio_base;
> -                       break;
> -               }
> -       } else {
> -               engine->mmio_base = info->mmio_base;
> -       }
> +       engine->mmio_base = __engine_mmio_base(dev_priv, info->mmio_bases);
>         engine->irq_shift = info->irq_shift;
>         engine->class = info->class;
>         engine->instance = info->instance;

Very neat.
-Chris