[Intel-gfx] [PATCH] drm/i915: Implement fair lru eviction across both rings. (v2)

[Daniel Vetter]

On Fri, Aug 06, 2010 at 12:25:45PM +0100, Chris Wilson wrote:
> Based in a large part upon Daniel Vetter's implementation and adapted
> for handling multiple rings in a single pass.
> 
> This should lead to better gtt usage and fixes the page-fault-of-doom
> triggered. The fairness is provided by scanning through the GTT space
> amalgamating space in rendering order. As soon as we have a contiguous
> space in the GTT large enough for the new object (and its alignment),
> evict any object which lies within that space. This should keep more
> objects resident in the GTT.
> 
> Doing throughput testing on a PineView machine with cairo-perf-trace
> indicates that there is very little difference with the new LRU scan,
> perhaps a small improvement... Except oddly for the poppler trace.
> 
> Reference:
> 
>   Bug 15911 - Intermittent X crash (freeze)
>   https://bugzilla.kernel.org/show_bug.cgi?id=15911
> 
>   Bug 20152 - cannot view JPG in firefox when running UXA
>   https://bugs.freedesktop.org/show_bug.cgi?id=20152
> 
>   Bug 24369 - Hang when scrolling firefox page with window in front
>   https://bugs.freedesktop.org/show_bug.cgi?id=24369
> 
>   Bug 28478 - Intermittent graphics lockups due to overflow/loop
>   https://bugs.freedesktop.org/show_bug.cgi?id=28478
> 
> v2: Attempt to clarify the logic and order of eviction through the use
> of comments and macros.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel at ffwll.ch>

One small nitpick below.

>  int
> -i915_gem_evict_something(struct drm_device *dev,
> -			 int min_size, unsigned alignment)
> +i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignment)
>  {

> [cut]

> +	return i915_gem_evict_everything(dev);

I think this should be equivalent to

	return -ENOMEM;

(minus wasting less cpu, of course ;) But that's easily fixable in another
patch (and probably better for bisection). Otherwise the patch looks good,
besides that my gut still thinks that One Scan to Rule Them All is
overkill. But that's simply bike-shedding, hence:

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