| Message ID | 20190409003729.20857-2-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IRQ initialization debloat and conversion to uncore | expand |
On Mon, Apr 08, 2019 at 05:37:27PM -0700, Paulo Zanoni wrote: > The whole point of having macros here is for the token pasting > necessary to automatically have IMR, IIR and IER selected. We don't > really need or want all the inlining that happens as a consequence. > The good thing about the current code is that it works regardless of > the relative offsets between these registers (they change after gen3). Did you mean "after gen4"? The DE/GT split happened at ilk, and I guess that's when the four registers also got reshuffled for some reason. Ignoring the funkyness of vlv/chv or course :) > > One thing which we can do is to split the logic of what we do with > imr/ier/iir to functions separate from the macros that pick them. > That's what we do in this commit. This allows us to get rid of the > gen8 duplicates and also all the inlining: > > add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623) > Function old new delta > gen3_irq_reset - 233 +233 > gen3_irq_init - 150 +150 > i8xx_irq_postinstall 459 442 -17 > gen11_irq_postinstall 804 744 -60 > ironlake_irq_postinstall 450 353 -97 > vlv_display_irq_postinstall 348 245 -103 > i965_irq_postinstall 378 275 -103 > i915_irq_postinstall 333 228 -105 > gen8_irq_power_well_post_enable 374 210 -164 > ironlake_irq_reset 397 218 -179 > vlv_display_irq_reset 616 433 -183 > i965_irq_reset 374 180 -194 > cherryview_irq_reset 379 185 -194 > i915_irq_reset 407 209 -198 > ibx_irq_reset 332 133 -199 > gen5_gt_irq_postinstall 533 332 -201 > gen8_irq_power_well_pre_disable 434 204 -230 > gen8_gt_irq_postinstall 469 196 -273 > gen8_de_irq_postinstall 1200 805 -395 > gen5_gt_irq_reset 471 76 -395 > gen8_gt_irq_reset 775 99 -676 > gen8_irq_reset 1100 333 -767 > gen11_irq_reset 1959 686 -1273 > Total: Before=2262051, After=2256428, chg -0.25% > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++------------- > 1 file changed, 73 insertions(+), 50 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 6454ddc37f8b..a1e7944fb5d0 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = { > [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP > }; > > -/* IIR can theoretically queue up two events. Be paranoid. */ > -#define GEN8_IRQ_RESET_NDX(type, which) do { \ > - I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ > - POSTING_READ(GEN8_##type##_IMR(which)); \ > - I915_WRITE(GEN8_##type##_IER(which), 0); \ > - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ > - POSTING_READ(GEN8_##type##_IIR(which)); \ > - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ > - POSTING_READ(GEN8_##type##_IIR(which)); \ > -} while (0) > - > -#define GEN3_IRQ_RESET(type) do { \ > - I915_WRITE(type##IMR, 0xffffffff); \ > - POSTING_READ(type##IMR); \ > - I915_WRITE(type##IER, 0); \ > - I915_WRITE(type##IIR, 0xffffffff); \ > - POSTING_READ(type##IIR); \ > - I915_WRITE(type##IIR, 0xffffffff); \ > - POSTING_READ(type##IIR); \ > -} while (0) > - > -#define GEN2_IRQ_RESET(type) do { \ > - I915_WRITE16(type##IMR, 0xffff); \ > - POSTING_READ16(type##IMR); \ > - I915_WRITE16(type##IER, 0); \ > - I915_WRITE16(type##IIR, 0xffff); \ > - POSTING_READ16(type##IIR); \ > - I915_WRITE16(type##IIR, 0xffff); \ > - POSTING_READ16(type##IIR); \ > -} while (0) > +static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, > + i915_reg_t iir, i915_reg_t ier) > +{ > + I915_WRITE(imr, 0xffffffff); > + POSTING_READ(imr); > + > + I915_WRITE(ier, 0); > + > + /* IIR can theoretically queue up two events. Be paranoid. */ > + I915_WRITE(iir, 0xffffffff); > + POSTING_READ(iir); > + I915_WRITE(iir, 0xffffffff); > + POSTING_READ(iir); > +} > + > +static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, > + i915_reg_t iir, i915_reg_t ier) > +{ > + I915_WRITE16(imr, 0xffff); > + POSTING_READ16(imr); > + > + I915_WRITE16(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); > +} > + > +#define GEN8_IRQ_RESET_NDX(type, which) \ > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > + > +#define GEN3_IRQ_RESET(type) \ > + gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) > + > +#define GEN2_IRQ_RESET(type) \ > + gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) > > /* > * We should clear IMR at preinstall/uninstall, and just check at postinstall. > @@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv, > POSTING_READ16(reg); > } > > -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \ > - gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \ > - I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \ > - I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \ > - POSTING_READ(GEN8_##type##_IMR(which)); \ > -} while (0) > - > -#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \ > - gen3_assert_iir_is_zero(dev_priv, type##IIR); \ > - I915_WRITE(type##IER, (ier_val)); \ > - I915_WRITE(type##IMR, (imr_val)); \ > - POSTING_READ(type##IMR); \ > -} while (0) > - > -#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \ > - gen2_assert_iir_is_zero(dev_priv, type##IIR); \ > - I915_WRITE16(type##IER, (ier_val)); \ > - I915_WRITE16(type##IMR, (imr_val)); \ > - POSTING_READ16(type##IMR); \ > -} while (0) > +static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, > + i915_reg_t iir, i915_reg_t ier, u32 imr_val, > + u32 ier_val) Maybe we should order these more like this foo(imr, imr_value, ier, ier_value, iir) ? Could make a bit more obvious which values goes to which register. But I suppose as long as they're always wrapped in those macros it doesn't really matter. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > +{ > + gen3_assert_iir_is_zero(dev_priv, iir); > + > + I915_WRITE(ier, ier_val); > + I915_WRITE(imr, imr_val); > + POSTING_READ(imr); > +} > + > +static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, > + i915_reg_t iir, i915_reg_t ier, u32 imr_val, > + u32 ier_val) > +{ > + gen2_assert_iir_is_zero(dev_priv, iir); > + > + I915_WRITE16(ier, ier_val); > + I915_WRITE16(imr, imr_val); > + POSTING_READ16(imr); > +} > + > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > + imr_val, ier_val) > + > +#define GEN3_IRQ_INIT(type, imr_val, ier_val) \ > + gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ > + ier_val) > + > +#define GEN2_IRQ_INIT(type, imr_val, ier_val) \ > + gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ > + ier_val) > > static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); > static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em ter, 2019-04-09 às 21:10 +0300, Ville Syrjälä escreveu: > On Mon, Apr 08, 2019 at 05:37:27PM -0700, Paulo Zanoni wrote: > > The whole point of having macros here is for the token pasting > > necessary to automatically have IMR, IIR and IER selected. We don't > > really need or want all the inlining that happens as a consequence. > > The good thing about the current code is that it works regardless of > > the relative offsets between these registers (they change after gen3). > > Did you mean "after gen4"? The DE/GT split happened at ilk, and I > guess that's when the four registers also got reshuffled for some > reason. Ignoring the funkyness of vlv/chv or course :) > You're right. I'll fix the commit message. > > One thing which we can do is to split the logic of what we do with > > imr/ier/iir to functions separate from the macros that pick them. > > That's what we do in this commit. This allows us to get rid of the > > gen8 duplicates and also all the inlining: > > > > add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623) > > Function old new delta > > gen3_irq_reset - 233 +233 > > gen3_irq_init - 150 +150 > > i8xx_irq_postinstall 459 442 -17 > > gen11_irq_postinstall 804 744 -60 > > ironlake_irq_postinstall 450 353 -97 > > vlv_display_irq_postinstall 348 245 -103 > > i965_irq_postinstall 378 275 -103 > > i915_irq_postinstall 333 228 -105 > > gen8_irq_power_well_post_enable 374 210 -164 > > ironlake_irq_reset 397 218 -179 > > vlv_display_irq_reset 616 433 -183 > > i965_irq_reset 374 180 -194 > > cherryview_irq_reset 379 185 -194 > > i915_irq_reset 407 209 -198 > > ibx_irq_reset 332 133 -199 > > gen5_gt_irq_postinstall 533 332 -201 > > gen8_irq_power_well_pre_disable 434 204 -230 > > gen8_gt_irq_postinstall 469 196 -273 > > gen8_de_irq_postinstall 1200 805 -395 > > gen5_gt_irq_reset 471 76 -395 > > gen8_gt_irq_reset 775 99 -676 > > gen8_irq_reset 1100 333 -767 > > gen11_irq_reset 1959 686 -1273 > > Total: Before=2262051, After=2256428, chg -0.25% > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++------------- > > 1 file changed, 73 insertions(+), 50 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 6454ddc37f8b..a1e7944fb5d0 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = { > > [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP > > }; > > > > -/* IIR can theoretically queue up two events. Be paranoid. */ > > -#define GEN8_IRQ_RESET_NDX(type, which) do { \ > > - I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ > > - POSTING_READ(GEN8_##type##_IMR(which)); \ > > - I915_WRITE(GEN8_##type##_IER(which), 0); \ > > - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ > > - POSTING_READ(GEN8_##type##_IIR(which)); \ > > - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ > > - POSTING_READ(GEN8_##type##_IIR(which)); \ > > -} while (0) > > - > > -#define GEN3_IRQ_RESET(type) do { \ > > - I915_WRITE(type##IMR, 0xffffffff); \ > > - POSTING_READ(type##IMR); \ > > - I915_WRITE(type##IER, 0); \ > > - I915_WRITE(type##IIR, 0xffffffff); \ > > - POSTING_READ(type##IIR); \ > > - I915_WRITE(type##IIR, 0xffffffff); \ > > - POSTING_READ(type##IIR); \ > > -} while (0) > > - > > -#define GEN2_IRQ_RESET(type) do { \ > > - I915_WRITE16(type##IMR, 0xffff); \ > > - POSTING_READ16(type##IMR); \ > > - I915_WRITE16(type##IER, 0); \ > > - I915_WRITE16(type##IIR, 0xffff); \ > > - POSTING_READ16(type##IIR); \ > > - I915_WRITE16(type##IIR, 0xffff); \ > > - POSTING_READ16(type##IIR); \ > > -} while (0) > > +static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, > > + i915_reg_t iir, i915_reg_t ier) > > +{ > > + I915_WRITE(imr, 0xffffffff); > > + POSTING_READ(imr); > > + > > + I915_WRITE(ier, 0); > > + > > + /* IIR can theoretically queue up two events. Be paranoid. */ > > + I915_WRITE(iir, 0xffffffff); > > + POSTING_READ(iir); > > + I915_WRITE(iir, 0xffffffff); > > + POSTING_READ(iir); > > +} > > + > > +static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, > > + i915_reg_t iir, i915_reg_t ier) > > +{ > > + I915_WRITE16(imr, 0xffff); > > + POSTING_READ16(imr); > > + > > + I915_WRITE16(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); > > +} > > + > > +#define GEN8_IRQ_RESET_NDX(type, which) \ > > + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) > > + > > +#define GEN3_IRQ_RESET(type) \ > > + gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) > > + > > +#define GEN2_IRQ_RESET(type) \ > > + gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) > > > > /* > > * We should clear IMR at preinstall/uninstall, and just check at postinstall. > > @@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv, > > POSTING_READ16(reg); > > } > > > > -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \ > > - gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \ > > - I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \ > > - I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \ > > - POSTING_READ(GEN8_##type##_IMR(which)); \ > > -} while (0) > > - > > -#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \ > > - gen3_assert_iir_is_zero(dev_priv, type##IIR); \ > > - I915_WRITE(type##IER, (ier_val)); \ > > - I915_WRITE(type##IMR, (imr_val)); \ > > - POSTING_READ(type##IMR); \ > > -} while (0) > > - > > -#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \ > > - gen2_assert_iir_is_zero(dev_priv, type##IIR); \ > > - I915_WRITE16(type##IER, (ier_val)); \ > > - I915_WRITE16(type##IMR, (imr_val)); \ > > - POSTING_READ16(type##IMR); \ > > -} while (0) > > +static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, > > + i915_reg_t iir, i915_reg_t ier, u32 imr_val, > > + u32 ier_val) > > Maybe we should order these more like this > foo(imr, imr_value, ier, ier_value, iir) ? Sure. I'll also try to add a few extra line breaks to the calls to try to make the arguments a little more readable. > > Could make a bit more obvious which values goes to which register. But > I suppose as long as they're always wrapped in those macros it doesn't > really matter. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks a lot! > > > +{ > > + gen3_assert_iir_is_zero(dev_priv, iir); > > + > > + I915_WRITE(ier, ier_val); > > + I915_WRITE(imr, imr_val); > > + POSTING_READ(imr); > > +} > > + > > +static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, > > + i915_reg_t iir, i915_reg_t ier, u32 imr_val, > > + u32 ier_val) > > +{ > > + gen2_assert_iir_is_zero(dev_priv, iir); > > + > > + I915_WRITE16(ier, ier_val); > > + I915_WRITE16(imr, imr_val); > > + POSTING_READ16(imr); > > +} > > + > > +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ > > + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ > > + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ > > + imr_val, ier_val) > > + > > +#define GEN3_IRQ_INIT(type, imr_val, ier_val) \ > > + gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ > > + ier_val) > > + > > +#define GEN2_IRQ_INIT(type, imr_val, ier_val) \ > > + gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ > > + ier_val) > > > > static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); > > static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); > > -- > > 2.20.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6454ddc37f8b..a1e7944fb5d0 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -136,36 +136,45 @@ static const u32 hpd_icp[HPD_NUM_PINS] = { [HPD_PORT_F] = SDE_TC4_HOTPLUG_ICP }; -/* IIR can theoretically queue up two events. Be paranoid. */ -#define GEN8_IRQ_RESET_NDX(type, which) do { \ - I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \ - POSTING_READ(GEN8_##type##_IMR(which)); \ - I915_WRITE(GEN8_##type##_IER(which), 0); \ - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ - POSTING_READ(GEN8_##type##_IIR(which)); \ - I915_WRITE(GEN8_##type##_IIR(which), 0xffffffff); \ - POSTING_READ(GEN8_##type##_IIR(which)); \ -} while (0) - -#define GEN3_IRQ_RESET(type) do { \ - I915_WRITE(type##IMR, 0xffffffff); \ - POSTING_READ(type##IMR); \ - I915_WRITE(type##IER, 0); \ - I915_WRITE(type##IIR, 0xffffffff); \ - POSTING_READ(type##IIR); \ - I915_WRITE(type##IIR, 0xffffffff); \ - POSTING_READ(type##IIR); \ -} while (0) - -#define GEN2_IRQ_RESET(type) do { \ - I915_WRITE16(type##IMR, 0xffff); \ - POSTING_READ16(type##IMR); \ - I915_WRITE16(type##IER, 0); \ - I915_WRITE16(type##IIR, 0xffff); \ - POSTING_READ16(type##IIR); \ - I915_WRITE16(type##IIR, 0xffff); \ - POSTING_READ16(type##IIR); \ -} while (0) +static void gen3_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, + i915_reg_t iir, i915_reg_t ier) +{ + I915_WRITE(imr, 0xffffffff); + POSTING_READ(imr); + + I915_WRITE(ier, 0); + + /* IIR can theoretically queue up two events. Be paranoid. */ + I915_WRITE(iir, 0xffffffff); + POSTING_READ(iir); + I915_WRITE(iir, 0xffffffff); + POSTING_READ(iir); +} + +static void gen2_irq_reset(struct drm_i915_private *dev_priv, i915_reg_t imr, + i915_reg_t iir, i915_reg_t ier) +{ + I915_WRITE16(imr, 0xffff); + POSTING_READ16(imr); + + I915_WRITE16(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); +} + +#define GEN8_IRQ_RESET_NDX(type, which) \ + gen3_irq_reset(dev_priv, GEN8_##type##_IMR(which), \ + GEN8_##type##_IIR(which), GEN8_##type##_IER(which)) + +#define GEN3_IRQ_RESET(type) \ + gen3_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) + +#define GEN2_IRQ_RESET(type) \ + gen2_irq_reset(dev_priv, type##IMR, type##IIR, type##IER) /* * We should clear IMR at preinstall/uninstall, and just check at postinstall. @@ -202,26 +211,40 @@ static void gen2_assert_iir_is_zero(struct drm_i915_private *dev_priv, POSTING_READ16(reg); } -#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) do { \ - gen3_assert_iir_is_zero(dev_priv, GEN8_##type##_IIR(which)); \ - I915_WRITE(GEN8_##type##_IER(which), (ier_val)); \ - I915_WRITE(GEN8_##type##_IMR(which), (imr_val)); \ - POSTING_READ(GEN8_##type##_IMR(which)); \ -} while (0) - -#define GEN3_IRQ_INIT(type, imr_val, ier_val) do { \ - gen3_assert_iir_is_zero(dev_priv, type##IIR); \ - I915_WRITE(type##IER, (ier_val)); \ - I915_WRITE(type##IMR, (imr_val)); \ - POSTING_READ(type##IMR); \ -} while (0) - -#define GEN2_IRQ_INIT(type, imr_val, ier_val) do { \ - gen2_assert_iir_is_zero(dev_priv, type##IIR); \ - I915_WRITE16(type##IER, (ier_val)); \ - I915_WRITE16(type##IMR, (imr_val)); \ - POSTING_READ16(type##IMR); \ -} while (0) +static void gen3_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, + i915_reg_t iir, i915_reg_t ier, u32 imr_val, + u32 ier_val) +{ + gen3_assert_iir_is_zero(dev_priv, iir); + + I915_WRITE(ier, ier_val); + I915_WRITE(imr, imr_val); + POSTING_READ(imr); +} + +static void gen2_irq_init(struct drm_i915_private *dev_priv, i915_reg_t imr, + i915_reg_t iir, i915_reg_t ier, u32 imr_val, + u32 ier_val) +{ + gen2_assert_iir_is_zero(dev_priv, iir); + + I915_WRITE16(ier, ier_val); + I915_WRITE16(imr, imr_val); + POSTING_READ16(imr); +} + +#define GEN8_IRQ_INIT_NDX(type, which, imr_val, ier_val) \ + gen3_irq_init(dev_priv, GEN8_##type##_IMR(which), \ + GEN8_##type##_IIR(which), GEN8_##type##_IER(which), \ + imr_val, ier_val) + +#define GEN3_IRQ_INIT(type, imr_val, ier_val) \ + gen3_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ + ier_val) + +#define GEN2_IRQ_INIT(type, imr_val, ier_val) \ + gen2_irq_init(dev_priv, type##IMR, type##IIR, type##IER, imr_val, \ + ier_val) static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir); static void gen9_guc_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
The whole point of having macros here is for the token pasting necessary to automatically have IMR, IIR and IER selected. We don't really need or want all the inlining that happens as a consequence. The good thing about the current code is that it works regardless of the relative offsets between these registers (they change after gen3). One thing which we can do is to split the logic of what we do with imr/ier/iir to functions separate from the macros that pick them. That's what we do in this commit. This allows us to get rid of the gen8 duplicates and also all the inlining: add/remove: 2/0 grow/shrink: 0/21 up/down: 383/-6006 (-5623) Function old new delta gen3_irq_reset - 233 +233 gen3_irq_init - 150 +150 i8xx_irq_postinstall 459 442 -17 gen11_irq_postinstall 804 744 -60 ironlake_irq_postinstall 450 353 -97 vlv_display_irq_postinstall 348 245 -103 i965_irq_postinstall 378 275 -103 i915_irq_postinstall 333 228 -105 gen8_irq_power_well_post_enable 374 210 -164 ironlake_irq_reset 397 218 -179 vlv_display_irq_reset 616 433 -183 i965_irq_reset 374 180 -194 cherryview_irq_reset 379 185 -194 i915_irq_reset 407 209 -198 ibx_irq_reset 332 133 -199 gen5_gt_irq_postinstall 533 332 -201 gen8_irq_power_well_pre_disable 434 204 -230 gen8_gt_irq_postinstall 469 196 -273 gen8_de_irq_postinstall 1200 805 -395 gen5_gt_irq_reset 471 76 -395 gen8_gt_irq_reset 775 99 -676 gen8_irq_reset 1100 333 -767 gen11_irq_reset 1959 686 -1273 Total: Before=2262051, After=2256428, chg -0.25% Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 123 +++++++++++++++++++------------- 1 file changed, 73 insertions(+), 50 deletions(-)