[Intel-gfx] [PATCH 02/12] drm/i915: remove some extra retiring

[Daniel Vetter]

On Thu, Apr 26, 2012 at 04:02:59PM -0700, Ben Widawsky wrote:
> Not every caller of gpu_idle needs to retire requests. Try to pick only
> the callers that need it. This was originally combined with the previous
> patch in the first series on the mailing list.
> 
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_dma.c            |    1 -
>  drivers/gpu/drm/i915/i915_gem.c            |    1 -
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    1 -
>  drivers/gpu/drm/i915/intel_overlay.c       |    2 --
>  4 files changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 36c8d5f..e73389d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -2016,7 +2016,6 @@ int i915_driver_unload(struct drm_device *dev)
>  	ret = i915_gpu_idle(dev);
>  	if (ret)
>  		DRM_ERROR("failed to idle hardware: %d\n", ret);
> -	i915_gem_retire_requests(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	/* Cancel the retire work handler, which should be idle now. */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 25be0e0..3b731ef 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3387,7 +3387,6 @@ i915_gem_idle(struct drm_device *dev)
>  		mutex_unlock(&dev->struct_mutex);
>  		return ret;
>  	}
> -	i915_gem_retire_requests(dev);

Imo this will still blow up resume on gen2/3 ... we may not leak any
request over a s/r cycle in general, because on resume we restart with the
seqno counting at 1. So when you then want to wait for a no longer active,
but not yet retired request after resume, that will take a while.

I also miss a bit of justification for the first hunk, but module unload
calling gpu_idle instead of gem_idle is a bit fishy in and off itself.

I think the parts below are ok, but I'm hunting an overlay regression in
dinq atm, so I'll hold of merging this.
-Daniel

>  	/* Under UMS, be paranoid and evict. */
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index cbba0aa..582f6c4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1223,7 +1223,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  			ret = i915_gpu_idle(dev);
>  			if (ret)
>  				goto err;
> -			i915_gem_retire_requests(dev);
>  
>  			BUG_ON(ring->sync_seqno[i]);
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
> index e06e46a..07a5cad 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -228,7 +228,6 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
>  	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
>  	if (ret)
>  		return ret;
> -	i915_gem_retire_requests(dev);
>  
>  	overlay->last_flip_req = 0;
>  	return 0;
> @@ -450,7 +449,6 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
>  	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
>  	if (ret)
>  		return ret;
> -	i915_gem_retire_requests(dev);
>  
>  	if (overlay->flip_tail)
>  		overlay->flip_tail(overlay);
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48