[Intel-gfx] [PATCH 09/18] drm/i915: possibly invalidate TLB before context switch

2012-03-18 Thread Ben Widawsky
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 
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   12 
 drivers/gpu/drm/i915/intel_ringbuffer.h |4 
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e892364..392e782 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -399,6 +399,10 @@ static int init_render_ring(struct intel_ring_buffer *ring)
return ret;
}
 
+   if (INTEL_INFO(dev)->gen == 6)
+   ring->itlb_before_ctx_switch =
+   !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_ALWAYS);
+
if (INTEL_INFO(dev)->gen >= 6) {
I915_WRITE(INSTPM,
   INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
@@ -927,6 +931,14 @@ int intel_ring_mi_set_context(struct intel_ring_buffer 
*ring,
 {
int ret;
 
+   if (IS_GEN6(ring->dev) && ring->itlb_before_ctx_switch) {
+   /* w/a: If Flush TLB Invalidation Mode is enabled, driver must
+* do a TLB invalidation prior to MI_SET_CONTEXT
+*/
+   gen6_render_ring_flush(ring, 0, 0);
+   }
+
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0ed98bb..e404e52 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -120,6 +120,10 @@ struct  intel_ring_buffer {
wait_queue_head_t irq_queue;
drm_local_map_t map;
 
+   /**
+* Do an explicit TLB flush before MI_SET_CONTEXT
+*/
+   bool itlb_before_ctx_switch;
struct i915_hw_context *default_context;
struct drm_i915_gem_object *last_context_obj;
 
-- 
1.7.9.4

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


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

2012-03-29 Thread Daniel Vetter
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 

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

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   12 
>  drivers/gpu/drm/i915/intel_ringbuffer.h |4 
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index e892364..392e782 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -399,6 +399,10 @@ static int init_render_ring(struct intel_ring_buffer 
> *ring)
>   return ret;
>   }
>  
> + if (INTEL_INFO(dev)->gen == 6)
> + ring->itlb_before_ctx_switch =
> + !!(I915_READ(GFX_MODE) & GFX_TLB_INVALIDATE_ALWAYS);
> +
>   if (INTEL_INFO(dev)->gen >= 6) {
>   I915_WRITE(INSTPM,
>  INSTPM_FORCE_ORDERING << 16 | INSTPM_FORCE_ORDERING);
> @@ -927,6 +931,14 @@ int intel_ring_mi_set_context(struct intel_ring_buffer 
> *ring,
>  {
>   int ret;
>  
> + if (IS_GEN6(ring->dev) && ring->itlb_before_ctx_switch) {
> + /* w/a: If Flush TLB Invalidation Mode is enabled, driver must
> +  * do a TLB invalidation prior to MI_SET_CONTEXT
> +  */
> + gen6_render_ring_flush(ring, 0, 0);
> + }
> +
> +
>   ret = intel_ring_begin(ring, 6);
>   if (ret)
>   return ret;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 0ed98bb..e404e52 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -120,6 +120,10 @@ struct  intel_ring_buffer {
>   wait_queue_head_t irq_queue;
>   drm_local_map_t map;
>  
> + /**
> +  * Do an explicit TLB flush before MI_SET_CONTEXT
> +  */
> + bool itlb_before_ctx_switch;
>   struct i915_hw_context *default_context;
>   struct drm_i915_gem_object *last_context_obj;
>  
> -- 
> 1.7.9.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: dan...@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


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

2012-03-30 Thread Ben Widawsky
On Thu, 29 Mar 2012 21:25:49 +0200
Daniel Vetter  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 
> 
> 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.

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


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

2012-03-30 Thread Daniel Vetter
On Fri, Mar 30, 2012 at 11:39:53AM -0700, Ben Widawsky wrote:
> On Thu, 29 Mar 2012 21:25:49 +0200
> Daniel Vetter  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 
> > 
> > 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: dan...@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