| Message ID | 1491834873-9345-5-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/04/2017 15:34, Oscar Mateo wrote: > There are some properties that logically belong to the engine class, and some > that belong to the engine instance. Make it explicit. > > v2: Commit message (Tvrtko) > > v3: > - Rebased > - Exec/uabi id should be per instance (Chris) > > v4: > - Rebased > - Avoid re-ordering fields for smaller diff (Tvrtko) > - Bug on oob access to the class array (Michal) > > v5: Bug on the right thing (Michal) > > v6: Rebased > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > Conflicts: > drivers/gpu/drm/i915/intel_engine_cs.c Snip this next time unless it sneaked in by accident. > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 65 ++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 5e5cda0..80cb0ff 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -26,71 +26,84 @@ > #include "intel_ringbuffer.h" > #include "intel_lrc.h" > > -static const struct engine_info { > +struct engine_class_info { > const char *name; > - unsigned int exec_id; > + int (*init_legacy)(struct intel_engine_cs *engine); > + int (*init_execlists)(struct intel_engine_cs *engine); > +}; > + > +static const struct engine_class_info intel_engine_classes[] = { > + [RENDER_CLASS] = { > + .name = "rcs", > + .init_execlists = logical_render_ring_init, > + .init_legacy = intel_init_render_ring_buffer, > + }, > + [COPY_ENGINE_CLASS] = { > + .name = "bcs", > + .init_execlists = logical_xcs_ring_init, > + .init_legacy = intel_init_blt_ring_buffer, > + }, > + [VIDEO_DECODE_CLASS] = { > + .name = "vcs", > + .init_execlists = logical_xcs_ring_init, > + .init_legacy = intel_init_bsd_ring_buffer, > + }, > + [VIDEO_ENHANCEMENT_CLASS] = { > + .name = "vecs", > + .init_execlists = logical_xcs_ring_init, > + .init_legacy = intel_init_vebox_ring_buffer, > + }, > +}; > + > +struct engine_info { > unsigned int hw_id; > + unsigned int exec_id; > u8 class; > u8 instance; > u32 mmio_base; > unsigned irq_shift; > - int (*init_legacy)(struct intel_engine_cs *engine); > - int (*init_execlists)(struct intel_engine_cs *engine); > -} intel_engines[] = { > +}; > + > +static const struct engine_info intel_engines[] = { > [RCS] = { > - .name = "rcs", > .hw_id = RCS_HW, > .exec_id = I915_EXEC_RENDER, > .class = RENDER_CLASS, > .instance = 0, > .mmio_base = RENDER_RING_BASE, > .irq_shift = GEN8_RCS_IRQ_SHIFT, > - .init_execlists = logical_render_ring_init, > - .init_legacy = intel_init_render_ring_buffer, > }, > [BCS] = { > - .name = "bcs", > .hw_id = BCS_HW, > .exec_id = I915_EXEC_BLT, > .class = COPY_ENGINE_CLASS, > .instance = 0, > .mmio_base = BLT_RING_BASE, > .irq_shift = GEN8_BCS_IRQ_SHIFT, > - .init_execlists = logical_xcs_ring_init, > - .init_legacy = intel_init_blt_ring_buffer, > }, > [VCS] = { > - .name = "vcs", > .hw_id = VCS_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > .instance = 0, > .mmio_base = GEN6_BSD_RING_BASE, > .irq_shift = GEN8_VCS1_IRQ_SHIFT, > - .init_execlists = logical_xcs_ring_init, > - .init_legacy = intel_init_bsd_ring_buffer, > }, > [VCS2] = { > - .name = "vcs", > .hw_id = VCS2_HW, > .exec_id = I915_EXEC_BSD, > .class = VIDEO_DECODE_CLASS, > .instance = 1, > .mmio_base = GEN8_BSD2_RING_BASE, > .irq_shift = GEN8_VCS2_IRQ_SHIFT, > - .init_execlists = logical_xcs_ring_init, > - .init_legacy = intel_init_bsd_ring_buffer, > }, > [VECS] = { > - .name = "vecs", > .hw_id = VECS_HW, > .exec_id = I915_EXEC_VEBOX, > .class = VIDEO_ENHANCEMENT_CLASS, > .instance = 0, > .mmio_base = VEBOX_RING_BASE, > .irq_shift = GEN8_VECS_IRQ_SHIFT, > - .init_execlists = logical_xcs_ring_init, > - .init_legacy = intel_init_vebox_ring_buffer, > }, > }; > > @@ -99,8 +112,12 @@ > enum intel_engine_id id) > { > const struct engine_info *info = &intel_engines[id]; > + const struct engine_class_info *class_info; > struct intel_engine_cs *engine; > > + GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); > + class_info = &intel_engine_classes[info->class]; > + > GEM_BUG_ON(dev_priv->engine[id]); > engine = kzalloc(sizeof(*engine), GFP_KERNEL); > if (!engine) > @@ -109,7 +126,7 @@ > engine->id = id; > engine->i915 = dev_priv; > WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", > - info->name, info->instance) >= sizeof(engine->name)); > + class_info->name, info->instance) >= sizeof(engine->name)); > engine->exec_id = info->exec_id; > engine->hw_id = engine->guc_id = info->hw_id; > engine->mmio_base = info->mmio_base; > @@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv) > int err = 0; > > for_each_engine(engine, dev_priv, id) { > + const struct engine_class_info *class_info = > + &intel_engine_classes[engine->class]; > int (*init)(struct intel_engine_cs *engine); > > if (i915.enable_execlists) > - init = intel_engines[id].init_execlists; > + init = class_info->init_execlists; > else > - init = intel_engines[id].init_legacy; > + init = class_info->init_legacy; > if (!init) { > kfree(engine); > dev_priv->engine[id] = NULL; > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 5e5cda0..80cb0ff 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -26,71 +26,84 @@ #include "intel_ringbuffer.h" #include "intel_lrc.h" -static const struct engine_info { +struct engine_class_info { const char *name; - unsigned int exec_id; + int (*init_legacy)(struct intel_engine_cs *engine); + int (*init_execlists)(struct intel_engine_cs *engine); +}; + +static const struct engine_class_info intel_engine_classes[] = { + [RENDER_CLASS] = { + .name = "rcs", + .init_execlists = logical_render_ring_init, + .init_legacy = intel_init_render_ring_buffer, + }, + [COPY_ENGINE_CLASS] = { + .name = "bcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_blt_ring_buffer, + }, + [VIDEO_DECODE_CLASS] = { + .name = "vcs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_bsd_ring_buffer, + }, + [VIDEO_ENHANCEMENT_CLASS] = { + .name = "vecs", + .init_execlists = logical_xcs_ring_init, + .init_legacy = intel_init_vebox_ring_buffer, + }, +}; + +struct engine_info { unsigned int hw_id; + unsigned int exec_id; u8 class; u8 instance; u32 mmio_base; unsigned irq_shift; - int (*init_legacy)(struct intel_engine_cs *engine); - int (*init_execlists)(struct intel_engine_cs *engine); -} intel_engines[] = { +}; + +static const struct engine_info intel_engines[] = { [RCS] = { - .name = "rcs", .hw_id = RCS_HW, .exec_id = I915_EXEC_RENDER, .class = RENDER_CLASS, .instance = 0, .mmio_base = RENDER_RING_BASE, .irq_shift = GEN8_RCS_IRQ_SHIFT, - .init_execlists = logical_render_ring_init, - .init_legacy = intel_init_render_ring_buffer, }, [BCS] = { - .name = "bcs", .hw_id = BCS_HW, .exec_id = I915_EXEC_BLT, .class = COPY_ENGINE_CLASS, .instance = 0, .mmio_base = BLT_RING_BASE, .irq_shift = GEN8_BCS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_blt_ring_buffer, }, [VCS] = { - .name = "vcs", .hw_id = VCS_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 0, .mmio_base = GEN6_BSD_RING_BASE, .irq_shift = GEN8_VCS1_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VCS2] = { - .name = "vcs", .hw_id = VCS2_HW, .exec_id = I915_EXEC_BSD, .class = VIDEO_DECODE_CLASS, .instance = 1, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_bsd_ring_buffer, }, [VECS] = { - .name = "vecs", .hw_id = VECS_HW, .exec_id = I915_EXEC_VEBOX, .class = VIDEO_ENHANCEMENT_CLASS, .instance = 0, .mmio_base = VEBOX_RING_BASE, .irq_shift = GEN8_VECS_IRQ_SHIFT, - .init_execlists = logical_xcs_ring_init, - .init_legacy = intel_init_vebox_ring_buffer, }, }; @@ -99,8 +112,12 @@ enum intel_engine_id id) { const struct engine_info *info = &intel_engines[id]; + const struct engine_class_info *class_info; struct intel_engine_cs *engine; + GEM_BUG_ON(info->class >= ARRAY_SIZE(intel_engine_classes)); + class_info = &intel_engine_classes[info->class]; + GEM_BUG_ON(dev_priv->engine[id]); engine = kzalloc(sizeof(*engine), GFP_KERNEL); if (!engine) @@ -109,7 +126,7 @@ engine->id = id; engine->i915 = dev_priv; WARN_ON(snprintf(engine->name, sizeof(engine->name), "%s%u", - info->name, info->instance) >= sizeof(engine->name)); + class_info->name, info->instance) >= sizeof(engine->name)); engine->exec_id = info->exec_id; engine->hw_id = engine->guc_id = info->hw_id; engine->mmio_base = info->mmio_base; @@ -190,12 +207,14 @@ int intel_engines_init(struct drm_i915_private *dev_priv) int err = 0; for_each_engine(engine, dev_priv, id) { + const struct engine_class_info *class_info = + &intel_engine_classes[engine->class]; int (*init)(struct intel_engine_cs *engine); if (i915.enable_execlists) - init = intel_engines[id].init_execlists; + init = class_info->init_execlists; else - init = intel_engines[id].init_legacy; + init = class_info->init_legacy; if (!init) { kfree(engine); dev_priv->engine[id] = NULL;