On Tue, Nov 11, 2025 at 09:34:02AM +0200, Jani Nikula wrote: > Duplicate gen2_irq_reset(), gen2_assert_iir_is_zero(), gen2_irq_init(), > gen2_error_reset(), and gen2_error_init() in intel_display_irq.c. > > This allows us to drop the duplicates from xe, and prepares for future > cleanups. Although duplication is undesirable in general, in this case > the local duplicates lead to a cleaner end result. > > There's a slight wrinkle in gen2_assert_iir_is_zero(). We need to use > non-device based logging until we pass in struct intel_display in a > separate change. > > v2: > - Keep xe compat stuff due to series reorder and rebase > - Keep the WARN as regular WARN > - Rename the functions in the same go > > Suggested-by: Ville Syrjala <[email protected]> > Signed-off-by: Jani Nikula <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]> > > --- > > Note: 'git show --color-moved' helps review > --- > .../gpu/drm/i915/display/intel_display_irq.c | 82 +++++++++++++++++-- > drivers/gpu/drm/xe/display/ext/i915_irq.c | 67 --------------- > 2 files changed, 73 insertions(+), 76 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c > b/drivers/gpu/drm/i915/display/intel_display_irq.c > index 43b27deb4a26..acfaff13c3ba 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > @@ -33,6 +33,72 @@ > #include "intel_psr_regs.h" > #include "intel_uncore.h" > > +static void irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs) > +{ > + intel_uncore_write(uncore, regs.imr, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.imr); > + > + intel_uncore_write(uncore, regs.ier, 0); > + > + /* IIR can theoretically queue up two events. Be paranoid. */ > + intel_uncore_write(uncore, regs.iir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.iir); > + intel_uncore_write(uncore, regs.iir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.iir); > +} > + > +/* > + * We should clear IMR at preinstall/uninstall, and just check at > postinstall. > + */ > +static void assert_iir_is_zero(struct intel_uncore *uncore, i915_reg_t reg) > +{ > + u32 val = intel_uncore_read(uncore, reg); > + > + if (val == 0) > + return; > + > + WARN(1, > + "Interrupt register 0x%x is not zero: 0x%08x\n", > + i915_mmio_reg_offset(reg), val); > + intel_uncore_write(uncore, reg, 0xffffffff); > + intel_uncore_posting_read(uncore, reg); > + intel_uncore_write(uncore, reg, 0xffffffff); > + intel_uncore_posting_read(uncore, reg); > +} > + > +static void irq_init(struct intel_uncore *uncore, struct i915_irq_regs regs, > + u32 imr_val, u32 ier_val) > +{ > + assert_iir_is_zero(uncore, regs.iir); > + > + intel_uncore_write(uncore, regs.ier, ier_val); > + intel_uncore_write(uncore, regs.imr, imr_val); > + intel_uncore_posting_read(uncore, regs.imr); > +} > + > +static void error_reset(struct intel_uncore *uncore, struct i915_error_regs > regs) > +{ > + intel_uncore_write(uncore, regs.emr, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.emr); > + > + intel_uncore_write(uncore, regs.eir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.eir); > + intel_uncore_write(uncore, regs.eir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.eir); > +} > + > +static void error_init(struct intel_uncore *uncore, struct i915_error_regs > regs, > + u32 emr_val) > +{ > + intel_uncore_write(uncore, regs.eir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.eir); > + intel_uncore_write(uncore, regs.eir, 0xffffffff); > + intel_uncore_posting_read(uncore, regs.eir); > + > + intel_uncore_write(uncore, regs.emr, emr_val); > + intel_uncore_posting_read(uncore, regs.emr); > +} > + > static void > intel_display_irq_regs_init(struct intel_display *display, struct > i915_irq_regs regs, > u32 imr_val, u32 ier_val) > @@ -41,7 +107,7 @@ intel_display_irq_regs_init(struct intel_display *display, > struct i915_irq_regs > intel_dmc_wl_get(display, regs.ier); > intel_dmc_wl_get(display, regs.iir); > > - gen2_irq_init(to_intel_uncore(display->drm), regs, imr_val, ier_val); > + irq_init(to_intel_uncore(display->drm), regs, imr_val, ier_val); > > intel_dmc_wl_put(display, regs.iir); > intel_dmc_wl_put(display, regs.ier); > @@ -55,7 +121,7 @@ intel_display_irq_regs_reset(struct intel_display > *display, struct i915_irq_regs > intel_dmc_wl_get(display, regs.ier); > intel_dmc_wl_get(display, regs.iir); > > - gen2_irq_reset(to_intel_uncore(display->drm), regs); > + irq_reset(to_intel_uncore(display->drm), regs); > > intel_dmc_wl_put(display, regs.iir); > intel_dmc_wl_put(display, regs.ier); > @@ -67,7 +133,7 @@ intel_display_irq_regs_assert_irr_is_zero(struct > intel_display *display, i915_re > { > intel_dmc_wl_get(display, reg); > > - gen2_assert_iir_is_zero(to_intel_uncore(display->drm), reg); > + assert_iir_is_zero(to_intel_uncore(display->drm), reg); > > intel_dmc_wl_put(display, reg); > } > @@ -1918,8 +1984,7 @@ static void _vlv_display_irq_reset(struct intel_display > *display) > else > intel_de_write(display, DPINVGTT, DPINVGTT_STATUS_MASK_VLV); > > - gen2_error_reset(to_intel_uncore(display->drm), > - VLV_ERROR_REGS); > + error_reset(to_intel_uncore(display->drm), VLV_ERROR_REGS); > > i915_hotplug_interrupt_update_locked(display, 0xffffffff, 0); > intel_de_rmw(display, PORT_HOTPLUG_STAT(display), 0, 0); > @@ -2014,8 +2079,7 @@ static void _vlv_display_irq_postinstall(struct > intel_display *display) > DPINVGTT_STATUS_MASK_VLV | > DPINVGTT_EN_MASK_VLV); > > - gen2_error_init(to_intel_uncore(display->drm), > - VLV_ERROR_REGS, ~vlv_error_mask()); > + error_init(to_intel_uncore(display->drm), VLV_ERROR_REGS, > ~vlv_error_mask()); > > pipestat_mask = PIPE_CRC_DONE_INTERRUPT_STATUS; > > @@ -2054,7 +2118,7 @@ static void ibx_display_irq_reset(struct intel_display > *display) > if (HAS_PCH_NOP(display)) > return; > > - gen2_irq_reset(to_intel_uncore(display->drm), SDE_IRQ_REGS); > + irq_reset(to_intel_uncore(display->drm), SDE_IRQ_REGS); > > if (HAS_PCH_CPT(display) || HAS_PCH_LPT(display)) > intel_de_write(display, SERR_INT, 0xffffffff); > @@ -2064,7 +2128,7 @@ void ilk_display_irq_reset(struct intel_display > *display) > { > struct intel_uncore *uncore = to_intel_uncore(display->drm); > > - gen2_irq_reset(uncore, DE_IRQ_REGS); > + irq_reset(uncore, DE_IRQ_REGS); > display->irq.ilk_de_imr_mask = ~0u; > > if (DISPLAY_VER(display) == 7) > diff --git a/drivers/gpu/drm/xe/display/ext/i915_irq.c > b/drivers/gpu/drm/xe/display/ext/i915_irq.c > index 3c6bca66ddab..1011c1c754d0 100644 > --- a/drivers/gpu/drm/xe/display/ext/i915_irq.c > +++ b/drivers/gpu/drm/xe/display/ext/i915_irq.c > @@ -7,73 +7,6 @@ > #include "i915_reg.h" > #include "intel_uncore.h" > > -void gen2_irq_reset(struct intel_uncore *uncore, struct i915_irq_regs regs) > -{ > - intel_uncore_write(uncore, regs.imr, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.imr); > - > - intel_uncore_write(uncore, regs.ier, 0); > - > - /* IIR can theoretically queue up two events. Be paranoid. */ > - intel_uncore_write(uncore, regs.iir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.iir); > - intel_uncore_write(uncore, regs.iir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.iir); > -} > - > -/* > - * We should clear IMR at preinstall/uninstall, and just check at > postinstall. > - */ > -void gen2_assert_iir_is_zero(struct intel_uncore *uncore, i915_reg_t reg) > -{ > - struct xe_device *xe = container_of(uncore, struct xe_device, uncore); > - u32 val = intel_uncore_read(uncore, reg); > - > - if (val == 0) > - return; > - > - drm_WARN(&xe->drm, 1, > - "Interrupt register 0x%x is not zero: 0x%08x\n", > - i915_mmio_reg_offset(reg), val); > - intel_uncore_write(uncore, reg, 0xffffffff); > - intel_uncore_posting_read(uncore, reg); > - intel_uncore_write(uncore, reg, 0xffffffff); > - intel_uncore_posting_read(uncore, reg); > -} > - > -void gen2_irq_init(struct intel_uncore *uncore, struct i915_irq_regs regs, > - u32 imr_val, u32 ier_val) > -{ > - gen2_assert_iir_is_zero(uncore, regs.iir); > - > - intel_uncore_write(uncore, regs.ier, ier_val); > - intel_uncore_write(uncore, regs.imr, imr_val); > - intel_uncore_posting_read(uncore, regs.imr); > -} > - > -void gen2_error_reset(struct intel_uncore *uncore, struct i915_error_regs > regs) > -{ > - intel_uncore_write(uncore, regs.emr, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.emr); > - > - intel_uncore_write(uncore, regs.eir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.eir); > - intel_uncore_write(uncore, regs.eir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.eir); > -} > - > -void gen2_error_init(struct intel_uncore *uncore, struct i915_error_regs > regs, > - u32 emr_val) > -{ > - intel_uncore_write(uncore, regs.eir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.eir); > - intel_uncore_write(uncore, regs.eir, 0xffffffff); > - intel_uncore_posting_read(uncore, regs.eir); > - > - intel_uncore_write(uncore, regs.emr, emr_val); > - intel_uncore_posting_read(uncore, regs.emr); > -} > - > bool intel_irqs_enabled(struct xe_device *xe) > { > return atomic_read(&xe->irq.enabled); > -- > 2.47.3 -- Ville Syrjälä Intel
