[PATCH] drm/i915: WA: FBC Render Nuke.

[PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW and VLV.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  2 ++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc4c223..81ac584 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -977,6 +977,8 @@
 /* Framebuffer compression for Ivybridge */
 #define IVB_FBC_RT_BASE			0x7020
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
 
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1879188..e830a9b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	if (!intel_edp_is_psr_enabled(dev))
 		I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3d2c236..69491db 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
+{
+	struct drm_device *dev = ring->dev;
+	int ret;
+
+	if (i915_enable_fbc == 0)
+		return 0;
+
+	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, FBC_REND_NUKE);
+	intel_ring_advance(ring);
+
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring);
+
 	return 0;
 }
 
@@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int blt_ring_flush(struct intel_ring_buffer *ring,
 			  u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev))
+		return gen7_ring_fbc_flush(ring);
+
 	return 0;
 }
 
-- 
1.8.1.4

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

Ops, sorry about miss "in-reply-to".. I was here fighting with my git
send-email for a long time doing many attempts...

Anyways, going to the point.
Ville, I decide to let it without SRM described in PM guide because it
doesn't not work properly. I' ve made many attempts here but it doesn'
t work rpopertly.
>From the MI_SOTRE_REGISTER_MEM in BSpec: "This command temporarily
halts command execution."
Besides that we don't have a reliable way to detect directly front
buffer rendering...

I understand the synchronization concerns, but this patch follows the
BSPec workaround and it is working properly here on my both machines
(IVB and HSW) for more than on week now without a single corruption or
slowness.

So, this is the best we have now and I kindly ask you guys to accept
it. It fixed many corruption and slowness that Timothy faced.
In parallel I'm going to contact PM guide writers asking about this
SRM and also asking them new updated versions.

Thanks in advance,
Rodrigo




On Tue, May 28, 2013 at 9:25 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> WaFbcNukeOn3DBlt for IVB, HSW and VLV.
>
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
>
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
>
> v3: Adding Wa name as Damien suggested.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc4c223..81ac584 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -977,6 +977,8 @@
>  /* Framebuffer compression for Ivybridge */
>  #define IVB_FBC_RT_BASE                        0x7020
>
> +#define MSG_FBC_REND_STATE     0x50380
> +#define   FBC_REND_NUKE                (1<<2)
>
>  #define _HSW_PIPE_SLICE_CHICKEN_1_A    0x420B0
>  #define _HSW_PIPE_SLICE_CHICKEN_1_B    0x420B4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1879188..e830a9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>         struct drm_i915_gem_object *obj = intel_fb->obj;
>         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> -       I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> +       I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>
>         if (!intel_edp_is_psr_enabled(dev))
>                 I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3d2c236..69491db 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>         return 0;
>  }
>
> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> +{
> +       struct drm_device *dev = ring->dev;
> +       int ret;
> +
> +       if (i915_enable_fbc == 0)
> +               return 0;
> +
> +       if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> +               return 0;
> +
> +       ret = intel_ring_begin(ring, 4);
> +       if (ret)
> +               return ret;
> +       intel_ring_emit(ring, MI_NOOP);
> +       /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
> +       intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +       intel_ring_emit(ring, MSG_FBC_REND_STATE);
> +       intel_ring_emit(ring, FBC_REND_NUKE);
> +       intel_ring_advance(ring);
> +
> +       return 0;
> +}
> +
>  static int
>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>                        u32 invalidate_domains, u32 flush_domains)
> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>         intel_ring_emit(ring, 0);
>         intel_ring_advance(ring);
>
> +       if (flush_domains)
> +               return gen7_ring_fbc_flush(ring);
> +
>         return 0;
>  }
>
> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>                           u32 invalidate, u32 flush)
>  {
> +       struct drm_device *dev = ring->dev;
>         uint32_t cmd;
>         int ret;
>
> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>         intel_ring_emit(ring, 0);
>         intel_ring_emit(ring, MI_NOOP);
>         intel_ring_advance(ring);
> +
> +       if (IS_GEN7(dev))
> +               return gen7_ring_fbc_flush(ring);
> +
>         return 0;
>  }
>
> --
> 1.8.1.4
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Ville Syrjälä]

On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote:
> WaFbcNukeOn3DBlt for IVB, HSW and VLV.

VLV doesn't have FBC, so this is a bit incorrect.

> 
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
> 
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
> 
> v3: Adding Wa name as Damien suggested.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc4c223..81ac584 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -977,6 +977,8 @@
>  /* Framebuffer compression for Ivybridge */
>  #define IVB_FBC_RT_BASE			0x7020
>  
> +#define MSG_FBC_REND_STATE	0x50380
> +#define   FBC_REND_NUKE		(1<<2)
>  
>  #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
>  #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1879188..e830a9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> +	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>  
>  	if (!intel_edp_is_psr_enabled(dev))
>  		I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3d2c236..69491db 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>  	return 0;
>  }
>  
> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> +{
> +	struct drm_device *dev = ring->dev;
> +	int ret;
> +
> +	if (i915_enable_fbc == 0)
> +		return 0;
> +
> +	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> +		return 0;
> +
> +	ret = intel_ring_begin(ring, 4);
> +	if (ret)
> +		return ret;
> +	intel_ring_emit(ring, MI_NOOP);
> +	/* WaFbcNukeOn3DBlt:ivb/hsw/vlv */

Another mention of vlv. I can see BSpec makes the same mistake in
the register description though.

> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +	intel_ring_emit(ring, MSG_FBC_REND_STATE);
> +	intel_ring_emit(ring, FBC_REND_NUKE);
> +	intel_ring_advance(ring);
> +
> +	return 0;
> +}
> +
>  static int
>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  		       u32 invalidate_domains, u32 flush_domains)
> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_advance(ring);
>  
> +	if (flush_domains)
> +		return gen7_ring_fbc_flush(ring);
> +
>  	return 0;
>  }
>  
> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>  			  u32 invalidate, u32 flush)
>  {
> +	struct drm_device *dev = ring->dev;
>  	uint32_t cmd;
>  	int ret;
>  
> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_emit(ring, MI_NOOP);
>  	intel_ring_advance(ring);
> +
> +	if (IS_GEN7(dev))
> +		return gen7_ring_fbc_flush(ring);

Should check flush_domains here as well?

So we're now using the same nuke mechanism from the blt ring too.
Should we then drop the regular blitter tracking things from fbc_enable?

Oh and what about vcs and vecs, should we nuke from those rings as well?
I guess it would be strange to write to the primary plane's buffer via
vcs, but I'm assuming vebox could write the same formats that we can
scan out...

> +
>  	return 0;
>  }
>  
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

Hi Ville,

Thanks for the comments.


On Fri, May 31, 2013 at 12:59 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote:
>> WaFbcNukeOn3DBlt for IVB, HSW and VLV.
>
> VLV doesn't have FBC, so this is a bit incorrect.

I'm going to remove the vlv mention that incorrectly came from spec...

>
>>
>> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
>> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
>> follows each render submission."
>>
>> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>>     module parameter check.
>>
>> v3: Adding Wa name as Damien suggested.
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>>  3 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index cc4c223..81ac584 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -977,6 +977,8 @@
>>  /* Framebuffer compression for Ivybridge */
>>  #define IVB_FBC_RT_BASE                      0x7020
>>
>> +#define MSG_FBC_REND_STATE   0x50380
>> +#define   FBC_REND_NUKE              (1<<2)
>>
>>  #define _HSW_PIPE_SLICE_CHICKEN_1_A  0x420B0
>>  #define _HSW_PIPE_SLICE_CHICKEN_1_B  0x420B4
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 1879188..e830a9b 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>>       struct drm_i915_gem_object *obj = intel_fb->obj;
>>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>
>> -     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
>> +     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>>
>>       if (!intel_edp_is_psr_enabled(dev))
>>               I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 3d2c236..69491db 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>>       return 0;
>>  }
>>
>> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
>> +{
>> +     struct drm_device *dev = ring->dev;
>> +     int ret;
>> +
>> +     if (i915_enable_fbc == 0)
>> +             return 0;
>> +
>> +     if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
>> +             return 0;
>> +
>> +     ret = intel_ring_begin(ring, 4);
>> +     if (ret)
>> +             return ret;
>> +     intel_ring_emit(ring, MI_NOOP);
>> +     /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
>
> Another mention of vlv. I can see BSpec makes the same mistake in
> the register description though.

... as you noticed.

>
>> +     intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>> +     intel_ring_emit(ring, MSG_FBC_REND_STATE);
>> +     intel_ring_emit(ring, FBC_REND_NUKE);
>> +     intel_ring_advance(ring);
>> +
>> +     return 0;
>> +}
>> +
>>  static int
>>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>>                      u32 invalidate_domains, u32 flush_domains)
>> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>>       intel_ring_emit(ring, 0);
>>       intel_ring_advance(ring);
>>
>> +     if (flush_domains)
>> +             return gen7_ring_fbc_flush(ring);
>> +
>>       return 0;
>>  }
>>
>> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>>                         u32 invalidate, u32 flush)
>>  {
>> +     struct drm_device *dev = ring->dev;
>>       uint32_t cmd;
>>       int ret;
>>
>> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>>       intel_ring_emit(ring, 0);
>>       intel_ring_emit(ring, MI_NOOP);
>>       intel_ring_advance(ring);
>> +
>> +     if (IS_GEN7(dev))
>> +             return gen7_ring_fbc_flush(ring);
>
> Should check flush_domains here as well?

here is flush domain by definition, isn' t it?

>
> So we're now using the same nuke mechanism from the blt ring too.
> Should we then drop the regular blitter tracking things from fbc_enable?

This is a good question. Since this is a critical patch and it is
working in the way it is I prefer to let it in the way it is and
promisse that I will try to drop old blitter tracking for ivb and hsw
later. If it works I'll send the drop in another patch.

>
> Oh and what about vcs and vecs, should we nuke from those rings as well?
> I guess it would be strange to write to the primary plane's buffer via
> vcs, but I'm assuming vebox could write the same formats that we can
> scan out...

To be truly honest with you I have no idea about these case. specs
just says to put after every pipe_control following flush
renderings... and blt.

>
>> +
>>       return 0;
>>  }
>>
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Ville Syrjälä]

On Fri, May 31, 2013 at 05:15:41PM -0300, Rodrigo Vivi wrote:
> Hi Ville,
> 
> Thanks for the comments.
> 
> 
> On Fri, May 31, 2013 at 12:59 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote:
> >> WaFbcNukeOn3DBlt for IVB, HSW and VLV.
> >
> > VLV doesn't have FBC, so this is a bit incorrect.
> 
> I'm going to remove the vlv mention that incorrectly came from spec...
> 
> >
> >>
> >> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> >> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> >> follows each render submission."
> >>
> >> v2: Chris noticed that flush_domains check was missing here and also suggested to do
> >>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
> >>     module parameter check.
> >>
> >> v3: Adding Wa name as Damien suggested.
> >>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
> >>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
> >>  3 files changed, 35 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index cc4c223..81ac584 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -977,6 +977,8 @@
> >>  /* Framebuffer compression for Ivybridge */
> >>  #define IVB_FBC_RT_BASE                      0x7020
> >>
> >> +#define MSG_FBC_REND_STATE   0x50380
> >> +#define   FBC_REND_NUKE              (1<<2)
> >>
> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_A  0x420B0
> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_B  0x420B4
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 1879188..e830a9b 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
> >>       struct drm_i915_gem_object *obj = intel_fb->obj;
> >>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >>
> >> -     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> >> +     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
> >>
> >>       if (!intel_edp_is_psr_enabled(dev))
> >>               I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> index 3d2c236..69491db 100644
> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
> >>       return 0;
> >>  }
> >>
> >> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> >> +{
> >> +     struct drm_device *dev = ring->dev;
> >> +     int ret;
> >> +
> >> +     if (i915_enable_fbc == 0)
> >> +             return 0;
> >> +
> >> +     if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> >> +             return 0;
> >> +
> >> +     ret = intel_ring_begin(ring, 4);
> >> +     if (ret)
> >> +             return ret;
> >> +     intel_ring_emit(ring, MI_NOOP);
> >> +     /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
> >
> > Another mention of vlv. I can see BSpec makes the same mistake in
> > the register description though.
> 
> ... as you noticed.
> 
> >
> >> +     intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> >> +     intel_ring_emit(ring, MSG_FBC_REND_STATE);
> >> +     intel_ring_emit(ring, FBC_REND_NUKE);
> >> +     intel_ring_advance(ring);
> >> +
> >> +     return 0;
> >> +}
> >> +
> >>  static int
> >>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >>                      u32 invalidate_domains, u32 flush_domains)
> >> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >>       intel_ring_emit(ring, 0);
> >>       intel_ring_advance(ring);
> >>
> >> +     if (flush_domains)
> >> +             return gen7_ring_fbc_flush(ring);
> >> +
> >>       return 0;
> >>  }
> >>
> >> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
> >>  static int blt_ring_flush(struct intel_ring_buffer *ring,
> >>                         u32 invalidate, u32 flush)
> >>  {
> >> +     struct drm_device *dev = ring->dev;
> >>       uint32_t cmd;
> >>       int ret;
> >>
> >> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
> >>       intel_ring_emit(ring, 0);
> >>       intel_ring_emit(ring, MI_NOOP);
> >>       intel_ring_advance(ring);
> >> +
> >> +     if (IS_GEN7(dev))
> >> +             return gen7_ring_fbc_flush(ring);
> >
> > Should check flush_domains here as well?
> 
> here is flush domain by definition, isn' t it?

How so?

> 
> >
> > So we're now using the same nuke mechanism from the blt ring too.
> > Should we then drop the regular blitter tracking things from fbc_enable?
> 
> This is a good question. Since this is a critical patch and it is
> working in the way it is I prefer to let it in the way it is and
> promisse that I will try to drop old blitter tracking for ivb and hsw
> later. If it works I'll send the drop in another patch.
> 
> >
> > Oh and what about vcs and vecs, should we nuke from those rings as well?
> > I guess it would be strange to write to the primary plane's buffer via
> > vcs, but I'm assuming vebox could write the same formats that we can
> > scan out...
> 
> To be truly honest with you I have no idea about these case. specs
> just says to put after every pipe_control following flush
> renderings... and blt.

IIRC the spec doesn't say anything about blt.

Ah, the PM giude tells you to do LRIs w/ blt too. But it actually says
that you should do "cache clean" LRIs insted of "nuke" LRIs.

> 
> >
> >> +
> >>       return 0;
> >>  }
> >>
> >> --
> >> 1.8.1.4
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> 
> 
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

On Mon, Jun 3, 2013 at 8:34 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, May 31, 2013 at 05:15:41PM -0300, Rodrigo Vivi wrote:
>> Hi Ville,
>>
>> Thanks for the comments.
>>
>>
>> On Fri, May 31, 2013 at 12:59 PM, Ville Syrjälä
>> <ville.syrjala@linux.intel.com> wrote:
>> > On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote:
>> >> WaFbcNukeOn3DBlt for IVB, HSW and VLV.
>> >
>> > VLV doesn't have FBC, so this is a bit incorrect.
>>
>> I'm going to remove the vlv mention that incorrectly came from spec...
>>
>> >
>> >>
>> >> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
>> >> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
>> >> follows each render submission."
>> >>
>> >> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>> >>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>> >>     module parameter check.
>> >>
>> >> v3: Adding Wa name as Damien suggested.
>> >>
>> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>> >>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>> >>  3 files changed, 35 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> >> index cc4c223..81ac584 100644
>> >> --- a/drivers/gpu/drm/i915/i915_reg.h
>> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >> @@ -977,6 +977,8 @@
>> >>  /* Framebuffer compression for Ivybridge */
>> >>  #define IVB_FBC_RT_BASE                      0x7020
>> >>
>> >> +#define MSG_FBC_REND_STATE   0x50380
>> >> +#define   FBC_REND_NUKE              (1<<2)
>> >>
>> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_A  0x420B0
>> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_B  0x420B4
>> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> >> index 1879188..e830a9b 100644
>> >> --- a/drivers/gpu/drm/i915/intel_pm.c
>> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> >> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>> >>       struct drm_i915_gem_object *obj = intel_fb->obj;
>> >>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> >>
>> >> -     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
>> >> +     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>> >>
>> >>       if (!intel_edp_is_psr_enabled(dev))
>> >>               I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
>> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> >> index 3d2c236..69491db 100644
>> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> >> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>> >>       return 0;
>> >>  }
>> >>
>> >> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
>> >> +{
>> >> +     struct drm_device *dev = ring->dev;
>> >> +     int ret;
>> >> +
>> >> +     if (i915_enable_fbc == 0)
>> >> +             return 0;
>> >> +
>> >> +     if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
>> >> +             return 0;
>> >> +
>> >> +     ret = intel_ring_begin(ring, 4);
>> >> +     if (ret)
>> >> +             return ret;
>> >> +     intel_ring_emit(ring, MI_NOOP);
>> >> +     /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
>> >
>> > Another mention of vlv. I can see BSpec makes the same mistake in
>> > the register description though.
>>
>> ... as you noticed.
>>
>> >
>> >> +     intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>> >> +     intel_ring_emit(ring, MSG_FBC_REND_STATE);
>> >> +     intel_ring_emit(ring, FBC_REND_NUKE);
>> >> +     intel_ring_advance(ring);
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >>  static int
>> >>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>> >>                      u32 invalidate_domains, u32 flush_domains)
>> >> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>> >>       intel_ring_emit(ring, 0);
>> >>       intel_ring_advance(ring);
>> >>
>> >> +     if (flush_domains)
>> >> +             return gen7_ring_fbc_flush(ring);
>> >> +
>> >>       return 0;
>> >>  }
>> >>
>> >> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>> >>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>> >>                         u32 invalidate, u32 flush)
>> >>  {
>> >> +     struct drm_device *dev = ring->dev;
>> >>       uint32_t cmd;
>> >>       int ret;
>> >>
>> >> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>> >>       intel_ring_emit(ring, 0);
>> >>       intel_ring_emit(ring, MI_NOOP);
>> >>       intel_ring_advance(ring);
>> >> +
>> >> +     if (IS_GEN7(dev))
>> >> +             return gen7_ring_fbc_flush(ring);
>> >
>> > Should check flush_domains here as well?
>>
>> here is flush domain by definition, isn' t it?
>
> How so?

this function is the ring->flush. how is it possible to have it out of
flush domain? or are the names just confusing me?

>
>>
>> >
>> > So we're now using the same nuke mechanism from the blt ring too.
>> > Should we then drop the regular blitter tracking things from fbc_enable?
>>
>> This is a good question. Since this is a critical patch and it is
>> working in the way it is I prefer to let it in the way it is and
>> promisse that I will try to drop old blitter tracking for ivb and hsw
>> later. If it works I'll send the drop in another patch.
>>
>> >
>> > Oh and what about vcs and vecs, should we nuke from those rings as well?
>> > I guess it would be strange to write to the primary plane's buffer via
>> > vcs, but I'm assuming vebox could write the same formats that we can
>> > scan out...
>>
>> To be truly honest with you I have no idea about these case. specs
>> just says to put after every pipe_control following flush
>> renderings... and blt.
>
> IIRC the spec doesn't say anything about blt.
>
> Ah, the PM giude tells you to do LRIs w/ blt too. But it actually says
> that you should do "cache clean" LRIs insted of "nuke" LRIs.

It is just the name, but same address and bit.
from BSPec: " Driver must program a MI_FLUSH_DW followed by a LRI into
the BCS ring to generate a cache clean message to FBC (LRI to offset
0x50380 with data 0x00000002)."

With this blit in the way it is I' m getting best rendering and power
saving performance.

>
>>
>> >
>> >> +
>> >>       return 0;
>> >>  }
>> >>
>> >> --
>> >> 1.8.1.4
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Ville Syrjälä
>> > Intel OTC
>>
>>
>>
>> --
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br
>
> --
> Ville Syrjälä
> Intel OTC



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Ville Syrjälä]

On Mon, Jun 03, 2013 at 01:50:46PM -0300, Rodrigo Vivi wrote:
> On Mon, Jun 3, 2013 at 8:34 AM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Fri, May 31, 2013 at 05:15:41PM -0300, Rodrigo Vivi wrote:
> >> Hi Ville,
> >>
> >> Thanks for the comments.
> >>
> >>
> >> On Fri, May 31, 2013 at 12:59 PM, Ville Syrjälä
> >> <ville.syrjala@linux.intel.com> wrote:
> >> > On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote:
> >> >> WaFbcNukeOn3DBlt for IVB, HSW and VLV.
> >> >
> >> > VLV doesn't have FBC, so this is a bit incorrect.
> >>
> >> I'm going to remove the vlv mention that incorrectly came from spec...
> >>
> >> >
> >> >>
> >> >> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> >> >> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> >> >> follows each render submission."
> >> >>
> >> >> v2: Chris noticed that flush_domains check was missing here and also suggested to do
> >> >>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
> >> >>     module parameter check.
> >> >>
> >> >> v3: Adding Wa name as Damien suggested.
> >> >>
> >> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> >> >> ---
> >> >>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
> >> >>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
> >> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
> >> >>  3 files changed, 35 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> >> index cc4c223..81ac584 100644
> >> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> >> @@ -977,6 +977,8 @@
> >> >>  /* Framebuffer compression for Ivybridge */
> >> >>  #define IVB_FBC_RT_BASE                      0x7020
> >> >>
> >> >> +#define MSG_FBC_REND_STATE   0x50380
> >> >> +#define   FBC_REND_NUKE              (1<<2)
> >> >>
> >> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_A  0x420B0
> >> >>  #define _HSW_PIPE_SLICE_CHICKEN_1_B  0x420B4
> >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> >> index 1879188..e830a9b 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> >> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
> >> >>       struct drm_i915_gem_object *obj = intel_fb->obj;
> >> >>       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >> >>
> >> >> -     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> >> >> +     I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
> >> >>
> >> >>       if (!intel_edp_is_psr_enabled(dev))
> >> >>               I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> >> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> >> index 3d2c236..69491db 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> >> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
> >> >>       return 0;
> >> >>  }
> >> >>
> >> >> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> >> >> +{
> >> >> +     struct drm_device *dev = ring->dev;
> >> >> +     int ret;
> >> >> +
> >> >> +     if (i915_enable_fbc == 0)
> >> >> +             return 0;
> >> >> +
> >> >> +     if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> >> >> +             return 0;
> >> >> +
> >> >> +     ret = intel_ring_begin(ring, 4);
> >> >> +     if (ret)
> >> >> +             return ret;
> >> >> +     intel_ring_emit(ring, MI_NOOP);
> >> >> +     /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
> >> >
> >> > Another mention of vlv. I can see BSpec makes the same mistake in
> >> > the register description though.
> >>
> >> ... as you noticed.
> >>
> >> >
> >> >> +     intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> >> >> +     intel_ring_emit(ring, MSG_FBC_REND_STATE);
> >> >> +     intel_ring_emit(ring, FBC_REND_NUKE);
> >> >> +     intel_ring_advance(ring);
> >> >> +
> >> >> +     return 0;
> >> >> +}
> >> >> +
> >> >>  static int
> >> >>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >> >>                      u32 invalidate_domains, u32 flush_domains)
> >> >> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >> >>       intel_ring_emit(ring, 0);
> >> >>       intel_ring_advance(ring);
> >> >>
> >> >> +     if (flush_domains)
> >> >> +             return gen7_ring_fbc_flush(ring);
> >> >> +
> >> >>       return 0;
> >> >>  }
> >> >>
> >> >> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
> >> >>  static int blt_ring_flush(struct intel_ring_buffer *ring,
> >> >>                         u32 invalidate, u32 flush)
> >> >>  {
> >> >> +     struct drm_device *dev = ring->dev;
> >> >>       uint32_t cmd;
> >> >>       int ret;
> >> >>
> >> >> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
> >> >>       intel_ring_emit(ring, 0);
> >> >>       intel_ring_emit(ring, MI_NOOP);
> >> >>       intel_ring_advance(ring);
> >> >> +
> >> >> +     if (IS_GEN7(dev))
> >> >> +             return gen7_ring_fbc_flush(ring);
> >> >
> >> > Should check flush_domains here as well?
> >>
> >> here is flush domain by definition, isn' t it?
> >
> > How so?
> 
> this function is the ring->flush. how is it possible to have it out of
> flush domain? or are the names just confusing me?

->flush() takes as parameters invalidate and flush domain bitmasks. You
already check for flush_domains in gen7_render_ring_flush(). If you're
just invalidating, nothing gets flushed to the memory and hence the nuke
procedure is not needed.

> >> >
> >> > So we're now using the same nuke mechanism from the blt ring too.
> >> > Should we then drop the regular blitter tracking things from fbc_enable?
> >>
> >> This is a good question. Since this is a critical patch and it is
> >> working in the way it is I prefer to let it in the way it is and
> >> promisse that I will try to drop old blitter tracking for ivb and hsw
> >> later. If it works I'll send the drop in another patch.
> >>
> >> >
> >> > Oh and what about vcs and vecs, should we nuke from those rings as well?
> >> > I guess it would be strange to write to the primary plane's buffer via
> >> > vcs, but I'm assuming vebox could write the same formats that we can
> >> > scan out...
> >>
> >> To be truly honest with you I have no idea about these case. specs
> >> just says to put after every pipe_control following flush
> >> renderings... and blt.
> >
> > IIRC the spec doesn't say anything about blt.
> >
> > Ah, the PM giude tells you to do LRIs w/ blt too. But it actually says
> > that you should do "cache clean" LRIs insted of "nuke" LRIs.
> 
> It is just the name, but same address and bit.

It's not the same bit. Nuke is bit 2, cache clean is bit 1.

> from BSPec: " Driver must program a MI_FLUSH_DW followed by a LRI into
> the BCS ring to generate a cache clean message to FBC (LRI to offset
> 0x50380 with data 0x00000002)."

0x2 == (1 << 1)
aka. the cache clean bit, just like the text said.

> 
> With this blit in the way it is I' m getting best rendering and power
> saving performance.
> 
> >
> >>
> >> >
> >> >> +
> >> >>       return 0;
> >> >>  }
> >> >>
> >> >> --
> >> >> 1.8.1.4
> >> >>
> >> >> _______________________________________________
> >> >> Intel-gfx mailing list
> >> >> Intel-gfx@lists.freedesktop.org
> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> > --
> >> > Ville Syrjälä
> >> > Intel OTC
> >>
> >>
> >>
> >> --
> >> Rodrigo Vivi
> >> Blog: http://blog.vivi.eng.br
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> 
> 
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.

v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  3 +++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc4c223..f37ddee 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -977,6 +977,9 @@
 /* Framebuffer compression for Ivybridge */
 #define IVB_FBC_RT_BASE			0x7020
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
+#define   FBC_REND_CACHE_CLEAN	(1<<1)
 
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1879188..e830a9b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	if (!intel_edp_is_psr_enabled(dev))
 		I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3d2c236..3e24639 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, bool nuke)
+{
+	struct drm_device *dev = ring->dev;
+	int ret;
+
+	if (i915_enable_fbc == 0)
+		return 0;
+
+	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, nuke ? FBC_REND_NUKE : FBC_REND_CACHE_CLEAN);
+	intel_ring_advance(ring);
+
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring, true);
+
 	return 0;
 }
 
@@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int blt_ring_flush(struct intel_ring_buffer *ring,
 			  u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev))
+		return gen7_ring_fbc_flush(ring, false);
+
 	return 0;
 }
 
-- 
1.7.11.7

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

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Ville Syrjälä]

On Mon, Jun 03, 2013 at 03:41:49PM -0300, Rodrigo Vivi wrote:
> WaFbcNukeOn3DBlt for IVB, HSW.
> 
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
> 
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
> 
> v3: Adding Wa name as Damien suggested.
> 
> v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.
> 
> v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  3 +++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc4c223..f37ddee 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -977,6 +977,9 @@
>  /* Framebuffer compression for Ivybridge */
>  #define IVB_FBC_RT_BASE			0x7020
>  
> +#define MSG_FBC_REND_STATE	0x50380
> +#define   FBC_REND_NUKE		(1<<2)
> +#define   FBC_REND_CACHE_CLEAN	(1<<1)
>  
>  #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
>  #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1879188..e830a9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> +	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>  
>  	if (!intel_edp_is_psr_enabled(dev))
>  		I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3d2c236..3e24639 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>  	return 0;
>  }
>  
> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, bool nuke)
> +{
> +	struct drm_device *dev = ring->dev;
> +	int ret;
> +
> +	if (i915_enable_fbc == 0)
> +		return 0;
> +
> +	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> +		return 0;
> +
> +	ret = intel_ring_begin(ring, 4);
> +	if (ret)
> +		return ret;
> +	intel_ring_emit(ring, MI_NOOP);
> +	/* WaFbcNukeOn3DBlt:ivb/hsw */
> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +	intel_ring_emit(ring, MSG_FBC_REND_STATE);
> +	intel_ring_emit(ring, nuke ? FBC_REND_NUKE : FBC_REND_CACHE_CLEAN);
> +	intel_ring_advance(ring);
> +
> +	return 0;
> +}
> +
>  static int
>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  		       u32 invalidate_domains, u32 flush_domains)
> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_advance(ring);
>  
> +	if (flush_domains)
> +		return gen7_ring_fbc_flush(ring, true);
> +
>  	return 0;
>  }
>  
> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>  			  u32 invalidate, u32 flush)
>  {
> +	struct drm_device *dev = ring->dev;
>  	uint32_t cmd;
>  	int ret;
>  
> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_emit(ring, MI_NOOP);
>  	intel_ring_advance(ring);
> +
> +	if (IS_GEN7(dev))
> +		return gen7_ring_fbc_flush(ring, false);

Still no flush_domains check?

Oh and looks like you need to rebase the patch since these functions
got renamed.

> +
>  	return 0;
>  }
>  
> -- 
> 1.7.11.7

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Chris Wilson]

On Tue, Jun 04, 2013 at 10:06:08AM +0300, Ville Syrjälä wrote:
> On Mon, Jun 03, 2013 at 03:41:49PM -0300, Rodrigo Vivi wrote:
> > +	if (IS_GEN7(dev))
> > +		return gen7_ring_fbc_flush(ring, false);
> 
> Still no flush_domains check?

And if we are being picky, not using the fbc_dirty flag either.
Also pass the dword to gen7_ring_fbc_flush() rather than bool nuke, as
it is more descriptive and flexible.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.

v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.

v6: Check for flush domain on blt (by Ville).
    Check for scanout dirty (by Chris).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 36 +++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47a9de0..757bbb7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1026,6 +1026,10 @@
 #define IPS_CTL		0x43408
 #define   IPS_ENABLE	(1 << 31)
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
+#define   FBC_REND_CACHE_CLEAN	(1<<1)
+
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
 #define   HSW_BYPASS_FBC_QUEUE		(1<<22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 50fe3d7..324d1ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0e72da6..cec5246 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,34 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_gem_object *obj = ring->obj;
+	int ret;
+
+	if (i915_enable_fbc == 0)
+		return 0;
+
+	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
+		return 0;
+
+	if (obj->base.write_domain != I915_GEM_DOMAIN_GTT)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, value);
+	intel_ring_advance(ring);
+
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +364,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
 	return 0;
 }
 
@@ -1685,6 +1716,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int gen6_ring_flush(struct intel_ring_buffer *ring,
 			   u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1707,6 +1739,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev) && flush)
+		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
+
 	return 0;
 }
 
-- 
1.7.11.7

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

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Chris Wilson]

On Thu, Jun 06, 2013 at 11:49:56AM -0300, Rodrigo Vivi wrote:
> WaFbcNukeOn3DBlt for IVB, HSW.
> 
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
> 
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
> 
> v3: Adding Wa name as Damien suggested.
> 
> v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.
> 
> v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.
> 
> v6: Check for flush domain on blt (by Ville).
>     Check for scanout dirty (by Chris).
Note quite what I had in mind, see
https://patchwork.kernel.org/patch/2606131/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

please just ignore this version...
going to try fbc_dirty and other changes here...

On Thu, Jun 6, 2013 at 12:00 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Jun 06, 2013 at 11:49:56AM -0300, Rodrigo Vivi wrote:
>> WaFbcNukeOn3DBlt for IVB, HSW.
>>
>> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
>> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
>> follows each render submission."
>>
>> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>>     module parameter check.
>>
>> v3: Adding Wa name as Damien suggested.
>>
>> v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.
>>
>> v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.
>>
>> v6: Check for flush domain on blt (by Ville).
>>     Check for scanout dirty (by Chris).
> Note quite what I had in mind, see
> https://patchwork.kernel.org/patch/2606131/
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[PATCH 1/2] drm/i915: Track when we dirty the scanout with render commands

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

This is required for tracking render damage for use with FBC and will be
used in subsequent patches.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c       | 13 +++++++++----
 drivers/gpu/drm/i915/intel_drv.h           |  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  1 +
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a8bb62c..c98333d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -786,7 +786,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *objects,
 			obj->dirty = 1;
 			obj->last_write_seqno = intel_ring_get_seqno(ring);
 			if (obj->pin_count) /* check for potential scanout */
-				intel_mark_fb_busy(obj);
+				intel_mark_fb_busy(obj, ring);
 		}
 
 		trace_i915_gem_object_change_domain(obj, old_read, old_write);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 40d047e..657f9a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7042,7 +7042,8 @@ void intel_mark_idle(struct drm_device *dev)
 	}
 }
 
-void intel_mark_fb_busy(struct drm_i915_gem_object *obj)
+void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
+			struct intel_ring_buffer *ring)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_crtc *crtc;
@@ -7054,8 +7055,12 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj)
 		if (!crtc->fb)
 			continue;
 
-		if (to_intel_framebuffer(crtc->fb)->obj == obj)
-			intel_increase_pllclock(crtc);
+		if (to_intel_framebuffer(crtc->fb)->obj != obj)
+			continue;
+
+		intel_increase_pllclock(crtc);
+		if (ring && intel_fbc_enabled(dev))
+			ring->fbc_dirty = true;
 	}
 }
 
@@ -7505,7 +7510,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 
 	intel_disable_fbc(dev);
-	intel_mark_fb_busy(obj);
+	intel_mark_fb_busy(obj, NULL);
 	mutex_unlock(&dev->struct_mutex);
 
 	trace_i915_flip_request(intel_crtc->plane, obj);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eae3dbc..2f87652 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -563,7 +563,8 @@ extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
 extern void intel_dvo_init(struct drm_device *dev);
 extern void intel_tv_init(struct drm_device *dev);
 extern void intel_mark_busy(struct drm_device *dev);
-extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj);
+extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
+			       struct intel_ring_buffer *ring);
 extern void intel_mark_idle(struct drm_device *dev);
 extern bool intel_lvds_init(struct drm_device *dev);
 extern bool intel_is_dual_link_lvds(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 4c7e103..efc403d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -140,6 +140,7 @@ struct  intel_ring_buffer {
 	 */
 	u32 outstanding_lazy_request;
 	bool gpu_caches_dirty;
+	bool fbc_dirty;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.7.11.7

[PATCH 1/2] drm/i915: Track when we dirty the scanout with render commands

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

This is required for tracking render damage for use with FBC and will be
used in subsequent patches.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c       | 13 +++++++++----
 drivers/gpu/drm/i915/intel_drv.h           |  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  1 +
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a8bb62c..c98333d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -786,7 +786,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *objects,
 			obj->dirty = 1;
 			obj->last_write_seqno = intel_ring_get_seqno(ring);
 			if (obj->pin_count) /* check for potential scanout */
-				intel_mark_fb_busy(obj);
+				intel_mark_fb_busy(obj, ring);
 		}
 
 		trace_i915_gem_object_change_domain(obj, old_read, old_write);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 40d047e..657f9a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7042,7 +7042,8 @@ void intel_mark_idle(struct drm_device *dev)
 	}
 }
 
-void intel_mark_fb_busy(struct drm_i915_gem_object *obj)
+void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
+			struct intel_ring_buffer *ring)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_crtc *crtc;
@@ -7054,8 +7055,12 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj)
 		if (!crtc->fb)
 			continue;
 
-		if (to_intel_framebuffer(crtc->fb)->obj == obj)
-			intel_increase_pllclock(crtc);
+		if (to_intel_framebuffer(crtc->fb)->obj != obj)
+			continue;
+
+		intel_increase_pllclock(crtc);
+		if (ring && intel_fbc_enabled(dev))
+			ring->fbc_dirty = true;
 	}
 }
 
@@ -7505,7 +7510,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 
 	intel_disable_fbc(dev);
-	intel_mark_fb_busy(obj);
+	intel_mark_fb_busy(obj, NULL);
 	mutex_unlock(&dev->struct_mutex);
 
 	trace_i915_flip_request(intel_crtc->plane, obj);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eae3dbc..2f87652 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -563,7 +563,8 @@ extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
 extern void intel_dvo_init(struct drm_device *dev);
 extern void intel_tv_init(struct drm_device *dev);
 extern void intel_mark_busy(struct drm_device *dev);
-extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj);
+extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
+			       struct intel_ring_buffer *ring);
 extern void intel_mark_idle(struct drm_device *dev);
 extern bool intel_lvds_init(struct drm_device *dev);
 extern bool intel_is_dual_link_lvds(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 4c7e103..efc403d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -140,6 +140,7 @@ struct  intel_ring_buffer {
 	 */
 	u32 outstanding_lazy_request;
 	bool gpu_caches_dirty;
+	bool fbc_dirty;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.7.11.7

[PATCH 2/2] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.

v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.

v6: Check for flush domain on blt (by Ville).
    Check for scanout dirty (by Chris).

v7: Apply proper fbc_dirty implemented by Chris.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47a9de0..757bbb7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1026,6 +1026,10 @@
 #define IPS_CTL		0x43408
 #define   IPS_ENABLE	(1 << 31)
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
+#define   FBC_REND_CACHE_CLEAN	(1<<1)
+
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
 #define   HSW_BYPASS_FBC_QUEUE		(1<<22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 50fe3d7..324d1ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0e72da6..85f06a8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,29 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_gem_object *obj = ring->obj;
+	int ret;
+
+	if (!ring->fbc_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, value);
+	intel_ring_advance(ring);
+
+	ring->fbc_dirty = false;
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +359,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
 	return 0;
 }
 
@@ -1685,6 +1711,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int gen6_ring_flush(struct intel_ring_buffer *ring,
 			   u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1707,6 +1734,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev) && flush)
+		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
+
 	return 0;
 }
 
-- 
1.7.11.7

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

[PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.

v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.

v6: Check for flush domain on blt (by Ville).
    Check for scanout dirty (by Chris).

v7: Apply proper fbc_dirty implemented by Chris.

v8: remove unused variables.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 29 +++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47a9de0..757bbb7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1026,6 +1026,10 @@
 #define IPS_CTL		0x43408
 #define   IPS_ENABLE	(1 << 31)
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
+#define   FBC_REND_CACHE_CLEAN	(1<<1)
+
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
 #define   HSW_BYPASS_FBC_QUEUE		(1<<22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 50fe3d7..324d1ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0e72da6..1ef081c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,27 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value)
+{
+	int ret;
+
+	if (!ring->fbc_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, value);
+	intel_ring_advance(ring);
+
+	ring->fbc_dirty = false;
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +357,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
 	return 0;
 }
 
@@ -1685,6 +1709,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int gen6_ring_flush(struct intel_ring_buffer *ring,
 			   u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1707,6 +1732,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev) && flush)
+		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
+
 	return 0;
 }
 
-- 
1.7.11.7

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

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Daniel Vetter]

On Thu, Jun 06, 2013 at 04:58:16PM -0300, Rodrigo Vivi wrote:
> WaFbcNukeOn3DBlt for IVB, HSW.
> 
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
> 
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
> 
> v3: Adding Wa name as Damien suggested.
> 
> v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.
> 
> v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one.
> 
> v6: Check for flush domain on blt (by Ville).
>     Check for scanout dirty (by Chris).
> 
> v7: Apply proper fbc_dirty implemented by Chris.
> 
> v8: remove unused variables.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

Both queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  4 ++++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 29 +++++++++++++++++++++++++++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 47a9de0..757bbb7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1026,6 +1026,10 @@
>  #define IPS_CTL		0x43408
>  #define   IPS_ENABLE	(1 << 31)
>  
> +#define MSG_FBC_REND_STATE	0x50380
> +#define   FBC_REND_NUKE		(1<<2)
> +#define   FBC_REND_CACHE_CLEAN	(1<<1)
> +
>  #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
>  #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
>  #define   HSW_BYPASS_FBC_QUEUE		(1<<22)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 50fe3d7..324d1ce 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> +	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>  
>  	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
>  		   IVB_DPFC_CTL_FENCE_EN |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 0e72da6..1ef081c 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -280,6 +280,27 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>  	return 0;
>  }
>  
> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value)
> +{
> +	int ret;
> +
> +	if (!ring->fbc_dirty)
> +		return 0;
> +
> +	ret = intel_ring_begin(ring, 4);
> +	if (ret)
> +		return ret;
> +	intel_ring_emit(ring, MI_NOOP);
> +	/* WaFbcNukeOn3DBlt:ivb/hsw */
> +	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +	intel_ring_emit(ring, MSG_FBC_REND_STATE);
> +	intel_ring_emit(ring, value);
> +	intel_ring_advance(ring);
> +
> +	ring->fbc_dirty = false;
> +	return 0;
> +}
> +
>  static int
>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  		       u32 invalidate_domains, u32 flush_domains)
> @@ -336,6 +357,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_advance(ring);
>  
> +	if (flush_domains)
> +		return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
> +
>  	return 0;
>  }
>  
> @@ -1685,6 +1709,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  static int gen6_ring_flush(struct intel_ring_buffer *ring,
>  			   u32 invalidate, u32 flush)
>  {
> +	struct drm_device *dev = ring->dev;
>  	uint32_t cmd;
>  	int ret;
>  
> @@ -1707,6 +1732,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring,
>  	intel_ring_emit(ring, 0);
>  	intel_ring_emit(ring, MI_NOOP);
>  	intel_ring_advance(ring);
> +
> +	if (IS_GEN7(dev) && flush)
> +		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

WaFbcNukeOn3DBlt for IVB, HSW.

According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
follows each render submission."

v2: Chris noticed that flush_domains check was missing here and also suggested to do
    LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
    module parameter check.

v3: Adding Wa name as Damien suggested.

v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h         |  2 ++
 drivers/gpu/drm/i915/intel_pm.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc4c223..81ac584 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -977,6 +977,8 @@
 /* Framebuffer compression for Ivybridge */
 #define IVB_FBC_RT_BASE			0x7020
 
+#define MSG_FBC_REND_STATE	0x50380
+#define   FBC_REND_NUKE		(1<<2)
 
 #define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
 #define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1879188..e830a9b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
 
 	if (!intel_edp_is_psr_enabled(dev))
 		I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3d2c236..46df1ff 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
 	return 0;
 }
 
+static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
+{
+	struct drm_device *dev = ring->dev;
+	int ret;
+
+	if (i915_enable_fbc == 0)
+		return 0;
+
+	if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+	intel_ring_emit(ring, MI_NOOP);
+	/* WaFbcNukeOn3DBlt:ivb/hsw */
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, MSG_FBC_REND_STATE);
+	intel_ring_emit(ring, FBC_REND_NUKE);
+	intel_ring_advance(ring);
+
+	return 0;
+}
+
 static int
 gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
@@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (flush_domains)
+		return gen7_ring_fbc_flush(ring);
+
 	return 0;
 }
 
@@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 static int blt_ring_flush(struct intel_ring_buffer *ring,
 			  u32 invalidate, u32 flush)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t cmd;
 	int ret;
 
@@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
+
+	if (IS_GEN7(dev))
+		return gen7_ring_fbc_flush(ring);
+
 	return 0;
 }
 
-- 
1.7.11.7

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

Re: [PATCH] drm/i915: WA: FBC Render Nuke.

[Rodrigo Vivi]

ops, ignore this... you were fast than my git send-email....
you are right.. my bad... going to test this clean version here...

On Mon, Jun 3, 2013 at 2:08 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> WaFbcNukeOn3DBlt for IVB, HSW.
>
> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> follows each render submission."
>
> v2: Chris noticed that flush_domains check was missing here and also suggested to do
>     LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
>     module parameter check.
>
> v3: Adding Wa name as Damien suggested.
>
> v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c         |  2 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc4c223..81ac584 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -977,6 +977,8 @@
>  /* Framebuffer compression for Ivybridge */
>  #define IVB_FBC_RT_BASE                        0x7020
>
> +#define MSG_FBC_REND_STATE     0x50380
> +#define   FBC_REND_NUKE                (1<<2)
>
>  #define _HSW_PIPE_SLICE_CHICKEN_1_A    0x420B0
>  #define _HSW_PIPE_SLICE_CHICKEN_1_B    0x420B4
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1879188..e830a9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>         struct drm_i915_gem_object *obj = intel_fb->obj;
>         struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> -       I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
> +       I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset);
>
>         if (!intel_edp_is_psr_enabled(dev))
>                 I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3d2c236..46df1ff 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
>         return 0;
>  }
>
> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> +{
> +       struct drm_device *dev = ring->dev;
> +       int ret;
> +
> +       if (i915_enable_fbc == 0)
> +               return 0;
> +
> +       if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> +               return 0;
> +
> +       ret = intel_ring_begin(ring, 4);
> +       if (ret)
> +               return ret;
> +       intel_ring_emit(ring, MI_NOOP);
> +       /* WaFbcNukeOn3DBlt:ivb/hsw */
> +       intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> +       intel_ring_emit(ring, MSG_FBC_REND_STATE);
> +       intel_ring_emit(ring, FBC_REND_NUKE);
> +       intel_ring_advance(ring);
> +
> +       return 0;
> +}
> +
>  static int
>  gen7_render_ring_flush(struct intel_ring_buffer *ring,
>                        u32 invalidate_domains, u32 flush_domains)
> @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
>         intel_ring_emit(ring, 0);
>         intel_ring_advance(ring);
>
> +       if (flush_domains)
> +               return gen7_ring_fbc_flush(ring);
> +
>         return 0;
>  }
>
> @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  static int blt_ring_flush(struct intel_ring_buffer *ring,
>                           u32 invalidate, u32 flush)
>  {
> +       struct drm_device *dev = ring->dev;
>         uint32_t cmd;
>         int ret;
>
> @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
>         intel_ring_emit(ring, 0);
>         intel_ring_emit(ring, MI_NOOP);
>         intel_ring_advance(ring);
> +
> +       if (IS_GEN7(dev))
> +               return gen7_ring_fbc_flush(ring);
> +
>         return 0;
>  }
>
> --
> 1.7.11.7
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br