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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@intel.com wrote:
> > > From: Akash Goel <akash.goel@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@intel.com>
> > > Signed-off-by: Sourab Gupta <sourab.gupta@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().

-- 
Ville Syrjälä
Intel OTC