[Intel-gfx] [PATCH] drm/i915: Invariably invalidate before ctx switch

[Ville Syrjälä]

On Thu, Apr 03, 2014 at 05:28:54PM +0200, Daniel Vetter wrote:
> On Thu, Apr 03, 2014 at 01:21:38PM +0300, Ville Syrjälä wrote:
> > On Thu, Apr 03, 2014 at 08:05:35AM +0100, Chris Wilson wrote:
> > > On Wed, Apr 02, 2014 at 10:30:23PM -0700, Ben Widawsky wrote:
> > > > We have been setting the bit which was originally BIOS dependent since:
> > > > commit f05bb0c7b624252a5e768287e340e8e45df96e42
> > > > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Date:   Sun Jan 20 16:33:32 2013 +0000
> > > > 
> > > >     drm/i915: GFX_MODE Flush TLB Invalidate Mode must be '1' for scanline waits
> > > > 
> > > > Therefore, we do not need to try to figure it out dynamically and we can
> > > > just always invalidate the TLBs.
> > > > 
> > > > It's a partial revert of:
> > > > commit 12b0286f49947a6cdc9285032d918466a8c3f5f9
> > > > Author: Ben Widawsky <ben at bwidawsk.net>
> > > > Date:   Mon Jun 4 14:42:50 2012 -0700
> > > > 
> > > >     drm/i915: possibly invalidate TLB before context switch
> > > > 
> > > > The original commit attempted to only invalidate when necessary
> > > > (very much a relic from the old days). Now, we can just always invalidate.
> > > > 
> > > > I guess the old TODO still exists. Since we seem to have abandoned ILK
> > > > contexts however, there isn't much point in even remembering.
> > > > 
> > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > > 
> > > Seems reasonable, except in most cases (execbuffer) there will be
> > > a following cache-invalidate as part of the move-to-gpu.
> > 
> > Except we still move_to_gpu() before the context switch. My fbc related
> > patch to change that order never got merged.
> 
> Hm, pointer to that thing?

http://lists.freedesktop.org/archives/intel-gfx/2013-November/036339.html

> I guess we need to sign up someone to dig out
> all your fbc patches and get them in line in prep for bdw. At least
> another opportunity to beat all that into better shape ...

If someone is serious about fixing FBC, I did start reworking the whole
locking mess, but it's sitting somewhere in a branch on my laptop in
half assed state.

The main new idea I had was to compute a "fbc score" for the current crtc
at modeset/setplane time (we hold the appropriate crtc lock here) and
then update/consult those scores under a new fbc lock. This way
almost all of the FBC state is protected by the fbc lock. Whenever we
need to make a decision which pipe gets to use fbc, we grab the lock and
pick the one with the highest score. And as we always reduce the score to
0 before we do anything that requires fbc to be disabled, and only set the
score to anything but 0 only after fbc can be enabled, we don't need to
grab any crtc locks in any fbc specific functions, which is nice.

The main extra complication is that the render tracking stuff still needs
to use struct_mutex as that's what execbuffer uses. And as an extra
complication whenever we decide to enable FBC on a specific plane, we
need to:
1) disable fbc if it's active on another plane. this needs to happen
   before step 2 since we can only do render tracking for one front buffer
   (with the gen7+ nuke mechanism I guess we might be able to track more
   that one actually).
2) enable render tracking for the new front buffer so any
   new rendering to the buffer will invalidate fbc
3) sample the current write seqno and wait for it, as there may
   be rendering already in flight w/o render tracking enabled
   (this is something our current tests fail to exercise
4) do the vblank wait to make sure we don't enable FBC too soon
   after enabling the plane. Alternatively we could keep the fbc score
   at 0 until at least one vblank has passed after the plane has been
   enabled, and then we won't even consider the plane for fbc until it's
   really ready for it
5) finally enable fbc

I've been hoping to find a bit of time and motivation to finish this off.
But so far that's not happened.

-- 
Ville Syrjälä
Intel OTC