Commit de1add36 authored by Tvrtko Ursulin's avatar Tvrtko Ursulin

drm/i915: Decouple execbuf uAPI from internal implementation

At the moment execbuf ring selection is fully coupled to
internal ring ids which is not a good thing on its own.

This dependency is also spread between two source files and
not spelled out at either side which makes it hidden and
fragile.

This patch decouples this dependency by introducing an explicit
translation table of execbuf uAPI to ring id close to the only
call site (i915_gem_do_execbuffer).

This way we are free to change driver internal implementation
details without breaking userspace. All state relating to the
uAPI is now contained in, or next to, i915_gem_do_execbuffer.

As a side benefit, this patch decreases the compiled size
of i915_gem_do_execbuffer.

v2: Extract ring selection into eb_select_ring. (Chris Wilson)
Signed-off-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1452870770-13981-1-git-send-email-tvrtko.ursulin@linux.intel.com
parent 7c17d377
...@@ -334,7 +334,7 @@ struct drm_i915_file_private { ...@@ -334,7 +334,7 @@ struct drm_i915_file_private {
unsigned boosts; unsigned boosts;
} rps; } rps;
struct intel_engine_cs *bsd_ring; unsigned int bsd_ring;
}; };
enum intel_dpll_id { enum intel_dpll_id {
...@@ -1299,7 +1299,7 @@ struct i915_gem_mm { ...@@ -1299,7 +1299,7 @@ struct i915_gem_mm {
bool busy; bool busy;
/* the indicator for dispatch video commands on two BSD rings */ /* the indicator for dispatch video commands on two BSD rings */
int bsd_ring_dispatch_index; unsigned int bsd_ring_dispatch_index;
/** Bit 6 swizzling required for X tiling */ /** Bit 6 swizzling required for X tiling */
uint32_t bit_6_swizzle_x; uint32_t bit_6_swizzle_x;
......
...@@ -5141,6 +5141,8 @@ int i915_gem_open(struct drm_device *dev, struct drm_file *file) ...@@ -5141,6 +5141,8 @@ int i915_gem_open(struct drm_device *dev, struct drm_file *file)
spin_lock_init(&file_priv->mm.lock); spin_lock_init(&file_priv->mm.lock);
INIT_LIST_HEAD(&file_priv->mm.request_list); INIT_LIST_HEAD(&file_priv->mm.request_list);
file_priv->bsd_ring = -1;
ret = i915_gem_context_open(dev, file); ret = i915_gem_context_open(dev, file);
if (ret) if (ret)
kfree(file_priv); kfree(file_priv);
......
...@@ -1325,33 +1325,23 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, ...@@ -1325,33 +1325,23 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
/** /**
* Find one BSD ring to dispatch the corresponding BSD command. * Find one BSD ring to dispatch the corresponding BSD command.
* The Ring ID is returned. * The ring index is returned.
*/ */
static int gen8_dispatch_bsd_ring(struct drm_device *dev, static unsigned int
struct drm_file *file) gen8_dispatch_bsd_ring(struct drm_i915_private *dev_priv, struct drm_file *file)
{ {
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_file_private *file_priv = file->driver_priv; struct drm_i915_file_private *file_priv = file->driver_priv;
/* Check whether the file_priv is using one ring */ /* Check whether the file_priv has already selected one ring. */
if (file_priv->bsd_ring) if ((int)file_priv->bsd_ring < 0) {
return file_priv->bsd_ring->id; /* If not, use the ping-pong mechanism to select one. */
else { mutex_lock(&dev_priv->dev->struct_mutex);
/* If no, use the ping-pong mechanism to select one ring */ file_priv->bsd_ring = dev_priv->mm.bsd_ring_dispatch_index;
int ring_id; dev_priv->mm.bsd_ring_dispatch_index ^= 1;
mutex_unlock(&dev_priv->dev->struct_mutex);
mutex_lock(&dev->struct_mutex);
if (dev_priv->mm.bsd_ring_dispatch_index == 0) {
ring_id = VCS;
dev_priv->mm.bsd_ring_dispatch_index = 1;
} else {
ring_id = VCS2;
dev_priv->mm.bsd_ring_dispatch_index = 0;
}
file_priv->bsd_ring = &dev_priv->ring[ring_id];
mutex_unlock(&dev->struct_mutex);
return ring_id;
} }
return file_priv->bsd_ring;
} }
static struct drm_i915_gem_object * static struct drm_i915_gem_object *
...@@ -1374,6 +1364,63 @@ eb_get_batch(struct eb_vmas *eb) ...@@ -1374,6 +1364,63 @@ eb_get_batch(struct eb_vmas *eb)
return vma->obj; return vma->obj;
} }
#define I915_USER_RINGS (4)
static const enum intel_ring_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 int
eb_select_ring(struct drm_i915_private *dev_priv,
struct drm_file *file,
struct drm_i915_gem_execbuffer2 *args,
struct intel_engine_cs **ring)
{
unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK;
if (user_ring_id > I915_USER_RINGS) {
DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id);
return -EINVAL;
}
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 -EINVAL;
}
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_ring(dev_priv, file);
} else if (bsd_idx >= I915_EXEC_BSD_RING1 &&
bsd_idx <= I915_EXEC_BSD_RING2) {
bsd_idx--;
} else {
DRM_DEBUG("execbuf with unknown bsd ring: %u\n",
bsd_idx);
return -EINVAL;
}
*ring = &dev_priv->ring[_VCS(bsd_idx)];
} else {
*ring = &dev_priv->ring[user_ring_map[user_ring_id]];
}
if (!intel_ring_initialized(*ring)) {
DRM_DEBUG("execbuf with invalid ring: %u\n", user_ring_id);
return -EINVAL;
}
return 0;
}
static int static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data, i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file, struct drm_file *file,
...@@ -1412,51 +1459,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, ...@@ -1412,51 +1459,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
if (args->flags & I915_EXEC_IS_PINNED) if (args->flags & I915_EXEC_IS_PINNED)
dispatch_flags |= I915_DISPATCH_PINNED; dispatch_flags |= I915_DISPATCH_PINNED;
if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) { ret = eb_select_ring(dev_priv, file, args, &ring);
DRM_DEBUG("execbuf with unknown ring: %d\n", if (ret)
(int)(args->flags & I915_EXEC_RING_MASK)); return ret;
return -EINVAL;
}
if (((args->flags & I915_EXEC_RING_MASK) != 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 -EINVAL;
}
if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT)
ring = &dev_priv->ring[RCS];
else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
if (HAS_BSD2(dev)) {
int ring_id;
switch (args->flags & I915_EXEC_BSD_MASK) {
case I915_EXEC_BSD_DEFAULT:
ring_id = gen8_dispatch_bsd_ring(dev, file);
ring = &dev_priv->ring[ring_id];
break;
case I915_EXEC_BSD_RING1:
ring = &dev_priv->ring[VCS];
break;
case I915_EXEC_BSD_RING2:
ring = &dev_priv->ring[VCS2];
break;
default:
DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
(int)(args->flags & I915_EXEC_BSD_MASK));
return -EINVAL;
}
} else
ring = &dev_priv->ring[VCS];
} else
ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1];
if (!intel_ring_initialized(ring)) {
DRM_DEBUG("execbuf with invalid ring: %d\n",
(int)(args->flags & I915_EXEC_RING_MASK));
return -EINVAL;
}
if (args->buffer_count < 1) { if (args->buffer_count < 1) {
DRM_DEBUG("execbuf with %d buffers\n", args->buffer_count); DRM_DEBUG("execbuf with %d buffers\n", args->buffer_count);
......
...@@ -149,14 +149,14 @@ struct i915_ctx_workarounds { ...@@ -149,14 +149,14 @@ struct i915_ctx_workarounds {
struct intel_engine_cs { struct intel_engine_cs {
const char *name; const char *name;
enum intel_ring_id { enum intel_ring_id {
RCS = 0x0, RCS = 0,
VCS,
BCS, BCS,
VECS, VCS,
VCS2 VCS2, /* Keep instances of the same type engine together. */
VECS
} id; } id;
#define I915_NUM_RINGS 5 #define I915_NUM_RINGS 5
#define LAST_USER_RING (VECS + 1) #define _VCS(n) (VCS + (n))
u32 mmio_base; u32 mmio_base;
struct drm_device *dev; struct drm_device *dev;
struct intel_ringbuffer *buffer; struct intel_ringbuffer *buffer;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment