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

2014-03-26 Thread Chris Wilson
On Wed, Mar 26, 2014 at 05:14:05AM +, Gupta, Sourab wrote:
 On Tue, 2014-03-25 at 10:59 +, Chris Wilson wrote:
  On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote:
   From: Akash Goel akash.g...@intel.com
   
   Added a new rendering specific Workaround 
   'WaTlbInvalidateStoreDataBefore'.
   This workaround has to be applied before doing TLB Invalidation on render 
   ring.
   In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
   Store data commands.
   Without this, hardware cannot guarantee the command after the PIPE_CONTROL
   with TLB inv will not use the old TLB values.
  
  Note, that our command programming sequence already has multiple dword
  writes between the flush of the last batch and the invalidation of the
  next.
  
  Is this w/a still required? Why?
  -Chris
  
 Hi Chris,
 Are you referring to the MI_STORE_DWORD_INDEX commands emitted as a part
 of add_request functions?
 We couldn't find any other place where we are storing dwords between
 flush of last batch and invalidation of next.
 In that case, we agree that as a part of command programming sequence,
 we'll have one set of store dwords emitted.
 
 But, as per spec, it is required to emit 2 sets of store dwords before
 the tlb invalidation.

At this moment, I am upset how vague this recommendation is. Why don't
the LRI count? Is there a timing requirement? Do the stores have to
be different pages to flush a TLB etc?

 Also, our motive for having this w/a is just being paranoid, and not
 assuming that dwords would already have been emitted before doing tlb
 invalidation. So, we try to explicitly ensure the same through our w/a.

Which would be as easy as doubling up the STORE_DW(seqno).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-03-26 Thread Gupta, Sourab
On Wed, 2014-03-26 at 07:54 +, Chris Wilson wrote:
 On Wed, Mar 26, 2014 at 05:14:05AM +, Gupta, Sourab wrote:
  On Tue, 2014-03-25 at 10:59 +, Chris Wilson wrote:
   On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote:
From: Akash Goel akash.g...@intel.com

Added a new rendering specific Workaround 
'WaTlbInvalidateStoreDataBefore'.
This workaround has to be applied before doing TLB Invalidation on 
render ring.
In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
Store data commands.
Without this, hardware cannot guarantee the command after the 
PIPE_CONTROL
with TLB inv will not use the old TLB values.
   
   Note, that our command programming sequence already has multiple dword
   writes between the flush of the last batch and the invalidation of the
   next.
   
   Is this w/a still required? Why?
   -Chris
   
  Hi Chris,
  Are you referring to the MI_STORE_DWORD_INDEX commands emitted as a part
  of add_request functions?
  We couldn't find any other place where we are storing dwords between
  flush of last batch and invalidation of next.
  In that case, we agree that as a part of command programming sequence,
  we'll have one set of store dwords emitted.
  
  But, as per spec, it is required to emit 2 sets of store dwords before
  the tlb invalidation.
 
 At this moment, I am upset how vague this recommendation is. Why don't
 the LRI count? Is there a timing requirement? Do the stores have to
 be different pages to flush a TLB etc?
 
Hi Chris,
I see your point. We do see the MI_LOAD_REGISTER_IMM calls, but we were
not sure whether they would serve the purpose (forgive our limited
visibility on the low level pipeline details). 
If the LRI + store dword combination does the job, then this w/a may not
be required and we can drop it.
Regards,
Sourab

  Also, our motive for having this w/a is just being paranoid, and not
  assuming that dwords would already have been emitted before doing tlb
  invalidation. So, we try to explicitly ensure the same through our w/a.
 
 Which would be as easy as doubling up the STORE_DW(seqno).
 -Chris
 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-03-25 Thread Chris Wilson
On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote:
 From: Akash Goel akash.g...@intel.com
 
 Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
 This workaround has to be applied before doing TLB Invalidation on render 
 ring.
 In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
 Store data commands.
 Without this, hardware cannot guarantee the command after the PIPE_CONTROL
 with TLB inv will not use the old TLB values.

Note, that our command programming sequence already has multiple dword
writes between the flush of the last batch and the invalidation of the
next.

Is this w/a still required? Why?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2014-03-25 Thread Gupta, Sourab
On Tue, 2014-03-25 at 10:59 +, Chris Wilson wrote:
 On Tue, Mar 25, 2014 at 03:23:34PM +0530, sourab.gu...@intel.com wrote:
  From: Akash Goel akash.g...@intel.com
  
  Added a new rendering specific Workaround 'WaTlbInvalidateStoreDataBefore'.
  This workaround has to be applied before doing TLB Invalidation on render 
  ring.
  In this WA, before pipecontrol with TLB invalidate set, need to add 2 MI
  Store data commands.
  Without this, hardware cannot guarantee the command after the PIPE_CONTROL
  with TLB inv will not use the old TLB values.
 
 Note, that our command programming sequence already has multiple dword
 writes between the flush of the last batch and the invalidation of the
 next.
 
 Is this w/a still required? Why?
 -Chris
 
Hi Chris,
Are you referring to the MI_STORE_DWORD_INDEX commands emitted as a part
of add_request functions?
We couldn't find any other place where we are storing dwords between
flush of last batch and invalidation of next.
In that case, we agree that as a part of command programming sequence,
we'll have one set of store dwords emitted.

But, as per spec, it is required to emit 2 sets of store dwords before
the tlb invalidation.
Also, our motive for having this w/a is just being paranoid, and not
assuming that dwords would already have been emitted before doing tlb
invalidation. So, we try to explicitly ensure the same through our w/a.

Regards,
Sourab



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx