[PATCH] [RFC] drm/i915: Warning when reads or writes are dropped

[PATCH] [RFC] drm/i915: Warning when reads or writes are dropped

[Ben Widawsky]

The GTFIFODBG register gives us 3 error types when the fifo is accessed
and full. Whenever we do a forcewake_put we can check this register to
see if any of the CPU related errors occurred.

Of more interest is perhaps the bit I am not checking which tells when
some other part of the chip makes a request and the FIFO is full. I
couldn't really decide on a good place to put that check.

This patch seems to have value to me, but I'm not sure it's worth the
cost of the extra MMIO read`. (I've yet to see this occur, but I haven't
actually been running with it for very long).

---
 drivers/gpu/drm/i915/i915_drv.c |    6 ++++++
 drivers/gpu/drm/i915/i915_reg.h |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 548e04b..e7f2a27 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -360,6 +360,12 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
+	u32 gtfifodbg = I915_READ(GTFIFODBG);
+	if (gtfifodbg & GT_FIFO_CPU_ERROR_MASK) {
+		DRM_ERROR("MMIO read or write has been dropped %x\n",
+			  gtfifodbg);
+		I915_WRITE(GTFIFODBG, gtfifodbg & GT_FIFO_CPU_ERROR_MASK);
+	}
 	I915_WRITE_NOTRACE(FORCEWAKE, 0);
 	POSTING_READ(FORCEWAKE);
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a09416..1408866 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3441,6 +3441,12 @@
 #define  FORCEWAKE				0xA18C
 #define  FORCEWAKE_ACK				0x130090
 
+#define  GTFIFODBG				0x120000
+#define    GT_FIFO_OVFERR			(1<<2)
+#define    GT_FIFO_CPU_ERROR_MASK		3
+#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
 
-- 
1.7.7.2

Re: [PATCH] [RFC] drm/i915: Warning when reads or writes are dropped

[Daniel Vetter]

On Fri, Nov 04, 2011 at 05:03:54PM -0700, Ben Widawsky wrote:
> The GTFIFODBG register gives us 3 error types when the fifo is accessed
> and full. Whenever we do a forcewake_put we can check this register to
> see if any of the CPU related errors occurred.
> 
> Of more interest is perhaps the bit I am not checking which tells when
> some other part of the chip makes a request and the FIFO is full. I
> couldn't really decide on a good place to put that check.
> 
> This patch seems to have value to me, but I'm not sure it's worth the
> cost of the extra MMIO read`. (I've yet to see this occur, but I haven't
> actually been running with it for very long).
> 

This looks like a nice little patch here. Care to update it for
spinlocked and multithreaded forcewake and maybe also check the other
errors? And also add it to the error_state output (just base it on top of
danvet/my-next to avoid conflicts with the oustanding error_state
cleanup).

Thanks, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] [RFC] drm/i915: Warning when reads or writes are dropped

[Chris Wilson]

On Tue, 17 Jan 2012 12:08:57 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 04, 2011 at 05:03:54PM -0700, Ben Widawsky wrote:
> > The GTFIFODBG register gives us 3 error types when the fifo is accessed
> > and full. Whenever we do a forcewake_put we can check this register to
> > see if any of the CPU related errors occurred.
> > 
> > Of more interest is perhaps the bit I am not checking which tells when
> > some other part of the chip makes a request and the FIFO is full. I
> > couldn't really decide on a good place to put that check.
> > 
> > This patch seems to have value to me, but I'm not sure it's worth the
> > cost of the extra MMIO read`. (I've yet to see this occur, but I haven't
> > actually been running with it for very long).
> > 
> 
> This looks like a nice little patch here. Care to update it for
> spinlocked and multithreaded forcewake and maybe also check the other
> errors? And also add it to the error_state output (just base it on top of
> danvet/my-next to avoid conflicts with the oustanding error_state
> cleanup).

The advantage of placing the warning in _put is that we could identify
the sequence of register writes that trigger the error, so a stack trace
would be more useful i.e. a WARN instead.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre