[Intel-gfx] [PATCH 4/4] drm/i915: report all active objects as busy

[Chris Wilson]

On Thu, 22 Apr 2010 22:12:52 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Reporting objects that are not currently used by the gpu but are
> dirty and not yet flushed creates havoc to libdrm's bo reuse:
> The usually immediatly following gtt_map afterwards will stall
> until the gpu has executed the writeback.

I like this and I think Eric will approve. Just one comment inline.
 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   27 +++++++++++++++++++--------
>  1 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8e18c9d..7950169 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4395,14 +4395,25 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  	i915_gem_retire_requests(dev);
>  
>  	obj_priv = to_intel_bo(obj);
> -	/* Don't count being on the flushing list against the object being
> -	 * done.  Otherwise, a buffer left on the flushing list but not getting
> -	 * flushed (because nobody's flushing that domain) won't ever return
> -	 * unbusy and get reused by libdrm's bo cache.  The other expected
> -	 * consumer of this interface, OpenGL's occlusion queries, also specs
> -	 * that the objects get unbusy "eventually" without any interference.
> -	 */
> -	args->busy = obj_priv->active && obj_priv->last_rendering_seqno != 0;
> +
> +	/* Count all active objects as busy, even if they are currently not used
> +	 * by the gpu. Users of this interface expect objects to eventually
> +	 * become non-busy without any further actions, therefore emit any
> +	 * necessary flushes here. */
> +	args->busy = obj_priv->active;
> +
> +	/* 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->write_domain) {
> +		uint32_t seqno;
> +
> +		i915_gem_flush(dev, 0, obj->write_domain);
> +		seqno = i915_add_request(dev, file_priv, obj->write_domain);
> +		if (seqno == 0)
> +			return -ENOMEM;

Return whilst holding the mutex and leaking a reference? Oh noes! Two wrongs
do not make a right. ;-)
-ickle

-- 
Chris Wilson, Intel Open Source Technology Centre