Re: [PATCH 09/18] drm/i915: possibly invalidate TLB before context switch

From: Daniel Vetter <daniel@ffwll.ch>
To: Ben Widawsky <ben@bwidawsk.net>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 09/18] drm/i915: possibly invalidate TLB before context switch
Date: Fri, 30 Mar 2012 21:01:24 +0200	[thread overview]
Message-ID: <20120330190124.GA23526@phenom.ffwll.local> (raw)
In-Reply-To: <20120330113953.1688ed5e@bwidawsk.net>

On Fri, Mar 30, 2012 at 11:39:53AM -0700, Ben Widawsky wrote:
> On Thu, 29 Mar 2012 21:25:49 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Sun, Mar 18, 2012 at 01:39:49PM -0700, Ben Widawsky wrote:
> > > From http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol1_Part3.pdf
> > > 
> > > [DevSNB] If Flush TLB invalidation Mode is enabled it’s the driver’s
> > > responsibility to invalidate the TLBs at least once after the previous
> > > context switch after any GTT mappings changed (including new GTT
> > > entries).  This can be done by a pipelined PIPE_CONTROL with TLB inv bit
> > > set immediately before MI_SET_CONTEXT.
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > Hm, I've beend decently confused about the meaning of
> > GFX_TLB_INVALIDATE_ALWAYS - it actually means that we flush tlbs on every
> > full flush (i.e. always) when it's reset. And we don't set this so this
> > workaround is pretty much just informational. I'm hence wondering whether
> > a big comment wouldn't be better?
> > 
> > -Daniel
> 
> It's probably not much difference in terms of LOC with the plus that the
> "comment" turns into correctly functioning code if we flip the bit.
> Just from a quick glance at code though, it seems we don't explicitly
> touch this bit for GEN6. Which was probably why I did it in the first
> place.
> 
> if (IS_GEN7(dev))
> 	I915_WRITE(GFX_MODE_GEN7,
> 		   GFX_MODE_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
> 		   GFX_MODE_ENABLE(GFX_REPLAY_MODE));
> 
> 
> Anyway, I really don't care enough to argue, but I like the way it is,
> and it's been tested the way it is; call the ball.

Add a quick comment where you insert the wa flush that in our current
config this isn't used and keep it. I don't care too much.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx