All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.