| Message ID | 20190410235344.31199-4-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IRQ initialization debloat and conversion to uncore, v2 | expand |
On Wed, Apr 10, 2019 at 04:53:42PM -0700, Paulo Zanoni wrote: > This discussion started because we use token pasting in the > GEN{2,3}_IRQ_INIT and GEN{2,3}_IRQ_RESET macros, so gen2-4 passes an > empty argument to those macros, making the code a little weird. The > original proposal was to just add a comment as the empty argument, but > Ville suggested we just add a prefix to the registers, and that indeed > sounds like a more elegant solution. > > Now doing this is kinda against our rules for register naming since we > only add gens or platform names as register prefixes when the given > gen/platform changes a register that already existed before. On the > other hand, we have so many instances of IIR/IMR in comments that > adding a prefix would make the users of these register more easily > findable, in addition to make our token pasting macros actually > readable. So IMHO opening an exception here is worth it. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 +-- > drivers/gpu/drm/i915/i915_gpu_error.c | 4 +- > drivers/gpu/drm/i915/i915_irq.c | 52 ++++++++++++------------- > drivers/gpu/drm/i915/i915_reg.h | 8 ++-- > drivers/gpu/drm/i915/i915_reset.c | 3 +- > drivers/gpu/drm/i915/intel_overlay.c | 4 +- > drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++--- > 7 files changed, 44 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 77b3252bdb2e..5823ffb17821 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -833,11 +833,11 @@ static int i915_interrupt_info(struct seq_file *m, void *data) > > } else if (!HAS_PCH_SPLIT(dev_priv)) { > seq_printf(m, "Interrupt enable: %08x\n", > - I915_READ(IER)); > + I915_READ(GEN2_IER)); > seq_printf(m, "Interrupt identity: %08x\n", > - I915_READ(IIR)); > + I915_READ(GEN2_IIR)); > seq_printf(m, "Interrupt mask: %08x\n", > - I915_READ(IMR)); > + I915_READ(GEN2_IMR)); > for_each_pipe(dev_priv, pipe) > seq_printf(m, "Pipe %c stat: %08x\n", > pipe_name(pipe), > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index 43b68fdc8967..f51ff683dd2e 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1635,9 +1635,9 @@ static void capture_reg_state(struct i915_gpu_state *error) > error->gtier[0] = I915_READ(GTIER); > error->ngtier = 1; > } else if (IS_GEN(dev_priv, 2)) { > - error->ier = I915_READ16(IER); > + error->ier = I915_READ16(GEN2_IER); > } else if (!IS_VALLEYVIEW(dev_priv)) { > - error->ier = I915_READ(IER); > + error->ier = I915_READ(GEN2_IER); > } > error->eir = I915_READ(EIR); > error->pgtbl_er = I915_READ(PGTBL_ER); > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index b1f1db2bd879..2910b06913af 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -153,16 +153,16 @@ static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, > > static void gen2_irq_reset(struct drm_i915_private *dev_priv) > { > - I915_WRITE16(IMR, 0xffff); > - POSTING_READ16(IMR); > + I915_WRITE16(GEN2_IMR, 0xffff); > + POSTING_READ16(GEN2_IMR); > > - I915_WRITE16(IER, 0); > + I915_WRITE16(GEN2_IER, 0); > > /* IIR can theoretically queue up two events. Be paranoid. */ > - I915_WRITE16(IIR, 0xffff); > - POSTING_READ16(IIR); > - I915_WRITE16(IIR, 0xffff); > - POSTING_READ16(IIR); > + I915_WRITE16(GEN2_IIR, 0xffff); > + POSTING_READ16(GEN2_IIR); > + I915_WRITE16(GEN2_IIR, 0xffff); > + POSTING_READ16(GEN2_IIR); > } > > #define GEN8_IRQ_RESET_NDX(type, which) \ > @@ -199,17 +199,17 @@ static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv, > > static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv) > { > - u16 val = I915_READ16(IIR); > + u16 val = I915_READ16(GEN2_IIR); > > if (val == 0) > return; > > WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n", > - i915_mmio_reg_offset(IIR), val); > - I915_WRITE16(IIR, 0xffff); > - POSTING_READ16(IIR); > - I915_WRITE16(IIR, 0xffff); > - POSTING_READ16(IIR); > + i915_mmio_reg_offset(GEN2_IIR), val); > + I915_WRITE16(GEN2_IIR, 0xffff); > + POSTING_READ16(GEN2_IIR); > + I915_WRITE16(GEN2_IIR, 0xffff); > + POSTING_READ16(GEN2_IIR); > } > > static void gen3_irq_init(struct drm_i915_private *dev_priv, > @@ -229,9 +229,9 @@ static void gen2_irq_init(struct drm_i915_private *dev_priv, > { > gen2_assert_iir_is_zero(dev_priv); > > - I915_WRITE16(IER, ier_val); > - I915_WRITE16(IMR, imr_val); > - POSTING_READ16(IMR); > + I915_WRITE16(GEN2_IER, ier_val); > + I915_WRITE16(GEN2_IMR, imr_val); > + POSTING_READ16(GEN2_IMR); > } > > #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > @@ -4344,7 +4344,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) > u16 eir = 0, eir_stuck = 0; > u16 iir; > > - iir = I915_READ16(IIR); > + iir = I915_READ16(GEN2_IIR); > if (iir == 0) > break; > > @@ -4357,7 +4357,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) > if (iir & I915_MASTER_ERROR_INTERRUPT) > i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck); > > - I915_WRITE16(IIR, iir); > + I915_WRITE16(GEN2_IIR, iir); > > if (iir & I915_USER_INTERRUPT) > intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); > @@ -4384,7 +4384,7 @@ static void i915_irq_reset(struct drm_device *dev) > > i9xx_pipestat_irq_reset(dev_priv); > > - GEN3_IRQ_RESET(); > + GEN3_IRQ_RESET(GEN2_); These do look a bit odd with the gen3+gen2 in the same sentence. Hence not entitely convinced that GEN2_ is the best prefix. But meh. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > static int i915_irq_postinstall(struct drm_device *dev) > @@ -4416,7 +4416,7 @@ static int i915_irq_postinstall(struct drm_device *dev) > dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT; > } > > - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask); > + GEN3_IRQ_INIT(GEN2_, dev_priv->irq_mask, enable_mask); > > /* Interrupt setup is already guaranteed to be single-threaded, this is > * just to make the assert_spin_locked check happy. */ > @@ -4448,7 +4448,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) > u32 hotplug_status = 0; > u32 iir; > > - iir = I915_READ(IIR); > + iir = I915_READ(GEN2_IIR); > if (iir == 0) > break; > > @@ -4465,7 +4465,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) > if (iir & I915_MASTER_ERROR_INTERRUPT) > i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck); > > - I915_WRITE(IIR, iir); > + I915_WRITE(GEN2_IIR, iir); > > if (iir & I915_USER_INTERRUPT) > intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); > @@ -4493,7 +4493,7 @@ static void i965_irq_reset(struct drm_device *dev) > > i9xx_pipestat_irq_reset(dev_priv); > > - GEN3_IRQ_RESET(); > + GEN3_IRQ_RESET(GEN2_); > } > > static int i965_irq_postinstall(struct drm_device *dev) > @@ -4536,7 +4536,7 @@ static int i965_irq_postinstall(struct drm_device *dev) > if (IS_G4X(dev_priv)) > enable_mask |= I915_BSD_USER_INTERRUPT; > > - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask); > + GEN3_IRQ_INIT(GEN2_, dev_priv->irq_mask, enable_mask); > > /* Interrupt setup is already guaranteed to be single-threaded, this is > * just to make the assert_spin_locked check happy. */ > @@ -4594,7 +4594,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) > u32 hotplug_status = 0; > u32 iir; > > - iir = I915_READ(IIR); > + iir = I915_READ(GEN2_IIR); > if (iir == 0) > break; > > @@ -4610,7 +4610,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) > if (iir & I915_MASTER_ERROR_INTERRUPT) > i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck); > > - I915_WRITE(IIR, iir); > + I915_WRITE(GEN2_IIR, iir); > > if (iir & I915_USER_INTERRUPT) > intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 9c206e803ab3..6a150243cabb 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2713,10 +2713,10 @@ enum i915_power_well_id { > #define VLV_GU_CTL0 _MMIO(VLV_DISPLAY_BASE + 0x2030) > #define VLV_GU_CTL1 _MMIO(VLV_DISPLAY_BASE + 0x2034) > #define SCPD0 _MMIO(0x209c) /* 915+ only */ > -#define IER _MMIO(0x20a0) > -#define IIR _MMIO(0x20a4) > -#define IMR _MMIO(0x20a8) > -#define ISR _MMIO(0x20ac) > +#define GEN2_IER _MMIO(0x20a0) > +#define GEN2_IIR _MMIO(0x20a4) > +#define GEN2_IMR _MMIO(0x20a8) > +#define GEN2_ISR _MMIO(0x20ac) > #define VLV_GUNIT_CLOCK_GATE _MMIO(VLV_DISPLAY_BASE + 0x2060) > #define GINT_DIS (1 << 22) > #define GCFG_DIS (1 << 8) > diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c > index 68875ba43b8d..b75ac660c3c2 100644 > --- a/drivers/gpu/drm/i915/i915_reset.c > +++ b/drivers/gpu/drm/i915/i915_reset.c > @@ -1223,7 +1223,8 @@ void i915_clear_error_registers(struct drm_i915_private *i915) > */ > DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir); > rmw_set(uncore, EMR, eir); > - intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT); > + intel_uncore_write(uncore, GEN2_IIR, > + I915_MASTER_ERROR_INTERRUPT); > } > > if (INTEL_GEN(i915) >= 8) { > diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c > index a882b8d42bd9..eb317759b5d3 100644 > --- a/drivers/gpu/drm/i915/intel_overlay.c > +++ b/drivers/gpu/drm/i915/intel_overlay.c > @@ -446,7 +446,7 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay) > if (!overlay->old_vma) > return 0; > > - if (I915_READ(ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) { > + if (I915_READ(GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) { > /* synchronous slowpath */ > struct i915_request *rq; > > @@ -1430,7 +1430,7 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv) > return NULL; > > error->dovsta = I915_READ(DOVSTA); > - error->isr = I915_READ(ISR); > + error->isr = I915_READ(GEN2_ISR); > error->base = overlay->flip_addr; > > memcpy_fromio(&error->regs, overlay->regs, sizeof(error->regs)); > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index af35f99c5940..029fd8ec1857 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -977,15 +977,15 @@ static void > i9xx_irq_enable(struct intel_engine_cs *engine) > { > engine->i915->irq_mask &= ~engine->irq_enable_mask; > - intel_uncore_write(engine->uncore, IMR, engine->i915->irq_mask); > - intel_uncore_posting_read_fw(engine->uncore, IMR); > + intel_uncore_write(engine->uncore, GEN2_IMR, engine->i915->irq_mask); > + intel_uncore_posting_read_fw(engine->uncore, GEN2_IMR); > } > > static void > i9xx_irq_disable(struct intel_engine_cs *engine) > { > engine->i915->irq_mask |= engine->irq_enable_mask; > - intel_uncore_write(engine->uncore, IMR, engine->i915->irq_mask); > + intel_uncore_write(engine->uncore, GEN2_IMR, engine->i915->irq_mask); > } > > static void > @@ -994,7 +994,7 @@ i8xx_irq_enable(struct intel_engine_cs *engine) > struct drm_i915_private *dev_priv = engine->i915; > > dev_priv->irq_mask &= ~engine->irq_enable_mask; > - I915_WRITE16(IMR, dev_priv->irq_mask); > + I915_WRITE16(GEN2_IMR, dev_priv->irq_mask); > POSTING_READ16(RING_IMR(engine->mmio_base)); > } > > @@ -1004,7 +1004,7 @@ i8xx_irq_disable(struct intel_engine_cs *engine) > struct drm_i915_private *dev_priv = engine->i915; > > dev_priv->irq_mask |= engine->irq_enable_mask; > - I915_WRITE16(IMR, dev_priv->irq_mask); > + I915_WRITE16(GEN2_IMR, dev_priv->irq_mask); > } > > static int > -- > 2.20.1
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 77b3252bdb2e..5823ffb17821 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -833,11 +833,11 @@ static int i915_interrupt_info(struct seq_file *m, void *data) } else if (!HAS_PCH_SPLIT(dev_priv)) { seq_printf(m, "Interrupt enable: %08x\n", - I915_READ(IER)); + I915_READ(GEN2_IER)); seq_printf(m, "Interrupt identity: %08x\n", - I915_READ(IIR)); + I915_READ(GEN2_IIR)); seq_printf(m, "Interrupt mask: %08x\n", - I915_READ(IMR)); + I915_READ(GEN2_IMR)); for_each_pipe(dev_priv, pipe) seq_printf(m, "Pipe %c stat: %08x\n", pipe_name(pipe), diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 43b68fdc8967..f51ff683dd2e 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1635,9 +1635,9 @@ static void capture_reg_state(struct i915_gpu_state *error) error->gtier[0] = I915_READ(GTIER); error->ngtier = 1; } else if (IS_GEN(dev_priv, 2)) { - error->ier = I915_READ16(IER); + error->ier = I915_READ16(GEN2_IER); } else if (!IS_VALLEYVIEW(dev_priv)) { - error->ier = I915_READ(IER); + error->ier = I915_READ(GEN2_IER); } error->eir = I915_READ(EIR); error->pgtbl_er = I915_READ(PGTBL_ER); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index b1f1db2bd879..2910b06913af 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -153,16 +153,16 @@ static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, static void gen2_irq_reset(struct drm_i915_private *dev_priv) { - I915_WRITE16(IMR, 0xffff); - POSTING_READ16(IMR); + I915_WRITE16(GEN2_IMR, 0xffff); + POSTING_READ16(GEN2_IMR); - I915_WRITE16(IER, 0); + I915_WRITE16(GEN2_IER, 0); /* IIR can theoretically queue up two events. Be paranoid. */ - I915_WRITE16(IIR, 0xffff); - POSTING_READ16(IIR); - I915_WRITE16(IIR, 0xffff); - POSTING_READ16(IIR); + I915_WRITE16(GEN2_IIR, 0xffff); + POSTING_READ16(GEN2_IIR); + I915_WRITE16(GEN2_IIR, 0xffff); + POSTING_READ16(GEN2_IIR); } #define GEN8_IRQ_RESET_NDX(type, which) \ @@ -199,17 +199,17 @@ static void gen3_assert_iir_is_zero(struct drm_i915_private *dev_priv, static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv) { - u16 val = I915_READ16(IIR); + u16 val = I915_READ16(GEN2_IIR); if (val == 0) return; WARN(1, "Interrupt register 0x%x is not zero: 0x%08x\n", - i915_mmio_reg_offset(IIR), val); - I915_WRITE16(IIR, 0xffff); - POSTING_READ16(IIR); - I915_WRITE16(IIR, 0xffff); - POSTING_READ16(IIR); + i915_mmio_reg_offset(GEN2_IIR), val); + I915_WRITE16(GEN2_IIR, 0xffff); + POSTING_READ16(GEN2_IIR); + I915_WRITE16(GEN2_IIR, 0xffff); + POSTING_READ16(GEN2_IIR); } static void gen3_irq_init(struct drm_i915_private *dev_priv, @@ -229,9 +229,9 @@ static void gen2_irq_init(struct drm_i915_private *dev_priv, { gen2_assert_iir_is_zero(dev_priv); - I915_WRITE16(IER, ier_val); - I915_WRITE16(IMR, imr_val); - POSTING_READ16(IMR); + I915_WRITE16(GEN2_IER, ier_val); + I915_WRITE16(GEN2_IMR, imr_val); + POSTING_READ16(GEN2_IMR); } #define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ @@ -4344,7 +4344,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) u16 eir = 0, eir_stuck = 0; u16 iir; - iir = I915_READ16(IIR); + iir = I915_READ16(GEN2_IIR); if (iir == 0) break; @@ -4357,7 +4357,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) if (iir & I915_MASTER_ERROR_INTERRUPT) i8xx_error_irq_ack(dev_priv, &eir, &eir_stuck); - I915_WRITE16(IIR, iir); + I915_WRITE16(GEN2_IIR, iir); if (iir & I915_USER_INTERRUPT) intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); @@ -4384,7 +4384,7 @@ static void i915_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - GEN3_IRQ_RESET(); + GEN3_IRQ_RESET(GEN2_); } static int i915_irq_postinstall(struct drm_device *dev) @@ -4416,7 +4416,7 @@ static int i915_irq_postinstall(struct drm_device *dev) dev_priv->irq_mask &= ~I915_DISPLAY_PORT_INTERRUPT; } - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask); + GEN3_IRQ_INIT(GEN2_, dev_priv->irq_mask, enable_mask); /* Interrupt setup is already guaranteed to be single-threaded, this is * just to make the assert_spin_locked check happy. */ @@ -4448,7 +4448,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) u32 hotplug_status = 0; u32 iir; - iir = I915_READ(IIR); + iir = I915_READ(GEN2_IIR); if (iir == 0) break; @@ -4465,7 +4465,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) if (iir & I915_MASTER_ERROR_INTERRUPT) i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck); - I915_WRITE(IIR, iir); + I915_WRITE(GEN2_IIR, iir); if (iir & I915_USER_INTERRUPT) intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); @@ -4493,7 +4493,7 @@ static void i965_irq_reset(struct drm_device *dev) i9xx_pipestat_irq_reset(dev_priv); - GEN3_IRQ_RESET(); + GEN3_IRQ_RESET(GEN2_); } static int i965_irq_postinstall(struct drm_device *dev) @@ -4536,7 +4536,7 @@ static int i965_irq_postinstall(struct drm_device *dev) if (IS_G4X(dev_priv)) enable_mask |= I915_BSD_USER_INTERRUPT; - GEN3_IRQ_INIT(, dev_priv->irq_mask, enable_mask); + GEN3_IRQ_INIT(GEN2_, dev_priv->irq_mask, enable_mask); /* Interrupt setup is already guaranteed to be single-threaded, this is * just to make the assert_spin_locked check happy. */ @@ -4594,7 +4594,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) u32 hotplug_status = 0; u32 iir; - iir = I915_READ(IIR); + iir = I915_READ(GEN2_IIR); if (iir == 0) break; @@ -4610,7 +4610,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) if (iir & I915_MASTER_ERROR_INTERRUPT) i9xx_error_irq_ack(dev_priv, &eir, &eir_stuck); - I915_WRITE(IIR, iir); + I915_WRITE(GEN2_IIR, iir); if (iir & I915_USER_INTERRUPT) intel_engine_breadcrumbs_irq(dev_priv->engine[RCS0]); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9c206e803ab3..6a150243cabb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2713,10 +2713,10 @@ enum i915_power_well_id { #define VLV_GU_CTL0 _MMIO(VLV_DISPLAY_BASE + 0x2030) #define VLV_GU_CTL1 _MMIO(VLV_DISPLAY_BASE + 0x2034) #define SCPD0 _MMIO(0x209c) /* 915+ only */ -#define IER _MMIO(0x20a0) -#define IIR _MMIO(0x20a4) -#define IMR _MMIO(0x20a8) -#define ISR _MMIO(0x20ac) +#define GEN2_IER _MMIO(0x20a0) +#define GEN2_IIR _MMIO(0x20a4) +#define GEN2_IMR _MMIO(0x20a8) +#define GEN2_ISR _MMIO(0x20ac) #define VLV_GUNIT_CLOCK_GATE _MMIO(VLV_DISPLAY_BASE + 0x2060) #define GINT_DIS (1 << 22) #define GCFG_DIS (1 << 8) diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c index 68875ba43b8d..b75ac660c3c2 100644 --- a/drivers/gpu/drm/i915/i915_reset.c +++ b/drivers/gpu/drm/i915/i915_reset.c @@ -1223,7 +1223,8 @@ void i915_clear_error_registers(struct drm_i915_private *i915) */ DRM_DEBUG_DRIVER("EIR stuck: 0x%08x, masking\n", eir); rmw_set(uncore, EMR, eir); - intel_uncore_write(uncore, IIR, I915_MASTER_ERROR_INTERRUPT); + intel_uncore_write(uncore, GEN2_IIR, + I915_MASTER_ERROR_INTERRUPT); } if (INTEL_GEN(i915) >= 8) { diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index a882b8d42bd9..eb317759b5d3 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -446,7 +446,7 @@ static int intel_overlay_release_old_vid(struct intel_overlay *overlay) if (!overlay->old_vma) return 0; - if (I915_READ(ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) { + if (I915_READ(GEN2_ISR) & I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT) { /* synchronous slowpath */ struct i915_request *rq; @@ -1430,7 +1430,7 @@ intel_overlay_capture_error_state(struct drm_i915_private *dev_priv) return NULL; error->dovsta = I915_READ(DOVSTA); - error->isr = I915_READ(ISR); + error->isr = I915_READ(GEN2_ISR); error->base = overlay->flip_addr; memcpy_fromio(&error->regs, overlay->regs, sizeof(error->regs)); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index af35f99c5940..029fd8ec1857 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -977,15 +977,15 @@ static void i9xx_irq_enable(struct intel_engine_cs *engine) { engine->i915->irq_mask &= ~engine->irq_enable_mask; - intel_uncore_write(engine->uncore, IMR, engine->i915->irq_mask); - intel_uncore_posting_read_fw(engine->uncore, IMR); + intel_uncore_write(engine->uncore, GEN2_IMR, engine->i915->irq_mask); + intel_uncore_posting_read_fw(engine->uncore, GEN2_IMR); } static void i9xx_irq_disable(struct intel_engine_cs *engine) { engine->i915->irq_mask |= engine->irq_enable_mask; - intel_uncore_write(engine->uncore, IMR, engine->i915->irq_mask); + intel_uncore_write(engine->uncore, GEN2_IMR, engine->i915->irq_mask); } static void @@ -994,7 +994,7 @@ i8xx_irq_enable(struct intel_engine_cs *engine) struct drm_i915_private *dev_priv = engine->i915; dev_priv->irq_mask &= ~engine->irq_enable_mask; - I915_WRITE16(IMR, dev_priv->irq_mask); + I915_WRITE16(GEN2_IMR, dev_priv->irq_mask); POSTING_READ16(RING_IMR(engine->mmio_base)); } @@ -1004,7 +1004,7 @@ i8xx_irq_disable(struct intel_engine_cs *engine) struct drm_i915_private *dev_priv = engine->i915; dev_priv->irq_mask |= engine->irq_enable_mask; - I915_WRITE16(IMR, dev_priv->irq_mask); + I915_WRITE16(GEN2_IMR, dev_priv->irq_mask); } static int
This discussion started because we use token pasting in the GEN{2,3}_IRQ_INIT and GEN{2,3}_IRQ_RESET macros, so gen2-4 passes an empty argument to those macros, making the code a little weird. The original proposal was to just add a comment as the empty argument, but Ville suggested we just add a prefix to the registers, and that indeed sounds like a more elegant solution. Now doing this is kinda against our rules for register naming since we only add gens or platform names as register prefixes when the given gen/platform changes a register that already existed before. On the other hand, we have so many instances of IIR/IMR in comments that adding a prefix would make the users of these register more easily findable, in addition to make our token pasting macros actually readable. So IMHO opening an exception here is worth it. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 6 +-- drivers/gpu/drm/i915/i915_gpu_error.c | 4 +- drivers/gpu/drm/i915/i915_irq.c | 52 ++++++++++++------------- drivers/gpu/drm/i915/i915_reg.h | 8 ++-- drivers/gpu/drm/i915/i915_reset.c | 3 +- drivers/gpu/drm/i915/intel_overlay.c | 4 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++--- 7 files changed, 44 insertions(+), 43 deletions(-)