Re: [PATCH 15/15] drm/i915/irq: add intel_display_irq_handler() to irq funcs

Ville Syrjälä
2026-04-29T11:56:38Z

On Wed, Apr 29, 2026 at 01:24:55PM +0300, Jani Nikula wrote:
> Call the platform specific display irq handler hooks via
> intel_display_irq_handler(). Add master_ctl to struct
> intel_display_irq_state, and pass the state pointer to the handler where
> necessary.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> .../gpu/drm/i915/display/intel_display_irq.c | 95 ++++++++++++++++---
> .../gpu/drm/i915/display/intel_display_irq.h | 11 +--
> drivers/gpu/drm/i915/i915_irq.c | 48 +++-------
> drivers/gpu/drm/xe/display/xe_display.c | 2 +-
> 4 files changed, 98 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index 662081e274bf..5aec1bedbd61 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -23,6 +23,7 @@
> #include "intel_fifo_underrun.h"
> #include "intel_gmbus.h"
> #include "intel_hotplug_irq.h"
> +#include "intel_lpe_audio.h"
> #include "intel_parent.h"
> #include "intel_pipe_crc_regs.h"
> #include "intel_plane.h"
> @@ -596,8 +597,8 @@ static void i9xx_pipestat_irq_ack(struct intel_display *display,
> spin_unlock(&display->irq.lock);
> }
>
> -void i915_pipestat_irq_handler(struct intel_display *display,
> - u32 iir, const u32 pipe_stats[I915_MAX_PIPES])
> +static void i915_pipestat_irq_handler(struct intel_display *display,
> + u32 iir, const u32 pipe_stats[I915_MAX_PIPES])
> {
> bool blc_event = false;
> enum pipe pipe;
> @@ -620,8 +621,8 @@ void i915_pipestat_irq_handler(struct intel_display *display,
> intel_opregion_asle_intr(display);
> }
>
> -void i965_pipestat_irq_handler(struct intel_display *display,
> - u32 iir, const u32 pipe_stats[I915_MAX_PIPES])
> +static void i965_pipestat_irq_handler(struct intel_display *display,
> + u32 iir, const u32 pipe_stats[I915_MAX_PIPES])
> {
> bool blc_event = false;
> enum pipe pipe;
> @@ -647,8 +648,8 @@ void i965_pipestat_irq_handler(struct intel_display *display,
> intel_gmbus_irq_handler(display);
> }
>
> -void valleyview_pipestat_irq_handler(struct intel_display *display,
> - const u32 pipe_stats[I915_MAX_PIPES])
> +static void valleyview_pipestat_irq_handler(struct intel_display *display,
> + const u32 pipe_stats[I915_MAX_PIPES])
> {
> enum pipe pipe;
>
> @@ -1020,7 +1021,8 @@ void ilk_display_irq_master_enable(struct intel_display *display, u32 de_ier, u3
> intel_de_write_fw(display, SDEIER, sde_ier);
> }
>
> -bool ilk_display_irq_handler(struct intel_display *display)
> +static bool ilk_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> {
> u32 de_iir;
> bool handled = false;
> @@ -1404,7 +1406,7 @@ static void gen8_read_and_ack_pch_irqs(struct intel_display *display, u32 *pch_i
> intel_de_write(display, PICAINTERRUPT_IER, pica_ier);
> }
>
> -void gen8_de_irq_handler(struct intel_display *display, u32 master_ctl)
> +static void gen8_de_irq_handler(struct intel_display *display, u32 master_ctl)
> {
> u32 iir;
> enum pipe pipe;
> @@ -1565,6 +1567,14 @@ void gen8_de_irq_handler(struct intel_display *display, u32 master_ctl)
> }
> }
>
> +static bool gen8_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> +{
> + gen8_de_irq_handler(display, state->master_ctl);
> +
> + return true;
> +}
> +
> u32 gen11_gu_misc_irq_ack(struct intel_display *display, const u32 master_ctl)
> {
> u32 iir;
> @@ -1589,7 +1599,8 @@ void gen11_gu_misc_irq_handler(struct intel_display *display, const u32 iir)
> intel_opregion_asle_intr(display);
> }
>
> -void gen11_display_irq_handler(struct intel_display *display)
> +static bool gen11_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> {
> u32 disp_ctl;
>
> @@ -1605,6 +1616,8 @@ void gen11_display_irq_handler(struct intel_display *display)
> intel_de_write(display, GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
>
> intel_display_rpm_assert_unblock(display);
> +
> + return true;
> }
>
> static void i915gm_irq_cstate_wa_enable(struct intel_display *display)
> @@ -1920,8 +1933,8 @@ static void vlv_display_error_irq_ack(struct intel_display *display,
> intel_de_write(display, VLV_EMR, emr);
> }
>
> -void vlv_display_error_irq_handler(struct intel_display *display,
> - u32 eir, u32 dpinvgtt)
> +static void vlv_display_error_irq_handler(struct intel_display *display,
> + u32 eir, u32 dpinvgtt)
> {
> drm_dbg(display->drm, "Master Error, EIR 0x%08x\n", eir);
>
> @@ -2020,6 +2033,28 @@ static void i9xx_display_irq_ack(struct intel_display *display,
> i9xx_pipestat_irq_ack(display, state->iir, state->pipe_stats);
> }
>
> +static bool i965_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> +{
> + if (state->hotplug_status)
> + i9xx_hpd_irq_handler(display, state->hotplug_status);
> +
> + i965_pipestat_irq_handler(display, state->iir, state->pipe_stats);
> +
> + return true;
> +}
> +
> +static bool i915_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> +{
> + if (state->hotplug_status)
> + i9xx_hpd_irq_handler(display, state->hotplug_status);
> +
> + i915_pipestat_irq_handler(display, state->iir, state->pipe_stats);
> +
> + return true;
> +}
> +
> static u32 vlv_error_mask(void)
> {
> /* TODO enable other errors too? */
> @@ -2088,6 +2123,28 @@ static void vlv_display_irq_ack(struct intel_display *display,
> i9xx_pipestat_irq_ack(display, state->iir, state->pipe_stats);
> }
>
> +static bool vlv_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> +{
> + u32 lpe_mask = I915_LPE_PIPE_A_INTERRUPT | I915_LPE_PIPE_B_INTERRUPT;
> +
> + if (display->platform.cherryview)
> + lpe_mask |= I915_LPE_PIPE_C_INTERRUPT;

I would prefer a function rather than the extra variable. The other
option is to just use the CHV mask always. There is nothing on the
extra bit on VLV so we never unmask it.

> +
> + if (state->iir & lpe_mask)
> + intel_lpe_audio_irq_handler(display);
> +
> + if (state->hotplug_status)
> + i9xx_hpd_irq_handler(display, state->hotplug_status);
> +
> + if (state->iir & I915_MASTER_ERROR_INTERRUPT)
> + vlv_display_error_irq_handler(display, state->eir, state->dpinvgtt);
> +
> + valleyview_pipestat_irq_handler(display, state->pipe_stats);
> +
> + return true;
> +}
> +
> static void ibx_display_irq_reset(struct intel_display *display)
> {
> if (HAS_PCH_NOP(display))
> @@ -2476,39 +2533,46 @@ struct intel_display_irq_funcs {
> void (*reset)(struct intel_display *display);
> void (*postinstall)(struct intel_display *display);
> void (*ack)(struct intel_display *display, struct intel_display_irq_state *state);
> + bool (*handler)(struct intel_display *display, const struct intel_display_irq_state *state);
> };
>
> struct intel_display_irq_funcs gen11_display_irq_funcs = {
> .reset = gen11_display_irq_reset,
> .postinstall = gen11_de_irq_postinstall,
> + .handler = gen11_display_irq_handler,
> };
>
> struct intel_display_irq_funcs gen8_display_irq_funcs = {
> .reset = gen8_display_irq_reset,
> .postinstall = gen8_de_irq_postinstall,
> + .handler = gen8_display_irq_handler,
> };
>
> struct intel_display_irq_funcs vlv_display_irq_funcs = {
> .reset = vlv_display_irq_reset,
> .postinstall = vlv_display_irq_postinstall,
> .ack = vlv_display_irq_ack,
> + .handler = vlv_display_irq_handler,
> };
>
> struct intel_display_irq_funcs ilk_display_irq_funcs = {
> .reset = ilk_display_irq_reset,
> .postinstall = ilk_de_irq_postinstall,
> + .handler = ilk_display_irq_handler,
> };
>
> struct intel_display_irq_funcs i965_display_irq_funcs = {
> .reset = i9xx_display_irq_reset,
> .postinstall = i965_display_irq_postinstall,
> .ack = i9xx_display_irq_ack,
> + .handler = i965_display_irq_handler,
> };
>
> struct intel_display_irq_funcs i915_display_irq_funcs = {
> .reset = i9xx_display_irq_reset,
> .postinstall = i915_display_irq_postinstall,
> .ack = i9xx_display_irq_ack,
> + .handler = i915_display_irq_handler,
> };
>
> void intel_display_irq_reset(struct intel_display *display)
> @@ -2528,6 +2592,15 @@ void intel_display_irq_ack(struct intel_display *display,
> display->irq.funcs->ack(display, state);
> }
>
> +bool intel_display_irq_handler(struct intel_display *display,
> + const struct intel_display_irq_state *state)
> +{
> + if (!display->irq.funcs->handler)
> + return true;
> +
> + return display->irq.funcs->handler(display, state);
> +}
> +
> void intel_display_irq_init(struct intel_display *display)
> {
> spin_lock_init(&display->irq.lock);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h
> index 3773a31e48f2..a1227cee885a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h
> @@ -51,14 +51,12 @@ void bdw_disable_vblank(struct drm_crtc *crtc);
>
> 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);
>
> u32 gen11_gu_misc_irq_ack(struct intel_display *display, const u32 master_ctl);
> void gen11_gu_misc_irq_handler(struct intel_display *display, const u32 iir);
>
> struct intel_display_irq_state {
> + u32 master_ctl;

Ideally I'd like a separate structs for different platforms,
but until I resurrect my old ack vs. handle split for all
platforms I guess we don't need anything else here for
ilk+. So good enough for now I suppose.

> u32 iir;
> u32 eir;
> u32 hotplug_status;
> @@ -69,6 +67,7 @@ struct intel_display_irq_state {
> void intel_display_irq_reset(struct intel_display *display);
> void intel_display_irq_postinstall(struct intel_display *display);
> void intel_display_irq_ack(struct intel_display *display, struct intel_display_irq_state *state);
> +bool intel_display_irq_handler(struct intel_display *display, const struct intel_display_irq_state *state);
>
> u32 i9xx_display_irq_enable_mask(struct intel_display *display);
>
> @@ -76,12 +75,6 @@ u32 i915_pipestat_enable_mask(struct intel_display *display, enum pipe pipe);
> void i915_enable_pipestat(struct intel_display *display, enum pipe pipe, u32 status_mask);
> void i915_disable_pipestat(struct intel_display *display, enum pipe pipe, u32 status_mask);
>
> -void i915_pipestat_irq_handler(struct intel_display *display, u32 iir, const u32 pipe_stats[I915_MAX_PIPES]);
> -void i965_pipestat_irq_handler(struct intel_display *display, u32 iir, const u32 pipe_stats[I915_MAX_PIPES]);
> -void valleyview_pipestat_irq_handler(struct intel_display *display, const u32 pipe_stats[I915_MAX_PIPES]);
> -
> -void vlv_display_error_irq_handler(struct intel_display *display, u32 eir, u32 dpinvgtt);
> -
> void intel_display_irq_init(struct intel_display *display);
>
> void i915gm_irq_cstate_wa(struct intel_display *display, bool enable);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bb65ce9d09b2..30ce462e92ab 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -38,8 +38,6 @@
>
> #include "display/intel_display_irq.h"
> #include "display/intel_hotplug.h"
> -#include "display/intel_hotplug_irq.h"
> -#include "display/intel_lpe_audio.h"
>
> #include "gt/intel_breadcrumbs.h"
> #include "gt/intel_gt.h"
> @@ -287,17 +285,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
> if (pm_iir)
> gen6_rps_irq_handler(&to_gt(dev_priv)->rps, pm_iir);
>
> - if (state.iir & (I915_LPE_PIPE_A_INTERRUPT |
> - I915_LPE_PIPE_B_INTERRUPT))
> - intel_lpe_audio_irq_handler(display);
> -
> - if (state.hotplug_status)
> - i9xx_hpd_irq_handler(display, state.hotplug_status);
> -
> - if (state.iir & I915_MASTER_ERROR_INTERRUPT)
> - vlv_display_error_irq_handler(display, state.eir, state.dpinvgtt);
> -
> - valleyview_pipestat_irq_handler(display, state.pipe_stats);
> + intel_display_irq_handler(display, &state);
> } while (0);
>
> pmu_irq_stats(dev_priv, ret);
> @@ -362,18 +350,7 @@ static irqreturn_t cherryview_irq_handler(int irq, void *arg)
> intel_uncore_write(&dev_priv->uncore, VLV_IER, ier);
> intel_uncore_write(&dev_priv->uncore, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
>
> - if (state.iir & (I915_LPE_PIPE_A_INTERRUPT |
> - I915_LPE_PIPE_B_INTERRUPT |
> - I915_LPE_PIPE_C_INTERRUPT))
> - intel_lpe_audio_irq_handler(display);
> -
> - if (state.hotplug_status)
> - i9xx_hpd_irq_handler(display, state.hotplug_status);
> -
> - if (state.iir & I915_MASTER_ERROR_INTERRUPT)
> - vlv_display_error_irq_handler(display, state.eir, state.dpinvgtt);
> -
> - valleyview_pipestat_irq_handler(display, state.pipe_stats);
> + intel_display_irq_handler(display, &state);
> } while (0);
>
> pmu_irq_stats(dev_priv, ret);
> @@ -420,7 +397,7 @@ static irqreturn_t ilk_irq_handler(int irq, void *arg)
> ret = IRQ_HANDLED;
> }
>
> - if (ilk_display_irq_handler(display))
> + if (intel_display_irq_handler(display, NULL))
> ret = IRQ_HANDLED;
>
> if (GRAPHICS_VER(i915) >= 6) {
> @@ -482,8 +459,11 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
>
> /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> if (master_ctl & ~GEN8_GT_IRQS) {
> + const struct intel_display_irq_state state = {
> + .master_ctl = master_ctl,
> + };
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> - gen8_de_irq_handler(display, master_ctl);
> + intel_display_irq_handler(display, &state);
> enable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
> }
>
> @@ -535,7 +515,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>
> /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> if (master_ctl & GEN11_DISPLAY_IRQ)
> - gen11_display_irq_handler(display);
> + intel_display_irq_handler(display, NULL);
>
> gu_misc_iir = gen11_gu_misc_irq_ack(display, master_ctl);
>
> @@ -602,7 +582,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg)
> gen11_gt_irq_handler(gt, master_ctl);
>
> if (master_ctl & GEN11_DISPLAY_IRQ)
> - gen11_display_irq_handler(display);
> + intel_display_irq_handler(display, NULL);
>
> gu_misc_iir = gen11_gu_misc_irq_ack(display, master_ctl);
>
> @@ -906,10 +886,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
> if (state.iir & I915_MASTER_ERROR_INTERRUPT)
> i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
>
> - if (state.hotplug_status)
> - i9xx_hpd_irq_handler(display, state.hotplug_status);
> -
> - i915_pipestat_irq_handler(display, state.iir, state.pipe_stats);
> + intel_display_irq_handler(display, &state);
> } while (0);
>
> pmu_irq_stats(dev_priv, ret);
> @@ -1013,10 +990,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
> if (state.iir & I915_MASTER_ERROR_INTERRUPT)
> i9xx_error_irq_handler(dev_priv, eir, eir_stuck);
>
> - if (state.hotplug_status)
> - i9xx_hpd_irq_handler(display, state.hotplug_status);
> -
> - i965_pipestat_irq_handler(display, state.iir, state.pipe_stats);
> + intel_display_irq_handler(display, &state);
> } while (0);
>
> pmu_irq_stats(dev_priv, IRQ_HANDLED);
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index 736a5e6938d6..4f283fb79554 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -214,7 +214,7 @@ void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl)
> return;
>
> if (master_ctl & DISPLAY_IRQ)
> - gen11_display_irq_handler(display);
> + intel_display_irq_handler(display, NULL);
> }
>
> void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir)
> --
> 2.47.3

--
Ville Syrjälä
Intel