On Tue, Sep 23, 2025 at 05:31:08PM +0300, Jani Nikula wrote: > Split out display irq handling on ilk. Since the master IRQ enable is in > DEIIR, we'll need to do this in two parts. First, add > ilk_display_irq_master_disable() to disable master and south interrupts, > and second, add (repurposed) ilk_display_irq_handler() to finish display > irq handling. > > It's not the prettiest thing you ever saw, but improves separation of > display irq handling. And removes HAS_PCH_NOP() and DISPLAY_VER() checks > from core irq code. > > v2: > - Separate ilk_display_irq_master_enable() (Ville) > - Use _fw mmio accessors (Ville) > > Signed-off-by: Jani Nikula <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]> > --- > .../gpu/drm/i915/display/intel_display_irq.c | 51 ++++++++++++++++++- > .../gpu/drm/i915/display/intel_display_irq.h | 5 +- > drivers/gpu/drm/i915/i915_irq.c | 31 +++-------- > 3 files changed, 58 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c > b/drivers/gpu/drm/i915/display/intel_display_irq.c > index 4d51900123ea..e1a812f6159b 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.c > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c > @@ -872,7 +872,7 @@ static void ilk_gtt_fault_irq_handler(struct > intel_display *display) > } > } > > -void ilk_display_irq_handler(struct intel_display *display, u32 de_iir) > +static void _ilk_display_irq_handler(struct intel_display *display, u32 > de_iir) > { > enum pipe pipe; > u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG; > @@ -923,7 +923,7 @@ void ilk_display_irq_handler(struct intel_display > *display, u32 de_iir) > ilk_display_rps_irq_handler(display); > } > > -void ivb_display_irq_handler(struct intel_display *display, u32 de_iir) > +static void _ivb_display_irq_handler(struct intel_display *display, u32 > de_iir) > { > enum pipe pipe; > u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB; > @@ -972,6 +972,53 @@ void ivb_display_irq_handler(struct intel_display > *display, u32 de_iir) > } > } > > +void ilk_display_irq_master_disable(struct intel_display *display, u32 > *de_ier, u32 *sde_ier) > +{ > + /* disable master interrupt before clearing iir */ > + *de_ier = intel_de_read_fw(display, DEIER); > + intel_de_write_fw(display, DEIER, *de_ier & ~DE_MASTER_IRQ_CONTROL); > + > + /* > + * Disable south interrupts. We'll only write to SDEIIR once, so further > + * interrupts will be stored on its back queue, and then we'll be able > + * to process them after we restore SDEIER (as soon as we restore it, > + * we'll get an interrupt if SDEIIR still has something to process due > + * to its back queue). > + */ > + if (!HAS_PCH_NOP(display)) { > + *sde_ier = intel_de_read_fw(display, SDEIER); > + intel_de_write_fw(display, SDEIER, 0); > + } else { > + *sde_ier = 0; > + } > +} > + > +void ilk_display_irq_master_enable(struct intel_display *display, u32 > de_ier, u32 sde_ier) > +{ > + intel_de_write_fw(display, DEIER, de_ier); > + > + if (sde_ier) > + intel_de_write_fw(display, SDEIER, sde_ier); > +} > + > +bool ilk_display_irq_handler(struct intel_display *display) > +{ > + u32 de_iir; > + bool handled = false; > + > + de_iir = intel_de_read_fw(display, DEIIR); > + if (de_iir) { > + intel_de_write_fw(display, DEIIR, de_iir); > + if (DISPLAY_VER(display) >= 7) > + _ivb_display_irq_handler(display, de_iir); > + else > + _ilk_display_irq_handler(display, de_iir); > + handled = true; > + } > + > + return handled; > +} > + > static u32 gen8_de_port_aux_mask(struct intel_display *display) > { > u32 mask; > diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h > b/drivers/gpu/drm/i915/display/intel_display_irq.h > index e44d88e0d7e7..84acd31948cf 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_irq.h > +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h > @@ -47,8 +47,9 @@ void i965_disable_vblank(struct drm_crtc *crtc); > void ilk_disable_vblank(struct drm_crtc *crtc); > void bdw_disable_vblank(struct drm_crtc *crtc); > > -void ivb_display_irq_handler(struct intel_display *display, u32 de_iir); > -void ilk_display_irq_handler(struct intel_display *display, u32 de_iir); > +void ilk_display_irq_master_disable(struct intel_display *display, u32 > *de_ier, u32 *sde_ier); > +void ilk_display_irq_master_enable(struct intel_display *display, u32 > de_ier, u32 sde_ier); > +bool ilk_display_irq_handler(struct intel_display *display); > void gen8_de_irq_handler(struct intel_display *display, u32 master_ctl); > void gen11_display_irq_handler(struct intel_display *display); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 90174ce9195c..11a727b74849 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -414,7 +414,7 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg) > struct drm_i915_private *i915 = arg; > struct intel_display *display = i915->display; > void __iomem * const regs = intel_uncore_regs(&i915->uncore); > - u32 de_iir, gt_iir, de_ier, sde_ier = 0; > + u32 gt_iir, de_ier = 0, sde_ier = 0; > irqreturn_t ret = IRQ_NONE; > > if (unlikely(!intel_irqs_enabled(i915))) > @@ -423,19 +423,8 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg) > /* IRQs are synced during runtime_suspend, we don't require a wakeref */ > disable_rpm_wakeref_asserts(&i915->runtime_pm); > > - /* disable master interrupt before clearing iir */ > - de_ier = raw_reg_read(regs, DEIER); > - raw_reg_write(regs, DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL); > - > - /* Disable south interrupts. We'll only write to SDEIIR once, so further > - * interrupts will will be stored on its back queue, and then we'll be > - * able to process them after we restore SDEIER (as soon as we restore > - * it, we'll get an interrupt if SDEIIR still has something to process > - * due to its back queue). */ > - if (!HAS_PCH_NOP(display)) { > - sde_ier = raw_reg_read(regs, SDEIER); > - raw_reg_write(regs, SDEIER, 0); > - } > + /* Disable master and south interrupts */ > + ilk_display_irq_master_disable(display, &de_ier, &sde_ier); > > /* Find, clear, then process each source of interrupt */ > > @@ -449,15 +438,8 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg) > ret = IRQ_HANDLED; > } > > - de_iir = raw_reg_read(regs, DEIIR); > - if (de_iir) { > - raw_reg_write(regs, DEIIR, de_iir); > - if (DISPLAY_VER(display) >= 7) > - ivb_display_irq_handler(display, de_iir); > - else > - ilk_display_irq_handler(display, de_iir); > + if (ilk_display_irq_handler(display)) > ret = IRQ_HANDLED; > - } > > if (GRAPHICS_VER(i915) >= 6) { > u32 pm_iir = raw_reg_read(regs, GEN6_PMIIR); > @@ -468,9 +450,8 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg) > } > } > > - raw_reg_write(regs, DEIER, de_ier); > - if (sde_ier) > - raw_reg_write(regs, SDEIER, sde_ier); > + /* Re-enable master and south interrupts */ > + ilk_display_irq_master_enable(display, de_ier, sde_ier); > > pmu_irq_stats(i915, ret); > > -- > 2.47.3 -- Ville Syrjälä Intel
