From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 15/15] drm/i915/irq: add intel_display_irq_handler() to irq funcs
Date: Wed, 29 Apr 2026 14:56:30 +0300 [thread overview]
Message-ID: <afHx7st1NYZulnQM@intel.com> (raw)
In-Reply-To: <4c2be8d52d11977f67d3d52ebf6414d4c0a341ef.1777458161.git.jani.nikula@intel.com>
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
next prev parent reply other threads:[~2026-04-29 11:56 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-29 10:24 [PATCH 00/15] drm/i915: refactor display funcs, add display irq hooks Jani Nikula
2026-04-29 10:24 ` [PATCH 01/15] drm/i915/display: move audio funcs under audio sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 02/15] drm/i915/display: move color funcs under color sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 03/15] drm/i915/display: move fdi funcs under fdi sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 04/15] drm/i915/display: move watermark funcs under wm sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 05/15] drm/i915/display: move hotplug irq funcs under hotplug sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 06/15] drm/i915/display: move dpll funcs under dpll sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 07/15] drm/i915/display: move cdclk funcs under cdclk sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 08/15] drm/i915/display: move display funcs under modeset sub-struct Jani Nikula
2026-04-29 10:24 ` [PATCH 09/15] drm/i915/irq: deduplicate dg1_de_irq_postinstall() and gen11_de_irq_postinstall() Jani Nikula
2026-04-29 10:24 ` [PATCH 10/15] drm/i915/irq: move VLV/CHV LPE irq handler call after irq acks Jani Nikula
2026-04-29 11:12 ` Ville Syrjälä
2026-04-30 7:49 ` Jani Nikula
2026-04-29 10:24 ` [PATCH 11/15] drm/i915/irq: constify pipe stats parameters Jani Nikula
2026-04-29 10:24 ` [PATCH 12/15] drm/i915/irq: add display irq funcs, start with intel_display_irq_reset() Jani Nikula
2026-04-29 10:24 ` [PATCH 13/15] drm/i915/irq: add intel_display_irq_postinstall() to irq funcs Jani Nikula
2026-04-29 10:24 ` [PATCH 14/15] drm/i915/irq: add intel_display_irq_ack() " Jani Nikula
2026-04-29 10:24 ` [PATCH 15/15] drm/i915/irq: add intel_display_irq_handler() " Jani Nikula
2026-04-29 11:56 ` Ville Syrjälä [this message]
2026-04-30 7:59 ` Jani Nikula
2026-04-30 10:28 ` Ville Syrjälä
2026-04-29 11:37 ` ✗ i915.CI.BAT: failure for drm/i915: refactor display funcs, add display irq hooks Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afHx7st1NYZulnQM@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox