[Intel-gfx] [PATCH 1/6] drm/i915/vlv: Added a rendering specific Hw WA 'WaTlbInvalidateStoreDataBefore'

[Chris Wilson]

On Mon, Mar 24, 2014 at 08:32:30PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 24, 2014 at 11:20:40AM +0000, Gupta, Sourab wrote:
> > On Mon, 2014-03-24 at 09:32 +0000, Chris Wilson wrote:
> > > On Mon, Mar 24, 2014 at 12:19:19PM +0530, sourab.gupta at intel.com wrote:
> > > > From: Akash Goel <akash.goel at intel.com>
> > > > 
> > > > Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
> > > > In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
> > > > Store data commands.
> > > > 
> > > > Signed-off-by: Akash Goel <akash.goel at intel.com>
> > > > Signed-off-by: Sourab Gupta <sourab.gupta at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 22 ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > index 87d1a2d..2812384 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > > @@ -2207,6 +2207,28 @@ intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring)
> > > >  	uint32_t flush_domains;
> > > >  	int ret;
> > > >  
> > > > +	if (IS_VALLEYVIEW(ring->dev)) {
> > > The ring flushes are vfuncs, so why is this here and not in a special
> > > vlv ring flush?
> > 
> > Yes, we can as well put it in the platform specific vlv flush. Since we
> > apply this WA only for invalidate_all_caches function, we have to
> > differentiate in the vlv flush function regarding where the flush
> > originated from. For this we plan to check the 'invalidate_domains'
> > field of flush function. (This field will be non-zero in case the call
> > originated from invalidate_all_caches function). So, we'll have a
> > vlv_render_ring_flush something like this:
> > 	if(invalidate_domains)
> > 		apply_our_wa;
> > 	gen7_render_ring_flush();
> > 
> > Does this look okay?
> 
> Since we supposdely need this for all gen6/gen7, I'd just add a new func
> (eg. gen6_tlb_invalidate_wa()) and call that from gen6_render_ring_flush(),
> gen7_render_ring_flush(), gen6_bsd_ring_flush() and gen6_ring_flush().

Now, I am extremely curious as to what the exact bug symptoms are. We
seem to have an absence of bug reports since SNB regarding random
corruption.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre