Both need to do the same dance (or at least should). Some slight changes: - busy_ioctl now unconditionally checks for olr. Before emitting a require flush would have prevent the olr check and hence required a second call to the busy ioctl to really emit the request. - the timeout wait now also retires request. Not really required for abi-reasons, but makes a notch more sense imo. I've tested this by pimping the i-g-t test some more and also checking the polling behviour of the wait_rendering_timeout ioctl versus what busy_ioctl returns. Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch> --- drivers/gpu/drm/i915/i915_gem.c | 61 ++++++++++++++++++-------------------- 1 files changed, 29 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index d2eaa00..521e294 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2000,6 +2000,31 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj) } /** + * Ensures that an object will eventually get non-busy by flushing any required + * write domains, emitting any outstanding lazy request and retiring and + * completed requests. The unplug moniker is stolen from the linux block layer. + */ +static int +i915_gem_unplug_object(struct drm_i915_gem_object *obj) +{ + int ret; + + if (obj->active) { + ret = i915_gem_object_flush_gpu_write_domain(obj); + if (ret) + return ret; + + ret = i915_gem_check_olr(obj->ring, + obj->last_rendering_seqno); + if (ret) + return ret; + i915_gem_retire_requests_ring(obj->ring); + } + + return 0; +} + +/** * i915_gem_wait_ioctl - implements DRM_IOCTL_I915_GEM_WAIT * @DRM_IOCTL_ARGS: standard ioctl arguments * @@ -2043,11 +2068,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) return -ENOENT; } - /* Need to make sure the object is flushed first. This non-obvious - * flush is required to enforce that (active && !olr) == no wait - * necessary. - */ - ret = i915_gem_object_flush_gpu_write_domain(obj); + /* Need to make sure the object gets un-active eventually. */ + ret = i915_gem_unplug_object(obj); if (ret) goto out; @@ -2059,10 +2081,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) if (seqno == 0) goto out; - ret = i915_gem_check_olr(ring, seqno); - if (ret) - goto out; - /* Do this after OLR check to make sure we make forward progress polling * on this IOCTL with a 0 timeout (like busy ioctl) */ @@ -3302,30 +3320,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, * become non-busy without any further actions, therefore emit any * necessary flushes here. */ - args->busy = obj->active; - if (args->busy) { - /* Unconditionally flush objects, even when the gpu still uses this - * object. Userspace calling this function indicates that it wants to - * use this buffer rather sooner than later, so issuing the required - * flush earlier is beneficial. - */ - if (obj->base.write_domain & I915_GEM_GPU_DOMAINS) { - ret = i915_gem_flush_ring(obj->ring, - 0, obj->base.write_domain); - } else { - ret = i915_gem_check_olr(obj->ring, - obj->last_rendering_seqno); - } + ret = i915_gem_unplug_object(obj); - /* Update the active list for the hardware's current position. - * Otherwise this only updates on a delayed timer or when irqs - * are actually unmasked, and our working set ends up being - * larger than required. - */ - i915_gem_retire_requests_ring(obj->ring); - - args->busy = obj->active; - } + args->busy = obj->active; drm_gem_object_unreference(&obj->base); unlock: -- 1.7.7.6