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@xxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
