* [PATCH 1/3] drm/i915: use gtfifodbg
2012-02-09 9:15 [PATCH 0/3 v3] GTFIFODBG Ben Widawsky
@ 2012-02-09 9:15 ` Ben Widawsky
2012-02-09 9:15 ` [PATCH 2/3] drm/i915: catch gtfifo errors on forcewake_put Ben Widawsky
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-02-09 9:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
Add register definitions for GTFIFODBG, and clear it during init time to
make sure state is correct.
This register tells us if either a read, or a write occurred while the
fifo was full. It seems like bit 2 is an OR of bit 0 and bit 1, so we
check that as well, but the documents are not quite clear.
Reviewed-by (v1): Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by (v1): Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c3afb78..1410fc4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3614,6 +3614,12 @@
#define ECOBUS 0xa180
#define FORCEWAKE_MT_ENABLE (1<<5)
+#define GTFIFODBG 0x120000
+#define GT_FIFO_CPU_ERROR_MASK 7
+#define GT_FIFO_OVFERR (1<<2)
+#define GT_FIFO_IAWRERR (1<<1)
+#define GT_FIFO_IARDERR (1<<0)
+
#define GT_FIFO_FREE_ENTRIES 0x120008
#define GT_FIFO_NUM_RESERVED_ENTRIES 20
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ebe71ed..4f1205e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8204,6 +8204,7 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
u32 gt_perf_status = I915_READ(GEN6_GT_PERF_STATUS);
u32 pcu_mbox, rc6_mask = 0;
+ u32 gtfifodbg;
int cur_freq, min_freq, max_freq;
int i;
@@ -8215,6 +8216,13 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
*/
I915_WRITE(GEN6_RC_STATE, 0);
mutex_lock(&dev_priv->dev->struct_mutex);
+
+ /* Clear the DBG now so we don't confuse earlier errors */
+ if ((gtfifodbg = I915_READ(GTFIFODBG))) {
+ DRM_ERROR("GT fifo had a previous error %x\n", gtfifodbg);
+ I915_WRITE(GTFIFODBG, gtfifodbg);
+ }
+
gen6_gt_force_wake_get(dev_priv);
/* disable the counters and set deterministic thresholds */
--
1.7.9
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] drm/i915: catch gtfifo errors on forcewake_put
2012-02-09 9:15 [PATCH 0/3 v3] GTFIFODBG Ben Widawsky
2012-02-09 9:15 ` [PATCH 1/3] drm/i915: use gtfifodbg Ben Widawsky
@ 2012-02-09 9:15 ` Ben Widawsky
2012-02-09 9:15 ` [PATCH 3/3] drm/i915: check gtfifodbg after possibly failed writes Ben Widawsky
2012-02-11 16:22 ` [PATCH 0/3 v3] GTFIFODBG Daniel Vetter
3 siblings, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-02-09 9:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
This is similar to a patch I wrote several months ago. It's been updated
for the new FORCEWAKE_MT. As recommended by Chris Wilson, use WARN()
instead of DRM_ERROR, so we can get a backtrace.
This shouldn't impact performance too much as the extra register read
can replace the POSTING_READ we had previously.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.c | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1658cfd..a8b6a78 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -379,16 +379,27 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
dev_priv->display.force_wake_get(dev_priv);
}
+static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
+{
+ u32 gtfifodbg;
+ gtfifodbg = I915_READ_NOTRACE(GTFIFODBG);
+ if (WARN(gtfifodbg & GT_FIFO_CPU_ERROR_MASK,
+ "MMIO read or write has been dropped %x\n", gtfifodbg))
+ I915_WRITE_NOTRACE(GTFIFODBG, GT_FIFO_CPU_ERROR_MASK);
+}
+
void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
{
I915_WRITE_NOTRACE(FORCEWAKE, 0);
- POSTING_READ(FORCEWAKE);
+ /* The below doubles as a POSTING_READ */
+ gen6_gt_check_fifodbg(dev_priv);
}
void __gen6_gt_force_wake_mt_put(struct drm_i915_private *dev_priv)
{
I915_WRITE_NOTRACE(FORCEWAKE_MT, (1<<16) | 0);
- POSTING_READ(FORCEWAKE_MT);
+ /* The below doubles as a POSTING_READ */
+ gen6_gt_check_fifodbg(dev_priv);
}
/*
--
1.7.9
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] drm/i915: check gtfifodbg after possibly failed writes
2012-02-09 9:15 [PATCH 0/3 v3] GTFIFODBG Ben Widawsky
2012-02-09 9:15 ` [PATCH 1/3] drm/i915: use gtfifodbg Ben Widawsky
2012-02-09 9:15 ` [PATCH 2/3] drm/i915: catch gtfifo errors on forcewake_put Ben Widawsky
@ 2012-02-09 9:15 ` Ben Widawsky
2012-02-11 16:22 ` [PATCH 0/3 v3] GTFIFODBG Daniel Vetter
3 siblings, 0 replies; 7+ messages in thread
From: Ben Widawsky @ 2012-02-09 9:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Ben Widawsky
If we don't have a sufficient number of free entries in the FIFO, we
proceed to do a write anyway. With this check we should have a clue if
that write actually failed or not.
After some discussion with Daniel Vetter regarding his original
complaint, we agreed upon this.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_drv.c | 15 ++++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a8b6a78..8276a0d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -413,8 +413,10 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
dev_priv->display.force_wake_put(dev_priv);
}
-void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
+int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
{
+ int ret = 0;
+
if (dev_priv->gt_fifo_count < GT_FIFO_NUM_RESERVED_ENTRIES) {
int loop = 500;
u32 fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
@@ -422,10 +424,13 @@ void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv)
udelay(10);
fifo = I915_READ_NOTRACE(GT_FIFO_FREE_ENTRIES);
}
- WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES);
+ if (WARN_ON(loop < 0 && fifo <= GT_FIFO_NUM_RESERVED_ENTRIES))
+ ++ret;
dev_priv->gt_fifo_count = fifo;
}
dev_priv->gt_fifo_count--;
+
+ return ret;
}
static int i915_drm_freeze(struct drm_device *dev)
@@ -960,11 +965,15 @@ __i915_read(64, q)
#define __i915_write(x, y) \
void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
+ u32 __fifo_ret = 0; \
trace_i915_reg_rw(true, reg, val, sizeof(val)); \
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
- __gen6_gt_wait_for_fifo(dev_priv); \
+ __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
write##y(val, dev_priv->regs + reg); \
+ if (unlikely(__fifo_ret)) { \
+ gen6_gt_check_fifodbg(dev_priv); \
+ } \
}
__i915_write(8, b)
__i915_write(16, w)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f02a5f5..ad6408f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1350,7 +1350,7 @@ extern void intel_display_print_error_state(struct seq_file *m,
*/
void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv);
void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv);
-void __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
+int __gen6_gt_wait_for_fifo(struct drm_i915_private *dev_priv);
/* We give fast paths for the really cool registers */
#define NEEDS_FORCE_WAKE(dev_priv, reg) \
--
1.7.9
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/3 v3] GTFIFODBG
2012-02-09 9:15 [PATCH 0/3 v3] GTFIFODBG Ben Widawsky
` (2 preceding siblings ...)
2012-02-09 9:15 ` [PATCH 3/3] drm/i915: check gtfifodbg after possibly failed writes Ben Widawsky
@ 2012-02-11 16:22 ` Daniel Vetter
2012-02-11 20:00 ` Chris Wilson
3 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2012-02-11 16:22 UTC (permalink / raw)
To: Ben Widawsky; +Cc: intel-gfx
On Thu, Feb 09, 2012 at 10:15:17AM +0100, Ben Widawsky wrote:
> Someday I will get simple patches in one version... Alas that day is not
> today.
>
> Ben Widawsky (3):
> drm/i915: use gtfifodbg
> drm/i915: catch gtfifo errors on forcewake_put
> drm/i915: check gtfifodbg after possibly failed writes
I've slurped these three in for -next, thanks for the patches. There's a
quibble though from your grumpy maintainer: Please keep a proper patch
revision log in the commit message and don't just drop it randomly. In
this case the r-b (v1) is pretty pointless because you don't say what
you've changed (and that these patches aren't v1) ...
Cheers, Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/3 v3] GTFIFODBG
2012-02-11 16:22 ` [PATCH 0/3 v3] GTFIFODBG Daniel Vetter
@ 2012-02-11 20:00 ` Chris Wilson
2012-02-11 23:22 ` Daniel Vetter
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2012-02-11 20:00 UTC (permalink / raw)
To: Daniel Vetter, Ben Widawsky; +Cc: intel-gfx
On Sat, 11 Feb 2012 17:22:05 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Feb 09, 2012 at 10:15:17AM +0100, Ben Widawsky wrote:
> > Someday I will get simple patches in one version... Alas that day is not
> > today.
> >
> > Ben Widawsky (3):
> > drm/i915: use gtfifodbg
> > drm/i915: catch gtfifo errors on forcewake_put
> > drm/i915: check gtfifodbg after possibly failed writes
>
> I've slurped these three in for -next, thanks for the patches. There's a
> quibble though from your grumpy maintainer: Please keep a proper patch
> revision log in the commit message and don't just drop it randomly. In
> this case the r-b (v1) is pretty pointless because you don't say what
> you've changed (and that these patches aren't v1) ...
Daniel, can you add my reviewed-by to that series?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3 v3] GTFIFODBG
2012-02-11 20:00 ` Chris Wilson
@ 2012-02-11 23:22 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2012-02-11 23:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: Ben Widawsky, intel-gfx
On Sat, Feb 11, 2012 at 08:00:39PM +0000, Chris Wilson wrote:
> On Sat, 11 Feb 2012 17:22:05 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Feb 09, 2012 at 10:15:17AM +0100, Ben Widawsky wrote:
> > > Someday I will get simple patches in one version... Alas that day is not
> > > today.
> > >
> > > Ben Widawsky (3):
> > > drm/i915: use gtfifodbg
> > > drm/i915: catch gtfifo errors on forcewake_put
> > > drm/i915: check gtfifodbg after possibly failed writes
> >
> > I've slurped these three in for -next, thanks for the patches. There's a
> > quibble though from your grumpy maintainer: Please keep a proper patch
> > revision log in the commit message and don't just drop it randomly. In
> > this case the r-b (v1) is pretty pointless because you don't say what
> > you've changed (and that these patches aren't v1) ...
>
> Daniel, can you add my reviewed-by to that series?
History fixed up, thanks for the reivew.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 7+ messages in thread