Re: [PATCH] drm/i915/ringbuffer: set force wake bit before reading ring register

* Re: [PATCH] drm/i915/ringbuffer: set force wake bit before reading ring register
  2010-11-09  9:17 [PATCH] drm/i915/ringbuffer: set force wake bit before reading ring register Zou Nan hai
@ 2010-11-09  9:17 ` Zou, Nanhai
  2010-11-09 10:50   ` Chris Wilson
  0 siblings, 1 reply; 7+ messages in thread
From: Zou, Nanhai @ 2010-11-09  9:17 UTC (permalink / raw)
  To: intel-gfx, Chris Wilson, Zhao, Jian J

>>-----Original Message-----
>>From: Zou, Nanhai
>>Sent: 2010年11月9日 17:18
>>To: intel-gfx@lists.freedesktop.org; Chris Wilson
>>Cc: Zou, Nanhai
>>Subject: [PATCH] drm/i915/ringbuffer: set force wake bit before reading ring
>>register
>>
>>before reading ring register, set force wake bit to prevent GT core
>>power down to low power state. otherwise we may read stale value.
>>
>>Signed-off-by: Zou Nan hai <nanhai.zou@intel.com>
>>---
>> drivers/gpu/drm/i915/i915_drv.h         |   14 ++++++++++++++
>> drivers/gpu/drm/i915/i915_reg.h         |    1 +
>> drivers/gpu/drm/i915/intel_ringbuffer.c |    3 ---
>> drivers/gpu/drm/i915/intel_ringbuffer.h |   11 +++++++----
>> 4 files changed, 22 insertions(+), 7 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>b/drivers/gpu/drm/i915/i915_drv.h
>>index 90414ae..53c0239 100644
>>--- a/drivers/gpu/drm/i915/i915_drv.h
>>+++ b/drivers/gpu/drm/i915/i915_drv.h
>>@@ -1325,4 +1325,18 @@ static inline void i915_write(struct drm_i915_private
>>*dev_priv, u32 reg,
>>
>> #define PRIMARY_RINGBUFFER_SIZE         (128*1024)
>>
>>+/* on SNB platform,
>>+   before reading ring registers forcewake bit
>>+   must be set to prevent GT core from power down
>>+*/
>>+
>>+static inline u32 i915_safe_read(struct intel_ring_buffer *ring,
>>+		unsigned int offset)
>>+{
>>+	u32 ret;
>>+	drm_i915_private_t *dev_priv = ring->dev->dev_private;
>>+	if (IS_GEN6(ring->dev))	 I915_WRITE(FORCEWAKE, 1);
>>+	ret = I915_READ(offset);
>>+	return ret;
>>+}
>> #endif
>>diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>b/drivers/gpu/drm/i915/i915_reg.h
>>index 25ed911..4d994d2 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -3052,4 +3052,5 @@
>> #define  EDP_LINK_TRAIN_800MV_0DB_SNB_B		(0x38<<22)
>> #define  EDP_LINK_TRAIN_VOL_EMP_MASK_SNB	(0x3f<<22)
>>
>>+#define  FORCEWAKE				0xA18C
>> #endif /* _I915_REG_H_ */
>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>index 7c1f3ff..2820235 100644
>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>@@ -572,7 +572,6 @@ err:
>> int intel_init_ring_buffer(struct drm_device *dev,
>> 			   struct intel_ring_buffer *ring)
>> {
>>-	struct drm_i915_private *dev_priv = dev->dev_private;
>> 	struct drm_i915_gem_object *obj_priv;
>> 	struct drm_gem_object *obj;
>> 	int ret;
>>@@ -691,8 +690,6 @@ int intel_wait_ring_buffer(struct drm_device *dev,
>> 			   struct intel_ring_buffer *ring, int n)
>> {
>> 	unsigned long end;
>>-	drm_i915_private_t *dev_priv = dev->dev_private;
>>-
>> 	trace_i915_ring_wait_begin (dev);
>> 	end = jiffies + 3 * HZ;
>> 	do {
>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
>>b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>index 3126c26..cde1cdd 100644
>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>>@@ -7,13 +7,16 @@ struct  intel_hw_status_page {
>> 	struct		drm_gem_object *obj;
>> };
>>
>>-#define I915_READ_TAIL(ring) I915_READ(RING_TAIL(ring->mmio_base))
>>+#define I915_READ_TAIL(ring) i915_safe_read(ring,
>>RING_TAIL(ring->mmio_base))
>> #define I915_WRITE_TAIL(ring, val) I915_WRITE(RING_TAIL(ring->mmio_base),
>>val)
>>-#define I915_READ_START(ring) I915_READ(RING_START(ring->mmio_base))
>>+
>>+#define I915_READ_START(ring) i915_safe_read(ring,
>>RING_START(ring->mmio_base))
>> #define I915_WRITE_START(ring, val) I915_WRITE(RING_START(ring->mmio_base),
>>val)
>>-#define I915_READ_HEAD(ring) I915_READ(RING_HEAD(ring->mmio_base))
>>+
>>+#define I915_READ_HEAD(ring)  i915_safe_read(ring,
>>RING_HEAD(ring->mmio_base))
>> #define I915_WRITE_HEAD(ring, val) I915_WRITE(RING_HEAD(ring->mmio_base),
>>val)
>>-#define I915_READ_CTL(ring) I915_READ(RING_CTL(ring->mmio_base))
>>+
>>+#define I915_READ_CTL(ring) i915_safe_read(ring,
>>RING_CTL(ring->mmio_base))
>> #define I915_WRITE_CTL(ring, val) I915_WRITE(RING_CTL(ring->mmio_base),
>>val)
>>
>> struct drm_i915_gem_execbuffer2;
>>--
>>1.7.1

I have tested this patch with the read ring head from status page workaround patch reverted.
Seems it works on my SNB box.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread