* [PATCH 0/5] pile of ilk related workarounds
@ 2012-10-18 9:49 Daniel Vetter
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Hi all,
Dave reported a gpu hang regression on his ilk, likely due to the re-enabled rc6
support (but not confirmed). So I've scrounged through the w/a database and
found a few bits. Preliminary verdict is that this seems to prevent the hangs.
If that stands for a few more days, I'd like to merge patches 1-3 to -fixes, the
last two for dinq.
Please review.
Thanks, Daniel
Daniel Vetter (5):
drm/i915: implement WaIssueDummyWriteToWakupFromRC6
drm/i915: implement WaDisableRenderCachePipelinedFlush
drm/i915: WaInsertNoopAfterBatchEndCommand
drm/i915: implement WaDisableSpriteDestColorKey
drm/i915: document a few more already implemented ilk w/as
drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 11 ++++++++++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++++++++---
drivers/gpu/drm/i915/intel_sprite.c | 27 +++++++++++++++++++++++----
6 files changed, 64 insertions(+), 8 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
@ 2012-10-18 9:49 ` Daniel Vetter
2012-10-18 11:10 ` Chris Wilson
` (2 more replies)
2012-10-18 9:49 ` [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush Daniel Vetter
` (3 subsequent siblings)
4 siblings, 3 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Or at least our best understanding of it.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..c3f4f04 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
return true;
}
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+ /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
+ * touching it for real. MI_MODE is masked, hence harmless to write 0
+ * into. */
+ I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
#define __i915_read(x, y) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
u##x val = 0; \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
unsigned long irqflags; \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
write##y(val, dev_priv->regs + reg + 0x180000); \
} else { \
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
@ 2012-10-18 9:49 ` Daniel Vetter
2012-10-18 11:21 ` Chris Wilson
2012-10-18 9:49 ` [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand Daniel Vetter
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Comment says for eaglelake/cantiga, but it's listed in the ilk table,
too. So apply it to both.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a4162dd..1970c54 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -661,6 +661,7 @@
#define MI_ARB_DISPLAY_PRIORITY_B_A (1 << 0) /* display B > display A */
#define CACHE_MODE_0 0x02120 /* 915+ only */
+#define CM0_PIPELINED_RENDER_FLUSH_DISABLE (1<<8)
#define CM0_IZ_OPT_DISABLE (1<<6)
#define CM0_ZR_OPT_DISABLE (1<<5)
#define CM0_STC_EVICT_DISABLE_LRA_SNB (1<<5)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 72f41aa..aa77639 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3397,6 +3397,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
I915_WRITE(_3D_CHICKEN2,
_3D_CHICKEN2_WM_READ_PIPELINED << 16 |
_3D_CHICKEN2_WM_READ_PIPELINED);
+
+ /* WaDisableRenderCachePipelinedFlush */
+ I915_WRITE(CACHE_MODE_0,
+ _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE));
}
static void gen6_init_clock_gating(struct drm_device *dev)
@@ -3728,6 +3732,10 @@ static void g4x_init_clock_gating(struct drm_device *dev)
if (IS_GM45(dev))
dspclk_gate |= DSSUNIT_CLOCK_GATE_DISABLE;
I915_WRITE(DSPCLK_GATE_D, dspclk_gate);
+
+ /* WaDisableRenderCachePipelinedFlush */
+ I915_WRITE(CACHE_MODE_0,
+ _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE));
}
static void crestline_init_clock_gating(struct drm_device *dev)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
2012-10-18 9:49 ` [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush Daniel Vetter
@ 2012-10-18 9:49 ` Daniel Vetter
2012-10-18 11:24 ` Chris Wilson
2012-10-18 9:49 ` [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey Daniel Vetter
2012-10-18 9:49 ` [PATCH 5/5] drm/i915: document a few more already implemented ilk w/as Daniel Vetter
4 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Comment says that this applies to earlier gens, too. Since two more
MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
to gen2.
v2: Correct the ringbuffer dword count for gen3, spotted by Chris
Wilson.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 984a0c5..38092dc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -969,7 +969,7 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
{
int ret;
- ret = intel_ring_begin(ring, 2);
+ ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
@@ -978,6 +978,11 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
MI_BATCH_GTT |
MI_BATCH_NON_SECURE_I965);
intel_ring_emit(ring, offset);
+ /* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+ * the batchbuffer start command. Unclear whether really required on
+ * gen3, but better safe than sorry. */
+ intel_ring_emit(ring, MI_NOOP);
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
@@ -996,7 +1001,10 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring,
intel_ring_emit(ring, MI_BATCH_BUFFER);
intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
intel_ring_emit(ring, offset + len - 8);
- intel_ring_emit(ring, 0);
+ /* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+ * the batchbuffer start command. Unclear whether really required on
+ * gen2, but better safe than sorry. */
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
@@ -1008,12 +1016,16 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring,
{
int ret;
- ret = intel_ring_begin(ring, 2);
+ ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
+ /* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+ * the batchbuffer start command. */
+ intel_ring_emit(ring, MI_NOOP);
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
` (2 preceding siblings ...)
2012-10-18 9:49 ` [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand Daniel Vetter
@ 2012-10-18 9:49 ` Daniel Vetter
2012-10-18 11:32 ` Chris Wilson
2012-10-18 9:49 ` [PATCH 5/5] drm/i915: document a few more already implemented ilk w/as Daniel Vetter
4 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Unfortunately this requires a bit of book-keeping to return the
right values for get_colorkey and to set things up correctly
when re-enabling the plane.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 27 +++++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 79f8ed6..e935e75 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -220,6 +220,8 @@ struct intel_plane {
struct drm_i915_gem_object *obj;
int max_downscale;
u32 lut_r[1024], lut_g[1024], lut_b[1024];
+ bool restore_ilk_dest_key; /* for a w/a */
+
void (*update_plane)(struct drm_plane *plane,
struct drm_framebuffer *fb,
struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7644f31..8bcacd5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -230,6 +230,13 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
dvscntr = I915_READ(DVSCNTR(pipe));
+ WARN_ON(!(dvscntr & DVS_ENABLE) && (dvscntr && DVS_DEST_KEY));
+
+ if (intel_plane->restore_ilk_dest_key) {
+ dvscntr |= DVS_DEST_KEY;
+ intel_plane->restore_ilk_dest_key = false;
+ }
+
/* Mask out pixel format bits in case we change it */
dvscntr &= ~DVS_PIXFORMAT_MASK;
dvscntr &= ~DVS_RGB_ORDER_XBGR;
@@ -311,8 +318,16 @@ ilk_disable_plane(struct drm_plane *plane)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane *intel_plane = to_intel_plane(plane);
int pipe = intel_plane->pipe;
+ uint32_t tmp;
- I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
+ /* WaDisableSpriteDestColorKey: We need to disable the dest key when
+ * disabling the sprite. */
+ tmp = I915_READ(DVSCNTR(pipe));
+ if (IS_GEN5(dev) &&(tmp & DVS_DEST_KEY))
+ intel_plane->restore_ilk_dest_key = true;
+ tmp &= ~(DVS_ENABLE | DVS_DEST_KEY);
+
+ I915_WRITE(DVSCNTR(pipe), tmp);
/* Disable the scaler */
I915_WRITE(DVSSCALE(pipe), 0);
/* Flush double buffered register updates */
@@ -365,6 +380,7 @@ ilk_update_colorkey(struct drm_plane *plane,
int ret = 0;
intel_plane = to_intel_plane(plane);
+ intel_plane->restore_ilk_dest_key = false;
I915_WRITE(DVSKEYVAL(intel_plane->pipe), key->min_value);
I915_WRITE(DVSKEYMAX(intel_plane->pipe), key->max_value);
@@ -372,9 +388,12 @@ ilk_update_colorkey(struct drm_plane *plane,
dvscntr = I915_READ(DVSCNTR(intel_plane->pipe));
dvscntr &= ~(DVS_SOURCE_KEY | DVS_DEST_KEY);
- if (key->flags & I915_SET_COLORKEY_DESTINATION)
- dvscntr |= DVS_DEST_KEY;
- else if (key->flags & I915_SET_COLORKEY_SOURCE)
+ if (key->flags & I915_SET_COLORKEY_DESTINATION) {
+ if ((dvscntr & DVS_ENABLE) || !IS_GEN5(dev))
+ dvscntr |= DVS_DEST_KEY;
+ else
+ intel_plane->restore_ilk_dest_key = true;
+ } else if (key->flags & I915_SET_COLORKEY_SOURCE)
dvscntr |= DVS_SOURCE_KEY;
I915_WRITE(DVSCNTR(intel_plane->pipe), dvscntr);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] drm/i915: document a few more already implemented ilk w/as
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
` (3 preceding siblings ...)
2012-10-18 9:49 ` [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey Daniel Vetter
@ 2012-10-18 9:49 ` Daniel Vetter
4 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 9:49 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Found while strolling docs.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_pm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index aa77639..45ffb2c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3342,6 +3342,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
/* Required for CxSR */
dspclk_gate |= DPARBUNIT_CLOCK_GATE_DISABLE;
+ /* WaDisableSVSMUnitClockGating */
I915_WRITE(PCH_3DCGDIS0,
MARIUNIT_CLOCK_GATE_DISABLE |
SVSMUNIT_CLOCK_GATE_DISABLE);
@@ -3353,7 +3354,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
/*
* According to the spec the following bits should be set in
* order to enable memory self-refresh
- * The bit 22/21 of 0x42004
+ * The bit 22/21 of 0x42004 (WaDisableDisplayFetchStrideStreching)
* The bit 5 of 0x42020
* The bit 15 of 0x45000
*/
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
@ 2012-10-18 11:10 ` Chris Wilson
2012-10-18 12:14 ` [PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6 Daniel Vetter
2012-10-18 12:16 ` Daniel Vetter
2012-10-18 11:17 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Chris Wilson
2012-10-18 11:25 ` Lespiau, Damien
2 siblings, 2 replies; 17+ messages in thread
From: Chris Wilson @ 2012-10-18 11:10 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Or at least our best understanding of it.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Looks legit. Just a minor alteration to include the w/a name inside
ilk_dummy_write() as well.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
2012-10-18 11:10 ` Chris Wilson
@ 2012-10-18 11:17 ` Chris Wilson
2012-10-18 11:21 ` Daniel Vetter
2012-10-18 11:25 ` Lespiau, Damien
2 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2012-10-18 11:17 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> #define __i915_read(x, y) \
> u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
> u##x val = 0; \
> + if (IS_GEN5(dev_priv->dev)) \
> + ilk_dummy_write(dev_priv); \
IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must
go through and add an __INTEL_INFO(dev_priv) and so
#define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5)
#define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev))
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6
2012-10-18 11:17 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Chris Wilson
@ 2012-10-18 11:21 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 11:21 UTC (permalink / raw)
To: Chris Wilson; +Cc: Intel Graphics Development
On Thu, Oct 18, 2012 at 1:17 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> #define __i915_read(x, y) \
>> u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
>> u##x val = 0; \
>> + if (IS_GEN5(dev_priv->dev)) \
>> + ilk_dummy_write(dev_priv); \
>
> IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must
> go through and add an __INTEL_INFO(dev_priv) and so
> #define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5)
> #define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev))
We could also teach the drm setup and teardown code manners and embed
struct drm_device into our own device struct and call it a day. Would
kill all that pointless circular pointer chasing. Been on my todo
since ages, will probably stay there for a while ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush
2012-10-18 9:49 ` [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush Daniel Vetter
@ 2012-10-18 11:21 ` Chris Wilson
2012-10-29 21:33 ` Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2012-10-18 11:21 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 18 Oct 2012 11:49:51 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Comment says for eaglelake/cantiga, but it's listed in the ilk table,
> too. So apply it to both.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Looks harmless due to the massive number of other p/c errata and that
pipelined render cache flushes have never been relied upon.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand
2012-10-18 9:49 ` [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand Daniel Vetter
@ 2012-10-18 11:24 ` Chris Wilson
2012-10-18 12:19 ` [PATCH] " Daniel Vetter
0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2012-10-18 11:24 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 18 Oct 2012 11:49:52 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Comment says that this applies to earlier gens, too. Since two more
> MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
> to gen2.
>
> v2: Correct the ringbuffer dword count for gen3, spotted by Chris
> Wilson.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I'm just not sold on this one, surely we would have spotted a need
before now? 12 extra cycles after every batch! :-p
Can you please fix up the cut'n'paste comments to not refer to
generations that never call the function?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
2012-10-18 11:10 ` Chris Wilson
2012-10-18 11:17 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Chris Wilson
@ 2012-10-18 11:25 ` Lespiau, Damien
2 siblings, 0 replies; 17+ messages in thread
From: Lespiau, Damien @ 2012-10-18 11:25 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
On Thu, Oct 18, 2012 at 10:49 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> +static void
> +ilk_dummy_write(struct drm_i915_private *dev_priv)
> +{
> + /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
> + * touching it for real. MI_MODE is masked, hence harmless to write 0
> + * into. */
I'd document the wa implemented here.
The summary line is missing an 'e' in Wake, it makes it harder to look for it.
--
Damien
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey
2012-10-18 9:49 ` [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey Daniel Vetter
@ 2012-10-18 11:32 ` Chris Wilson
0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2012-10-18 11:32 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
On Thu, 18 Oct 2012 11:49:53 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Unfortunately this requires a bit of book-keeping to return the
> right values for get_colorkey and to set things up correctly
> when re-enabling the plane.
Needs to worry about per-pipe values and ilk_get_colorkey().
Maybe the code would be simpler if we unconditionally shadowed
DVSCNTR[I915_NUM_PIPES].
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6
2012-10-18 11:10 ` Chris Wilson
@ 2012-10-18 12:14 ` Daniel Vetter
2012-10-18 12:16 ` Daniel Vetter
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 12:14 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Or at least our best understanding of it.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..c3f4f04 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
return true;
}
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+ /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
+ * touching it for real. MI_MODE is masked, hence harmless to write 0
+ * into. */
+ I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
#define __i915_read(x, y) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
u##x val = 0; \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
unsigned long irqflags; \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
write##y(val, dev_priv->regs + reg + 0x180000); \
} else { \
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6
2012-10-18 11:10 ` Chris Wilson
2012-10-18 12:14 ` [PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6 Daniel Vetter
@ 2012-10-18 12:16 ` Daniel Vetter
1 sibling, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 12:16 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Or at least our best understanding of it.
v2: Fixup commit message and put the wa name into the comment block.
And actually update the commit, too.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_drv.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..fb6b633 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
return true;
}
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+ /* WaIssueDummyWriteToWakeupFromRC6: Issue a dummy write to wake up the
+ * chip from rc6 before touching it for real. MI_MODE is masked, hence
+ * harmless to write 0 into. */
+ I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
#define __i915_read(x, y) \
u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
u##x val = 0; \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
unsigned long irqflags; \
spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
+ if (IS_GEN5(dev_priv->dev)) \
+ ilk_dummy_write(dev_priv); \
if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
write##y(val, dev_priv->regs + reg + 0x180000); \
} else { \
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] drm/i915: WaInsertNoopAfterBatchEndCommand
2012-10-18 11:24 ` Chris Wilson
@ 2012-10-18 12:19 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-18 12:19 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter
Comment says that this applies to earlier gens, too. Since two more
MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
to gen2.
v2: Correct the ringbuffer dword count for gen3, spotted by Chris
Wilson.
v3: Fixup the comments.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 984a0c5..90db51d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -969,7 +969,7 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
{
int ret;
- ret = intel_ring_begin(ring, 2);
+ ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
@@ -978,6 +978,10 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
MI_BATCH_GTT |
MI_BATCH_NON_SECURE_I965);
intel_ring_emit(ring, offset);
+ /* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+ * the batchbuffer start command. */
+ intel_ring_emit(ring, MI_NOOP);
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
@@ -996,7 +1000,10 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring,
intel_ring_emit(ring, MI_BATCH_BUFFER);
intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
intel_ring_emit(ring, offset + len - 8);
- intel_ring_emit(ring, 0);
+ /* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+ * the batchbuffer start command. Unclear whether really required on
+ * gen2, but better safe than sorry. */
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
@@ -1008,12 +1015,17 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring,
{
int ret;
- ret = intel_ring_begin(ring, 2);
+ ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
+ /* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+ * the batchbuffer start command. Unclear whether really required on
+ * gen3, but better safe than sorry. */
+ intel_ring_emit(ring, MI_NOOP);
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
return 0;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush
2012-10-18 11:21 ` Chris Wilson
@ 2012-10-29 21:33 ` Daniel Vetter
0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2012-10-29 21:33 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, Intel Graphics Development
On Thu, Oct 18, 2012 at 12:21:17PM +0100, Chris Wilson wrote:
> On Thu, 18 Oct 2012 11:49:51 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Comment says for eaglelake/cantiga, but it's listed in the ilk table,
> > too. So apply it to both.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Looks harmless due to the massive number of other p/c errata and that
> pipelined render cache flushes have never been relied upon.
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
I've queued the first two here for -next (since they don't seem to fix any
reported bugs).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-10-29 21:32 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-18 9:49 [PATCH 0/5] pile of ilk related workarounds Daniel Vetter
2012-10-18 9:49 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Daniel Vetter
2012-10-18 11:10 ` Chris Wilson
2012-10-18 12:14 ` [PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6 Daniel Vetter
2012-10-18 12:16 ` Daniel Vetter
2012-10-18 11:17 ` [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6 Chris Wilson
2012-10-18 11:21 ` Daniel Vetter
2012-10-18 11:25 ` Lespiau, Damien
2012-10-18 9:49 ` [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush Daniel Vetter
2012-10-18 11:21 ` Chris Wilson
2012-10-29 21:33 ` Daniel Vetter
2012-10-18 9:49 ` [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand Daniel Vetter
2012-10-18 11:24 ` Chris Wilson
2012-10-18 12:19 ` [PATCH] " Daniel Vetter
2012-10-18 9:49 ` [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey Daniel Vetter
2012-10-18 11:32 ` Chris Wilson
2012-10-18 9:49 ` [PATCH 5/5] drm/i915: document a few more already implemented ilk w/as Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).