* [regression] drm/i915/ringbuffer: Simplify the ring irq refcounting
@ 2011-01-05 10:02 Alexey Fisher
2011-01-05 10:36 ` [PATCH] drm/i915: Make the ring IMR handling private Chris Wilson
0 siblings, 1 reply; 3+ messages in thread
From: Alexey Fisher @ 2011-01-05 10:02 UTC (permalink / raw)
To: intel-gfx
Hi,
i have regression on board Intel DG45ID, graphic:
Vendor: 0x8086, Device: 0x2e22, Revision: 0x03 (A3)
Symptom:
PC freeze complete (no ssh access), after GDM login. Some times just
after login, some time i can start firefox and browse some minutes.
xserver-xorg-video-intel:
2:2.13.903+git20110104.a7c7a910-0ubuntu0sarvatt~maverick
xserver-xorg:
2:2.13.903+git20110104.a7c7a910-0ubuntu0sarvatt~maverick
commit 74fe9bb76464289b2f994bcd188d906f30912326
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Tue Jan 4 22:22:56 2011 +0000
drm/i915/ringbuffer: Simplify the ring irq refcounting
... and move it under the spinlock to gain the appropriate memory
barriers.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32752
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
--
Regards,
Alexey
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] drm/i915: Make the ring IMR handling private
2011-01-05 10:02 [regression] drm/i915/ringbuffer: Simplify the ring irq refcounting Alexey Fisher
@ 2011-01-05 10:36 ` Chris Wilson
2011-01-05 11:54 ` Alexey Fisher
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2011-01-05 10:36 UTC (permalink / raw)
To: Alexey Fisher; +Cc: intel-gfx
As the IMR for the USER interrupts are not modified elsewhere, we can
separate the spinlock used for these from that of hpd and pipestats.
Those two IMR are manipulated under an IRQ and so need heavier locking.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 26 ++++++++++++++------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 13cad98..03e3370 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -526,7 +526,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring)
if (!dev->irq_enabled)
return false;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (ring->irq_refcount++ == 0) {
if (HAS_PCH_SPLIT(dev))
ironlake_enable_irq(dev_priv,
@@ -534,7 +534,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring)
else
i915_enable_irq(dev_priv, I915_USER_INTERRUPT);
}
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
return true;
}
@@ -545,7 +545,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring)
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (--ring->irq_refcount == 0) {
if (HAS_PCH_SPLIT(dev))
ironlake_disable_irq(dev_priv,
@@ -554,7 +554,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring)
else
i915_disable_irq(dev_priv, I915_USER_INTERRUPT);
}
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
}
void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
@@ -620,10 +620,10 @@ ring_get_irq(struct intel_ring_buffer *ring, u32 flag)
if (!dev->irq_enabled)
return false;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (ring->irq_refcount++ == 0)
ironlake_enable_irq(dev_priv, flag);
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
return true;
}
@@ -634,10 +634,10 @@ ring_put_irq(struct intel_ring_buffer *ring, u32 flag)
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (--ring->irq_refcount == 0)
ironlake_disable_irq(dev_priv, flag);
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
}
static bool
@@ -649,13 +649,13 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
if (!dev->irq_enabled)
return false;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (ring->irq_refcount++ == 0) {
ring->irq_mask &= ~rflag;
I915_WRITE_IMR(ring, ring->irq_mask);
ironlake_enable_irq(dev_priv, gflag);
}
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
return true;
}
@@ -666,13 +666,13 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
struct drm_device *dev = ring->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- spin_lock(&dev_priv->irq_lock);
+ spin_lock(&ring->irq_lock);
if (--ring->irq_refcount == 0) {
ring->irq_mask |= rflag;
I915_WRITE_IMR(ring, ring->irq_mask);
ironlake_disable_irq(dev_priv, gflag);
}
- spin_unlock(&dev_priv->irq_lock);
+ spin_unlock(&ring->irq_lock);
}
static bool
@@ -814,6 +814,8 @@ int intel_init_ring_buffer(struct drm_device *dev,
INIT_LIST_HEAD(&ring->active_list);
INIT_LIST_HEAD(&ring->request_list);
INIT_LIST_HEAD(&ring->gpu_write_list);
+
+ spin_lock_init(&ring->irq_lock);
ring->irq_mask = ~0;
if (I915_NEED_GFX_HWS(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6b1d9a5..be9087e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -55,6 +55,7 @@ struct intel_ring_buffer {
int effective_size;
struct intel_hw_status_page status_page;
+ spinlock_t irq_lock;
u32 irq_refcount;
u32 irq_mask;
u32 irq_seqno; /* last seq seem at irq time */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Make the ring IMR handling private
2011-01-05 10:36 ` [PATCH] drm/i915: Make the ring IMR handling private Chris Wilson
@ 2011-01-05 11:54 ` Alexey Fisher
0 siblings, 0 replies; 3+ messages in thread
From: Alexey Fisher @ 2011-01-05 11:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
Tested. It's working.
Thank you.
Am Mittwoch, den 05.01.2011, 10:36 +0000 schrieb Chris Wilson:
> As the IMR for the USER interrupts are not modified elsewhere, we can
> separate the spinlock used for these from that of hpd and pipestats.
> Those two IMR are manipulated under an IRQ and so need heavier locking.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 26 ++++++++++++++------------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> 2 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 13cad98..03e3370 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -526,7 +526,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring)
> if (!dev->irq_enabled)
> return false;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (ring->irq_refcount++ == 0) {
> if (HAS_PCH_SPLIT(dev))
> ironlake_enable_irq(dev_priv,
> @@ -534,7 +534,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring)
> else
> i915_enable_irq(dev_priv, I915_USER_INTERRUPT);
> }
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
>
> return true;
> }
> @@ -545,7 +545,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring)
> struct drm_device *dev = ring->dev;
> drm_i915_private_t *dev_priv = dev->dev_private;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (--ring->irq_refcount == 0) {
> if (HAS_PCH_SPLIT(dev))
> ironlake_disable_irq(dev_priv,
> @@ -554,7 +554,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring)
> else
> i915_disable_irq(dev_priv, I915_USER_INTERRUPT);
> }
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
> }
>
> void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
> @@ -620,10 +620,10 @@ ring_get_irq(struct intel_ring_buffer *ring, u32 flag)
> if (!dev->irq_enabled)
> return false;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (ring->irq_refcount++ == 0)
> ironlake_enable_irq(dev_priv, flag);
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
>
> return true;
> }
> @@ -634,10 +634,10 @@ ring_put_irq(struct intel_ring_buffer *ring, u32 flag)
> struct drm_device *dev = ring->dev;
> drm_i915_private_t *dev_priv = dev->dev_private;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (--ring->irq_refcount == 0)
> ironlake_disable_irq(dev_priv, flag);
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
> }
>
> static bool
> @@ -649,13 +649,13 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
> if (!dev->irq_enabled)
> return false;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (ring->irq_refcount++ == 0) {
> ring->irq_mask &= ~rflag;
> I915_WRITE_IMR(ring, ring->irq_mask);
> ironlake_enable_irq(dev_priv, gflag);
> }
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
>
> return true;
> }
> @@ -666,13 +666,13 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag)
> struct drm_device *dev = ring->dev;
> drm_i915_private_t *dev_priv = dev->dev_private;
>
> - spin_lock(&dev_priv->irq_lock);
> + spin_lock(&ring->irq_lock);
> if (--ring->irq_refcount == 0) {
> ring->irq_mask |= rflag;
> I915_WRITE_IMR(ring, ring->irq_mask);
> ironlake_disable_irq(dev_priv, gflag);
> }
> - spin_unlock(&dev_priv->irq_lock);
> + spin_unlock(&ring->irq_lock);
> }
>
> static bool
> @@ -814,6 +814,8 @@ int intel_init_ring_buffer(struct drm_device *dev,
> INIT_LIST_HEAD(&ring->active_list);
> INIT_LIST_HEAD(&ring->request_list);
> INIT_LIST_HEAD(&ring->gpu_write_list);
> +
> + spin_lock_init(&ring->irq_lock);
> ring->irq_mask = ~0;
>
> if (I915_NEED_GFX_HWS(dev)) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 6b1d9a5..be9087e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -55,6 +55,7 @@ struct intel_ring_buffer {
> int effective_size;
> struct intel_hw_status_page status_page;
>
> + spinlock_t irq_lock;
> u32 irq_refcount;
> u32 irq_mask;
> u32 irq_seqno; /* last seq seem at irq time */
--
Regards,
Alexey
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-01-05 11:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-05 10:02 [regression] drm/i915/ringbuffer: Simplify the ring irq refcounting Alexey Fisher
2011-01-05 10:36 ` [PATCH] drm/i915: Make the ring IMR handling private Chris Wilson
2011-01-05 11:54 ` Alexey Fisher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox