On Thu, Apr 06, 2017 at 08:00:15AM -0700, 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)
>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 71 +++++++++++++++++++++-------------
> 1 file changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index c6a73d0..6eab22d 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;
> + 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 exec_id;
> unsigned int hw_id;
> enum intel_engine_class 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,
> + .hw_id = RCS_HW,
> .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,
> + .hw_id = BCS_HW,
> .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,
> + .hw_id = VCS_HW,
> .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,
> + .hw_id = VCS2_HW,
> .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,
> + .hw_id = VECS_HW,
> .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,6 +112,8 @@
> enum intel_engine_id id)
> {
> const struct engine_info *info = &intel_engines[id];
> + const struct engine_class_info *class_info =
> + &intel_engine_classes[info->class];
Hmm, maybe we should add some protection against out-of-bound access
to the class info array ? We can start with:
GEM_BUG_ON(info->class > ARRAY_SIZE(intel_engine_classes));
-Michal
> struct intel_engine_cs *engine;
>
> GEM_BUG_ON(dev_priv->engine[id]);
> @@ -109,7 +124,7 @@
> engine->id = id;
> engine->i915 = dev_priv;
> snprintf(engine->name, sizeof(engine->name), "%s%u",
> - info->name, info->instance);
> + class_info->name, info->instance);
> engine->exec_id = info->exec_id;
> engine->hw_id = engine->guc_id = info->hw_id;
> engine->mmio_base = info->mmio_base;
> @@ -190,12 +205,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;
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
