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 <[email protected]>
Cc: Zhipeng Gong <[email protected]>
Cc: Zhao Yakui <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Imre Deak <[email protected]>
Cc: Daniel Vetter <[email protected]
---
 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to