[Intel-gfx] [PATCH 1/2] drm/i915: ring irq cleanups

2012-03-29 Thread Ben Widawsky
- gen6 put/get only need one argument
rflags and gflags are always the same (see above explanation)
- remove a couple redundantly defined IRQs
- reordered some lines to make things go in descending order

Every ring has its own interrupts, enables, masks, and status bits that
are fed into the main interrupt enable/mask/status registers. At one
point in time it seemed like a good idea to make our functions support
the notion that each interrupt may have a different bit position in the
corresponding register (blitter parser error may be bit n in IMR, but
bit m in blitter IMR). It turned out though that the HW designers did us
a solid on Gen6+ and this unfortunate situation has been avoided. This
allows our interrupt code to be cleaned up a bit.

I jammed this into one commit because there should be no functional
change with this commit, and staging it into multiple commits was
unnecessarily artificial IMO.

CC: Chris Wilson 
CC: Jesse Barnes 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_irq.c |   16 +++---
 drivers/gpu/drm/i915/i915_reg.h |6 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |   36 +++
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 998116e..c550901 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -503,9 +503,9 @@ static irqreturn_t ivybridge_irq_handler(DRM_IRQ_ARGS)
 
if (gt_iir & (GT_USER_INTERRUPT | GT_PIPE_NOTIFY))
notify_ring(dev, &dev_priv->ring[RCS]);
-   if (gt_iir & GT_GEN6_BSD_USER_INTERRUPT)
+   if (gt_iir & GEN6_BSD_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[VCS]);
-   if (gt_iir & GT_BLT_USER_INTERRUPT)
+   if (gt_iir & GEN6_BLITTER_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[BCS]);
 
if (de_iir & DE_GSE_IVB)
@@ -571,7 +571,7 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS)
atomic_inc(&dev_priv->irq_received);
 
if (IS_GEN6(dev))
-   bsd_usr_interrupt = GT_GEN6_BSD_USER_INTERRUPT;
+   bsd_usr_interrupt = GEN6_BSD_USER_INTERRUPT;
 
/* disable master interrupt before clearing iir  */
de_ier = I915_READ(DEIER);
@@ -605,7 +605,7 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS)
notify_ring(dev, &dev_priv->ring[RCS]);
if (gt_iir & bsd_usr_interrupt)
notify_ring(dev, &dev_priv->ring[VCS]);
-   if (gt_iir & GT_BLT_USER_INTERRUPT)
+   if (gt_iir & GEN6_BLITTER_USER_INTERRUPT)
notify_ring(dev, &dev_priv->ring[BCS]);
 
if (de_iir & DE_GSE)
@@ -1869,8 +1869,8 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
if (IS_GEN6(dev))
render_irqs =
GT_USER_INTERRUPT |
-   GT_GEN6_BSD_USER_INTERRUPT |
-   GT_BLT_USER_INTERRUPT;
+   GEN6_BSD_USER_INTERRUPT |
+   GEN6_BLITTER_USER_INTERRUPT;
else
render_irqs =
GT_USER_INTERRUPT |
@@ -1942,8 +1942,8 @@ static int ivybridge_irq_postinstall(struct drm_device 
*dev)
I915_WRITE(GTIIR, I915_READ(GTIIR));
I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 
-   render_irqs = GT_USER_INTERRUPT | GT_GEN6_BSD_USER_INTERRUPT |
-   GT_BLT_USER_INTERRUPT;
+   render_irqs = GT_USER_INTERRUPT | GEN6_BSD_USER_INTERRUPT |
+   GEN6_BLITTER_USER_INTERRUPT;
I915_WRITE(GTIER, render_irqs);
POSTING_READ(GTIER);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 52a06be..e906885 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -589,7 +589,7 @@
 #define   GEN6_RENDER_COMMAND_PARSER_MASTER_ERROR  (1 << 3)
 #define   GEN6_RENDER_SYNC_STATUS  (1 << 2)
 #define   GEN6_RENDER_DEBUG_INTERRUPT  (1 << 1)
-#define   GEN6_RENDER_USER_INTERRUPT   (1 << 0)
+#define   GEN6_RENDER_USER_INTERRUPT   (1 << 0) /* Alias for 
GT_USER_INTERRUPT */
 
 #define GEN6_BLITTER_HWSTAM0x22098
 #define GEN6_BLITTER_IMR   0x220a8
@@ -3069,12 +3069,10 @@
 #define DEIER   0x4400c
 
 /* GT interrupt */
+#define GT_BSD_USER_INTERRUPT   (1 << 5)
 #define GT_PIPE_NOTIFY (1 << 4)
 #define GT_SYNC_STATUS  (1 << 2)
 #define GT_USER_INTERRUPT   (1 << 0)
-#define GT_BSD_USER_INTERRUPT   (1 << 5)
-#define GT_GEN6_BSD_USER_INTERRUPT (1 << 12)
-#define GT_BLT_USER_INTERRUPT  (1 << 22)
 
 #define GTISR   0x44010
 #define GTIMR   0x44014
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b7b50a5..25b9b9c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -788,7 +788,7 @@ ring_add_r

Re: [Intel-gfx] [PATCH 1/2] drm/i915: ring irq cleanups

2012-03-30 Thread Chris Wilson
On Thu, 29 Mar 2012 19:11:26 -0700, Ben Widawsky  wrote:
> - gen6 put/get only need one argument
> rflags and gflags are always the same (see above explanation)
> - remove a couple redundantly defined IRQs
> - reordered some lines to make things go in descending order
> 
> Every ring has its own interrupts, enables, masks, and status bits that
> are fed into the main interrupt enable/mask/status registers. At one
> point in time it seemed like a good idea to make our functions support
> the notion that each interrupt may have a different bit position in the
> corresponding register (blitter parser error may be bit n in IMR, but
> bit m in blitter IMR). It turned out though that the HW designers did us
> a solid on Gen6+ and this unfortunate situation has been avoided. This
> allows our interrupt code to be cleaned up a bit.
> 
> I jammed this into one commit because there should be no functional
> change with this commit, and staging it into multiple commits was
> unnecessarily artificial IMO.
> 
> CC: Chris Wilson 
> CC: Jesse Barnes 
> Signed-off-by: Ben Widawsky 

Those two patches are
Reviewed-by: Chris Wilson 

We can go further in simplifying get/put irq. First requires a
dev_priv->gt.enable_irq() and then refactor the common (pro|epi)logue.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 1/2] drm/i915: ring irq cleanups

2012-03-30 Thread Daniel Vetter
On Fri, Mar 30, 2012 at 09:33:22AM +0100, Chris Wilson wrote:
> On Thu, 29 Mar 2012 19:11:26 -0700, Ben Widawsky  wrote:
> > - gen6 put/get only need one argument
> > rflags and gflags are always the same (see above explanation)
> > - remove a couple redundantly defined IRQs
> > - reordered some lines to make things go in descending order
> > 
> > Every ring has its own interrupts, enables, masks, and status bits that
> > are fed into the main interrupt enable/mask/status registers. At one
> > point in time it seemed like a good idea to make our functions support
> > the notion that each interrupt may have a different bit position in the
> > corresponding register (blitter parser error may be bit n in IMR, but
> > bit m in blitter IMR). It turned out though that the HW designers did us
> > a solid on Gen6+ and this unfortunate situation has been avoided. This
> > allows our interrupt code to be cleaned up a bit.
> > 
> > I jammed this into one commit because there should be no functional
> > change with this commit, and staging it into multiple commits was
> > unnecessarily artificial IMO.
> > 
> > CC: Chris Wilson 
> > CC: Jesse Barnes 
> > Signed-off-by: Ben Widawsky 
> 
> Those two patches are
> Reviewed-by: Chris Wilson 

Both patches applied, with a few things added to i915_reg.h for the first
one - I got confused about this stuff too much. Thanks for wrestling that
red dragon.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx