3 years too late, but hopefully better late than never, start to rectify the damage caused by commit a8ebba75b358 ("drm/i915: Use the coarse ping-pong mechanism based on drm fd to dispatch the BSD command on BDW GT3") and compounded by commit 8d360dffd6d8 ("drm/i915: Specify bsd rings through exec flag") which did not allocate BSD2 its own identifier but overloaded the existing BSD uabi id. Worse, those patches *overrode* existing behaviour (i.e. were not backwards or forwards compatible) which makes undoing the damage tricky. As an ABI break seems unavoidable, make it so. The ramification is that libva on a few bdw/skl boxes will lose access to the second BSD engine until it is updated to request the second engine explicitly (which is what libva preferred to do anyway!) Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Zhipeng Gong <zhipeng.gong@xxxxxxxxx> Cc: Zhao Yakui <yakui.zhao@xxxxxxxxx> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> Cc: Imre Deak <imre.deak@xxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx --- drivers/gpu/drm/i915/i915_drv.h | 2 - drivers/gpu/drm/i915/i915_gem.c | 2 - drivers/gpu/drm/i915/i915_gem_execbuffer.c | 79 +----------------------------- drivers/gpu/drm/i915/intel_engine_cs.c | 3 +- include/uapi/drm/i915_drm.h | 13 ++--- 5 files changed, 11 insertions(+), 88 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 65e3666b9add..8119d74b6a2a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -540,8 +540,6 @@ struct drm_i915_file_private { unsigned boosts; } rps; - unsigned int bsd_engine; - /* Client can have a maximum of 3 contexts banned before * it is denied of creating new contexts. As one context * ban needs 4 consecutive hangs, and more if there is diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 15282eef5012..c2a660c7538b 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4873,8 +4873,6 @@ int i915_gem_open(struct drm_device *dev, struct drm_file *file) spin_lock_init(&file_priv->mm.lock); INIT_LIST_HEAD(&file_priv->mm.request_list); - file_priv->bsd_engine = -1; - ret = i915_gem_context_open(dev, file); if (ret) kfree(file_priv); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 21c1478a6944..a24c3ce3a871 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1948,82 +1948,6 @@ eb_submit(struct i915_execbuffer *eb) return 0; } -/** - * Find one BSD ring to dispatch the corresponding BSD command. - * The engine index is returned. - */ -static unsigned int -gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, - struct drm_file *file) -{ - struct drm_i915_file_private *file_priv = file->driver_priv; - - /* Check whether the file_priv has already selected one ring. */ - if ((int)file_priv->bsd_engine < 0) - file_priv->bsd_engine = atomic_fetch_xor(1, - &dev_priv->mm.bsd_engine_dispatch_index); - - return file_priv->bsd_engine; -} - -#define I915_USER_RINGS (4) - -static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { - [I915_EXEC_DEFAULT] = RCS, - [I915_EXEC_RENDER] = RCS, - [I915_EXEC_BLT] = BCS, - [I915_EXEC_BSD] = VCS, - [I915_EXEC_VEBOX] = VECS -}; - -static struct intel_engine_cs * -eb_select_engine(struct drm_i915_private *dev_priv, - struct drm_file *file, - struct drm_i915_gem_execbuffer2 *args) -{ - unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; - struct intel_engine_cs *engine; - - if (user_ring_id > I915_USER_RINGS) { - DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); - return NULL; - } - - if ((user_ring_id != I915_EXEC_BSD) && - ((args->flags & I915_EXEC_BSD_MASK) != 0)) { - DRM_DEBUG("execbuf with non bsd ring but with invalid " - "bsd dispatch flags: %d\n", (int)(args->flags)); - return NULL; - } - - if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) { - unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK; - - if (bsd_idx == I915_EXEC_BSD_DEFAULT) { - bsd_idx = gen8_dispatch_bsd_engine(dev_priv, file); - } else if (bsd_idx >= I915_EXEC_BSD_RING1 && - bsd_idx <= I915_EXEC_BSD_RING2) { - bsd_idx >>= I915_EXEC_BSD_SHIFT; - bsd_idx--; - } else { - DRM_DEBUG("execbuf with unknown bsd ring: %u\n", - bsd_idx); - return NULL; - } - - engine = dev_priv->engine[_VCS(bsd_idx)]; - } else { - engine = dev_priv->engine[user_ring_map[user_ring_id]]; - } - - if (!engine) { - DRM_DEBUG("execbuf with invalid ring: %u\n", user_ring_id); - return NULL; - } - - return engine; -} - static struct dma_fence * dma_buf_get_fence(int fd, unsigned int flags) { @@ -2107,7 +2031,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (args->flags & I915_EXEC_IS_PINNED) eb.dispatch_flags |= I915_DISPATCH_PINNED; - eb.engine = eb_select_engine(eb.i915, file, args); + eb.engine = intel_engine_lookup(eb.i915, + args->flags & I915_EXEC_RING_MASK); if (!eb.engine) return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 8a197f826d38..c76a64483d64 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -64,7 +64,7 @@ static const struct engine_info { }, [VCS2] = { .name = "bsd2 ring", - .uabi_id = I915_EXEC_BSD, + .uabi_id = I915_EXEC_BSD2, .hw_id = VCS2_HW, .mmio_base = GEN8_BSD2_RING_BASE, .irq_shift = GEN8_VCS2_IRQ_SHIFT, @@ -1134,6 +1134,7 @@ intel_engine_lookup(struct drm_i915_private *i915, u32 uabi_id) [I915_EXEC_BLT] = BCS, [I915_EXEC_BSD] = VCS, [I915_EXEC_VEBOX] = VECS, + [I915_EXEC_BSD2] = VCS2, }; if (uabi_id >= ARRAY_SIZE(uabi_map)) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index deab27c74480..72460826f818 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -870,12 +870,13 @@ struct drm_i915_gem_execbuffer2 { __u32 num_cliprects; /** This is a struct drm_clip_rect *cliprects */ __u64 cliprects_ptr; -#define I915_EXEC_RING_MASK (7<<0) -#define I915_EXEC_DEFAULT (0<<0) -#define I915_EXEC_RENDER (1<<0) -#define I915_EXEC_BSD (2<<0) -#define I915_EXEC_BLT (3<<0) -#define I915_EXEC_VEBOX (4<<0) +#define I915_EXEC_RING_MASK 0x7 +#define I915_EXEC_DEFAULT 0 +#define I915_EXEC_RENDER 1 +#define I915_EXEC_BSD 2 +#define I915_EXEC_BLT 3 +#define I915_EXEC_VEBOX 4 +#define I915_EXEC_BSD2 5 /* Used for switching the constants addressing mode on gen4+ RENDER ring. * Gen6+ only supports relative addressing to dynamic state (default) and -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx