On 15/01/16 12:27, Chris Wilson wrote:
On Fri, Jan 15, 2016 at 11:58:32AM +0000, Tvrtko Ursulin wrote:
On 15/01/16 11:06, Chris Wilson wrote:
Tvrtko was looking through the execbuffer-ioctl and noticed that the
uABI was tightly coupled to our internal engine identifiers. Close
inspection also revealed that we leak those internal engine identifiers
through the busy-ioctl, and those internal identifiers already do not
match the user identifiers. Fortuitiously, there is only one user of the
set of busy rings from the busy-ioctl, and they only wish to choose
between the RENDER and the BLT engines.
Let's fix the userspace ABI while we still can.
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 18 ++++++++++++++----
drivers/gpu/drm/i915/intel_lrc.c | 5 +++++
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
4 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bb44bad15403..85797813a3de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret)
goto unref;
- BUILD_BUG_ON(I915_NUM_RINGS > 16);
- args->busy = obj->active << 16;
- if (obj->last_write_req)
- args->busy |= obj->last_write_req->ring->id;
+ args->busy = 0;
+ if (obj->active) {
+ int i;
+
+ for (i = 0; i < I915_NUM_RINGS; i++) {
+ struct drm_i915_gem_request *req;
+
+ req = obj->last_read_req[i];
+ if (req)
+ args->busy |= 1 << (16 + req->ring->exec_id);
If I got it right bit 16 was RCS, now will always be clear. And
blitter was bit 17 and now is 19.
Sssh. You weren't meant to point that out.
I am willing to take the hit in order to decouple the uABI from
internals.
I am also willing to codify this busy-ioctl ABI into igt!
Looks like your DDX is the only user not using it in the boolean mode?
And libdrm is a bit confused in its return statements:
ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_BUSY, &busy);
if (ret == 0) {
bo_gem->idle = !busy.busy;
return busy.busy;
} else {
return false;
}
return (ret == 0 && busy.busy);
Looks like it was a boolean as well until commit
02f93c21e6e1c3dad9d99349989daa84a8c0b5fb quite possibly by accident
started exposing the bits.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
