Re: [Intel-gfx] [PATCH 1/3] drm/i915: Warn when display irq functions are called without display

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Mar 22, 2021 at 11:10:53AM -0700, José Roberto de Souza wrote:
> With previous changes none of those warnings will be printed but let's
> add them so CI can caught regressions.
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_fifo_underrun.c  |  2 ++
>  drivers/gpu/drm/i915/display/intel_hotplug.c    |  2 ++
>  drivers/gpu/drm/i915/i915_irq.c                 | 17 +++++++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> index 813a4f7033e1..f3631e319e5d 100644
> --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> @@ -373,6 +373,8 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +

Sprinkling these everywhere isn't great. Would be better to stick them
into just the top level display irq handlers.

>  	/* We may be called too early in init, thanks BIOS! */
>  	if (crtc == NULL)
>  		return;
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index f46a1b7190b8..77ce4a54a137 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -478,6 +478,8 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  	if (!pin_mask)
>  		return;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	spin_lock(&dev_priv->irq_lock);
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 44aed4cbf894..cbb2aae4fc13 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -519,6 +519,8 @@ void i915_enable_pipestat(struct drm_i915_private *dev_priv,
>  	i915_reg_t reg = PIPESTAT(pipe);
>  	u32 enable_mask;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +

Not much point in adding these to any gmch code. It's never
going to trip.

>  	drm_WARN_ONCE(&dev_priv->drm, status_mask & ~PIPESTAT_INT_STATUS_MASK,
>  		      "pipe %c: status_mask=0x%x\n",
>  		      pipe_name(pipe), status_mask);
> @@ -1273,6 +1275,7 @@ static u32 intel_hpd_hotplug_enables(struct drm_i915_private *i915,
>  
>  static void gmbus_irq_handler(struct drm_i915_private *dev_priv)
>  {
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
>  	wake_up_all(&dev_priv->gmbus_wait_queue);
>  }
>  
> @@ -1366,6 +1369,8 @@ static void i9xx_pipe_crc_irq_handler(struct drm_i915_private *dev_priv,
>  {
>  	u32 res1, res2;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (INTEL_GEN(dev_priv) >= 3)
>  		res1 = intel_uncore_read(&dev_priv->uncore, PIPE_CRC_RES_RES1_I915(pipe));
>  	else
> @@ -1558,6 +1563,8 @@ static u32 i9xx_hpd_irq_ack(struct drm_i915_private *dev_priv)
>  	u32 hotplug_status = 0, hotplug_status_mask;
>  	int i;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (IS_G4X(dev_priv) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		hotplug_status_mask = HOTPLUG_INT_STATUS_G4X |
> @@ -1597,6 +1604,8 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  	u32 pin_mask = 0, long_mask = 0;
>  	u32 hotplug_trigger;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (IS_G4X(dev_priv) ||
>  	    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  		hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> @@ -2038,6 +2047,8 @@ static void ilk_display_irq_handler(struct drm_i915_private *dev_priv,
>  	enum pipe pipe;
>  	u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (hotplug_trigger)
>  		ilk_hpd_irq_handler(dev_priv, hotplug_trigger);
>  
> @@ -2087,6 +2098,8 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
>  	enum pipe pipe;
>  	u32 hotplug_trigger = de_iir & DE_DP_A_HOTPLUG_IVB;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (hotplug_trigger)
>  		ilk_hpd_irq_handler(dev_priv, hotplug_trigger);
>  
> @@ -2421,6 +2434,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>  	u32 iir;
>  	enum pipe pipe;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (master_ctl & GEN8_DE_MISC_IRQ) {
>  		iir = intel_uncore_read(&dev_priv->uncore, GEN8_DE_MISC_IIR);
>  		if (iir) {
> @@ -3477,6 +3492,8 @@ static void spt_hpd_irq_setup(struct drm_i915_private *dev_priv)
>  {
>  	u32 hotplug_irqs, enabled_irqs;
>  
> +	drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_DISPLAY(dev_priv));
> +
>  	if (INTEL_PCH_TYPE(dev_priv) >= PCH_CNP)
>  		intel_uncore_write(&dev_priv->uncore, SHPD_FILTER_CNT, SHPD_FILTER_CNT_500_ADJ);
>  
> -- 
> 2.31.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx