Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
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 v3 3/6] drm/i915/irq: add display irq funcs, start with intel_display_irq_reset()
Date: Wed, 13 May 2026 16:56:08 +0300	[thread overview]
Message-ID: <agSC-HPVDeD_GnrM@intel.com> (raw)
In-Reply-To: <8cf49ccdb19c7263ac832714577d1a5cf2e20f00.1778666967.git.jani.nikula@intel.com>

On Wed, May 13, 2026 at 01:10:08PM +0300, Jani Nikula wrote:
> Introduce display irq hooks with struct intel_display_irq_funcs, and add
> the ->reset hook as the first thing. Call the reset hooks from i915 and
> xe core via intel_display_irq_reset().
> 
> Relocate the gen8 and gen11 HAS_DISPLAY() check to
> intel_display_irq_reset(), as the funcs pointer won't be initialized for
> no display.
> 
> Note: We're increasingly moving to the territory of not touching display
> at all if there's no display or it has been fused off. Which is good,
> but care must be taken to not have hardware setup required also for no
> display cases in display code. Also note that the line is fuzzy for
> older platforms, but there we also don't have fusing.
> 
> v2:
> - make the structs static const (Sashiko)
> - relocate HAS_DISPLAY() (Sashiko)
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  .../gpu/drm/i915/display/intel_display_core.h |  3 +
>  .../gpu/drm/i915/display/intel_display_irq.c  | 61 +++++++++++++++----
>  .../gpu/drm/i915/display/intel_display_irq.h  |  6 +-
>  drivers/gpu/drm/i915/i915_irq.c               | 16 ++---
>  drivers/gpu/drm/xe/display/xe_display.c       |  2 +-
>  5 files changed, 63 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 76745ce6a716..3dc5ac75a98b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -475,6 +475,9 @@ struct intel_display {
>  	} ips;
>  
>  	struct {
> +		/* internal display irq functions */
> +		const struct intel_display_irq_funcs *funcs;
> +
>  		/* protects the irq masks */
>  		spinlock_t lock;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
> index d30b063714b0..62a849673454 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
> @@ -1947,7 +1947,7 @@ static void _vlv_display_irq_reset(struct intel_display *display)
>  	display->irq.vlv_imr_mask = ~0u;
>  }
>  
> -void vlv_display_irq_reset(struct intel_display *display)
> +static void vlv_display_irq_reset(struct intel_display *display)
>  {
>  	spin_lock_irq(&display->irq.lock);
>  	if (display->irq.vlv_display_irqs_enabled)
> @@ -1955,7 +1955,7 @@ void vlv_display_irq_reset(struct intel_display *display)
>  	spin_unlock_irq(&display->irq.lock);
>  }
>  
> -void i9xx_display_irq_reset(struct intel_display *display)
> +static void i9xx_display_irq_reset(struct intel_display *display)
>  {
>  	if (HAS_HOTPLUG(display)) {
>  		i915_hotplug_interrupt_update(display, 0xffffffff, 0);
> @@ -2076,7 +2076,7 @@ static void ibx_display_irq_reset(struct intel_display *display)
>  		intel_de_write(display, SERR_INT, 0xffffffff);
>  }
>  
> -void ilk_display_irq_reset(struct intel_display *display)
> +static void ilk_display_irq_reset(struct intel_display *display)
>  {
>  	irq_reset(display, DE_IRQ_REGS);
>  	display->irq.ilk_de_imr_mask = ~0u;
> @@ -2092,13 +2092,10 @@ void ilk_display_irq_reset(struct intel_display *display)
>  	ibx_display_irq_reset(display);
>  }
>  
> -void gen8_display_irq_reset(struct intel_display *display)
> +static void gen8_display_irq_reset(struct intel_display *display)
>  {
>  	enum pipe pipe;
>  
> -	if (!HAS_DISPLAY(display))
> -		return;
> -
>  	intel_de_write(display, EDP_PSR_IMR, 0xffffffff);
>  	intel_de_write(display, EDP_PSR_IIR, 0xffffffff);
>  
> @@ -2114,15 +2111,12 @@ void gen8_display_irq_reset(struct intel_display *display)
>  		ibx_display_irq_reset(display);
>  }
>  
> -void gen11_display_irq_reset(struct intel_display *display)
> +static void gen11_display_irq_reset(struct intel_display *display)
>  {
>  	enum pipe pipe;
>  	u32 trans_mask = BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
>  		BIT(TRANSCODER_C) | BIT(TRANSCODER_D);
>  
> -	if (!HAS_DISPLAY(display))
> -		return;
> -
>  	intel_de_write(display, GEN11_DISPLAY_INT_CTL, 0);
>  
>  	if (DISPLAY_VER(display) >= 12) {
> @@ -2453,6 +2447,38 @@ struct intel_display_irq_funcs {
>  	void (*reset)(struct intel_display *display);
>  };
>  
> +static const struct intel_display_irq_funcs gen11_display_irq_funcs = {
> +	.reset = gen11_display_irq_reset,
> +};
> +
> +static const struct intel_display_irq_funcs gen8_display_irq_funcs = {
> +	.reset = gen8_display_irq_reset,
> +};
> +
> +static const struct intel_display_irq_funcs vlv_display_irq_funcs = {
> +	.reset = vlv_display_irq_reset,
> +};
> +
> +static const struct intel_display_irq_funcs ilk_display_irq_funcs = {
> +	.reset = ilk_display_irq_reset,
> +};
> +
> +static const struct intel_display_irq_funcs i965_display_irq_funcs = {
> +	.reset = i9xx_display_irq_reset,
> +};
> +
> +static const struct intel_display_irq_funcs i915_display_irq_funcs = {
> +	.reset = i9xx_display_irq_reset,
> +};
> +
> +void intel_display_irq_reset(struct intel_display *display)
> +{
> +	if (!HAS_DISPLAY(display))
> +		return;
> +
> +	display->irq.funcs->reset(display);
> +}
> +
>  void intel_display_irq_init(struct intel_display *display)
>  {
>  	spin_lock_init(&display->irq.lock);
> @@ -2463,6 +2489,19 @@ void intel_display_irq_init(struct intel_display *display)
>  
>  	INIT_WORK(&display->irq.vblank_notify_work,
>  		  intel_display_vblank_notify_work);
> +
> +	if (DISPLAY_VER(display) >= 11)
> +		display->irq.funcs = &gen11_display_irq_funcs;
> +	else if (display->platform.cherryview || display->platform.valleyview)
> +		display->irq.funcs = &vlv_display_irq_funcs;
> +	else if (DISPLAY_VER(display) >= 8)
> +		display->irq.funcs = &gen8_display_irq_funcs;
> +	else if (DISPLAY_VER(display) >= 5)
> +		display->irq.funcs = &ilk_display_irq_funcs;
> +	else if (DISPLAY_VER(display) == 4)
> +		display->irq.funcs = &i965_display_irq_funcs;
> +	else
> +		display->irq.funcs = &i915_display_irq_funcs;
>  }
>  
>  struct intel_display_irq_snapshot {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.h b/drivers/gpu/drm/i915/display/intel_display_irq.h
> index d25b9ea4272b..21b2145656cd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_irq.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_irq.h
> @@ -58,11 +58,7 @@ 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);
>  
> -void i9xx_display_irq_reset(struct intel_display *display);
> -void ilk_display_irq_reset(struct intel_display *display);
> -void vlv_display_irq_reset(struct intel_display *display);
> -void gen8_display_irq_reset(struct intel_display *display);
> -void gen11_display_irq_reset(struct intel_display *display);
> +void intel_display_irq_reset(struct intel_display *display);
>  
>  u32 i9xx_display_irq_enable_mask(struct intel_display *display);
>  void i915_display_irq_postinstall(struct intel_display *display);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index ef9eadf38a53..c4f56a869910 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -640,7 +640,7 @@ static void ilk_irq_reset(struct drm_i915_private *dev_priv)
>  	struct intel_display *display = dev_priv->display;
>  
>  	/* The master interrupt enable is in DEIER, reset display irq first */
> -	ilk_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  	gen5_gt_irq_reset(to_gt(dev_priv));
>  }
>  
> @@ -653,7 +653,7 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
>  
>  	gen5_gt_irq_reset(to_gt(dev_priv));
>  
> -	vlv_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  }
>  
>  static void gen8_irq_reset(struct drm_i915_private *dev_priv)
> @@ -664,7 +664,7 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv)
>  	gen8_master_intr_disable(intel_uncore_regs(uncore));
>  
>  	gen8_gt_irq_reset(to_gt(dev_priv));
> -	gen8_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
>  }
>  
> @@ -677,7 +677,7 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
>  	gen11_master_intr_disable(intel_uncore_regs(&dev_priv->uncore));
>  
>  	gen11_gt_irq_reset(gt);
> -	gen11_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  
>  	gen2_irq_reset(uncore, GEN11_GU_MISC_IRQ_REGS);
>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
> @@ -695,7 +695,7 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
>  	for_each_gt(gt, dev_priv, i)
>  		gen11_gt_irq_reset(gt);
>  
> -	gen11_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  
>  	gen2_irq_reset(uncore, GEN11_GU_MISC_IRQ_REGS);
>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
> @@ -715,7 +715,7 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
>  
>  	gen2_irq_reset(uncore, GEN8_PCU_IRQ_REGS);
>  
> -	vlv_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  }
>  
>  static void ilk_irq_postinstall(struct drm_i915_private *dev_priv)
> @@ -864,7 +864,7 @@ static void i915_irq_reset(struct drm_i915_private *dev_priv)
>  	struct intel_display *display = dev_priv->display;
>  	struct intel_uncore *uncore = &dev_priv->uncore;
>  
> -	i9xx_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  
>  	gen2_error_reset(uncore, GEN2_ERROR_REGS);
>  	gen2_irq_reset(uncore, GEN2_IRQ_REGS);
> @@ -951,7 +951,7 @@ static void i965_irq_reset(struct drm_i915_private *dev_priv)
>  	struct intel_display *display = dev_priv->display;
>  	struct intel_uncore *uncore = &dev_priv->uncore;
>  
> -	i9xx_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  
>  	gen2_error_reset(uncore, GEN2_ERROR_REGS);
>  	gen2_irq_reset(uncore, GEN2_IRQ_REGS);
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index aa73023b7398..ba3225878c61 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -236,7 +236,7 @@ void xe_display_irq_reset(struct xe_device *xe)
>  	if (!xe->info.probe_display)
>  		return;
>  
> -	gen11_display_irq_reset(display);
> +	intel_display_irq_reset(display);
>  }
>  
>  void xe_display_irq_postinstall(struct xe_device *xe)
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2026-05-13 13:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 10:10 [PATCH v3 0/6] drm/i915: add display irq hooks Jani Nikula
2026-05-13 10:10 ` [PATCH v3 1/6] drm/i915/irq: deduplicate dg1_de_irq_postinstall() and gen11_de_irq_postinstall() Jani Nikula
2026-05-13 13:53   ` Ville Syrjälä
2026-05-13 10:10 ` [PATCH v3 2/6] drm/i915/irq: constify pipe stats parameters Jani Nikula
2026-05-13 13:54   ` Ville Syrjälä
2026-05-13 10:10 ` [PATCH v3 3/6] drm/i915/irq: add display irq funcs, start with intel_display_irq_reset() Jani Nikula
2026-05-13 13:56   ` Ville Syrjälä [this message]
2026-05-13 10:10 ` [PATCH v3 4/6] drm/i915/irq: add intel_display_irq_postinstall() to irq funcs Jani Nikula
2026-05-13 13:57   ` Ville Syrjälä
2026-05-13 10:10 ` [PATCH v3 5/6] drm/i915/irq: add intel_display_irq_ack() " Jani Nikula
2026-05-13 14:30   ` Ville Syrjälä
2026-05-13 16:15     ` Jani Nikula
2026-05-13 10:10 ` [PATCH v3 6/6] drm/i915/irq: add intel_display_irq_handler() " Jani Nikula
2026-05-13 12:38   ` Ville Syrjälä
2026-05-13 12:22 ` ✗ i915.CI.BAT: failure for drm/i915: add display irq hooks (rev2) Patchwork
2026-05-13 12:22 ` Patchwork
2026-05-13 12:22 ` Patchwork
2026-05-13 12:22 ` 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=agSC-HPVDeD_GnrM@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