| Message ID | 1303890079-2518-1-git-send-email-boqun.feng@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 27 Apr 2011 15:41:18 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote: > Remove ring_put_irq/ring_get_irq:drivers/gpu/drm/i915/intel_ringbuffer.c > , they are only used by bsd_ring_put_irq/bsd_ring_get_irq. > Expand the code in bsd_ring_put_irq/bsd_ring_get_irq. Why is this change useful?
I have discussed this with Chris in my earlier patch. This change is a clean-up, since ring_put_irq and ring_get_irq are only used by bsd_ring_put_irq and bsd_ring_get_irq. And once this change is made, it is more clear to see the difference between g4x and ironlake BSD interrupt control interface, because they are handled in a single function and they are different at the interrupt mask reg addresss as well as the interrupt flag . Thanks Feng, Boqun -----Original Message----- From: Keith Packard [mailto:keithp@keithp.com] Sent: Thursday, April 28, 2011 12:43 AM To: Feng, Boqun; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915:merge ring_put/get_irq into bsd_ring_put/get_irq On Wed, 27 Apr 2011 15:41:18 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote: > Remove ring_put_irq/ring_get_irq:drivers/gpu/drm/i915/intel_ringbuffer.c > , they are only used by bsd_ring_put_irq/bsd_ring_get_irq. > Expand the code in bsd_ring_put_irq/bsd_ring_get_irq. Why is this change useful?
On Thu, 28 Apr 2011 10:06:51 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote: > I have discussed this with Chris in my earlier patch. > > This change is a clean-up, since ring_put_irq and ring_get_irq are only used by > bsd_ring_put_irq and bsd_ring_get_irq. > > And once this change is made, it is more clear to see the difference between > g4x and ironlake BSD interrupt control interface, because they are handled in > a single function and they are different at the interrupt mask reg addresss as > well as the interrupt flag please put important details like that in the commit message; having some way to evaluate the utility of the patch is very important for something which doesn't actually change how the code works.
I got it! Thanks for reminding me. :) -----Original Message----- From: Keith Packard [mailto:keithp@keithp.com] Sent: Thursday, April 28, 2011 10:41 AM To: Feng, Boqun; intel-gfx@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH 1/2] drm/i915:merge ring_put/get_irq into bsd_ring_put/get_irq On Thu, 28 Apr 2011 10:06:51 +0800, "Feng, Boqun" <boqun.feng@intel.com> wrote: > I have discussed this with Chris in my earlier patch. > > This change is a clean-up, since ring_put_irq and ring_get_irq are > only used by bsd_ring_put_irq and bsd_ring_get_irq. > > And once this change is made, it is more clear to see the difference > between g4x and ironlake BSD interrupt control interface, because they > are handled in a single function and they are different at the > interrupt mask reg addresss as well as the interrupt flag please put important details like that in the commit message; having some way to evaluate the utility of the patch is very important for something which doesn't actually change how the code works. -- keith.packard@intel.com
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e9e6f71..06c921f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -600,7 +600,7 @@ ring_add_request(struct intel_ring_buffer *ring, } static bool -ring_get_irq(struct intel_ring_buffer *ring, u32 flag) +gen6_ring_get_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; @@ -609,27 +609,33 @@ ring_get_irq(struct intel_ring_buffer *ring, u32 flag) return false; spin_lock(&ring->irq_lock); - if (ring->irq_refcount++ == 0) - ironlake_enable_irq(dev_priv, flag); + if (ring->irq_refcount++ == 0) { + ring->irq_mask &= ~rflag; + I915_WRITE_IMR(ring, ring->irq_mask); + ironlake_enable_irq(dev_priv, gflag); + } spin_unlock(&ring->irq_lock); return true; } static void -ring_put_irq(struct intel_ring_buffer *ring, u32 flag) +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(&ring->irq_lock); - if (--ring->irq_refcount == 0) - ironlake_disable_irq(dev_priv, flag); + if (--ring->irq_refcount == 0) { + ring->irq_mask |= rflag; + I915_WRITE_IMR(ring, ring->irq_mask); + ironlake_disable_irq(dev_priv, gflag); + } spin_unlock(&ring->irq_lock); } static bool -gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) +bsd_ring_get_irq(struct intel_ring_buffer *ring) { struct drm_device *dev = ring->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -638,42 +644,24 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) return false; 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); - } + if (ring->irq_refcount++ == 0) + ironlake_enable_irq(dev_priv, GT_BSD_USER_INTERRUPT); spin_unlock(&ring->irq_lock); return true; } - static void -gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) +bsd_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(&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); - } + if (--ring->irq_refcount == 0) + ironlake_disable_irq(dev_priv, GT_BSD_USER_INTERRUPT); spin_unlock(&ring->irq_lock); } -static bool -bsd_ring_get_irq(struct intel_ring_buffer *ring) -{ - return ring_get_irq(ring, GT_BSD_USER_INTERRUPT); -} -static void -bsd_ring_put_irq(struct intel_ring_buffer *ring) -{ - ring_put_irq(ring, GT_BSD_USER_INTERRUPT); -} - static int ring_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length) {