[Intel-gfx] [PATCH 4/4] drm/i915: Opportunistically reduce flushing at execbuf

[Ben Widawsky]

On Mon, Dec 15, 2014 at 08:39:35PM +0000, Chris Wilson wrote:
> On Mon, Dec 15, 2014 at 11:56:05AM -0800, Ben Widawsky wrote:
> > On Mon, Dec 15, 2014 at 08:20:50AM +0000, Chris Wilson wrote:
> > > On Mon, Dec 15, 2014 at 08:55:32AM +0100, Daniel Vetter wrote:
> > > > On Sun, Dec 14, 2014 at 03:37:36PM -0800, Ben Widawsky wrote:
> > > > > On Sun, Dec 14, 2014 at 03:12:21PM +0200, Ville Syrjälä wrote:
> > > > > > On Sat, Dec 13, 2014 at 07:08:24PM -0800, Ben Widawsky wrote:
> > > > > > > If we're moving a bunch of buffers from the CPU domain to the GPU domain, and
> > > > > > > we've already blown out the entire cache via a wbinvd, there is nothing more to
> > > > > > > do.
> > > > > > > 
> > > > > > > With this and the previous patches, I am seeing a 3x FPS increase on a certain
> > > > > > > benchmark which uses a giant 2d array texture. Unless I missed something in the
> > > > > > > code, it should only effect non-LLC i915 platforms.
> > > > > > > 
> > > > > > > I haven't yet run any numbers for other benchmarks, nor have I attempted to
> > > > > > > check if various conformance tests still pass.
> > > > > > > 
> > > > > > > NOTE: As mentioned in the previous patch, if one can easily obtain the largest
> > > > > > > buffer and attempt to flush it first, the results would be even more desirable.
> > > > > > 
> > > > > > So even with that optimization if you only have tons of small buffers
> > > > > > that need to be flushed you'd still take the clflush path for every
> > > > > > single one.
> > > > > > 
> > > > > > How difficult would it to calculate the total size to be flushed first,
> > > > > > and then make the clflush vs. wbinvd decision base on that?
> > > > > > 
> > > > > 
> > > > > I'll write the patch and send it to Eero for test.
> > > > > 
> > > > > It's not hard, and I think that's a good idea as well. One reason I didn't put
> > > > > such code in this series is that moves away from a global DRM solution (and like
> > > > > I said in the cover-letter, I am fine with that). Implementing this, I think in
> > > > > the i915 code we'd just iterate through the BOs until we got to a certain
> > > > > threshold, then just call wbinvd() from i915 and not even both with drm_cache.
> > > > > You could also maybe try to shorcut if there are more than X buffers.
> > > > 
> > > > I don't mind an i915 specific solution (we have them already in many
> > > > places). So will wait for the results of this experiments before merging
> > > > more patches.
> > > 
> > > I actually think an i915 specific solution is required, as the making
> > > drm_clflush_pages autoselect is likely to cause regressions on unaware
> > > drivers (e.g. anything that has a reservation loop in execbuf like i915).
> > 
> > Assuming the stall is gone as Jesse said in the other thread, I can't envision a
> > scenario where wbinvd would do worse on large objects.
> 
> It is the multiple wbinvd performed at each execbuffer that is worrisome.
> -Chris
> 

This patch attempts to avoid that by dropping all flushing after the first
WBINVD.

-- 
Ben Widawsky, Intel Open Source Technology Center