Re: [PATCH 3/4] drm/i915: remove intel_uncore_check_errors()

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Paulo Zanoni <paulo.r.zanoni@intel.com> writes:

> I added this code on 8664281b64c457705db72fc60143d03827e75ca9, on
> April 12 2013. Back then, we only had support for detecting unclaimed
> registers on I915_WRITE operations, and we didn't have the
> i915.mmio_debug infrastructure.
>
> I tried to remember exactly why I added this code, and the only reason
> I can think is: to help debugging. With this code, we would be able to
> differentiate between the "your interrupt handler did something wrong"
> and the "something bad happened before the interrupt handler" cases,
> at the cost of one extra register read operation per interrupt.
>
> Since then, we added unclaimed register checking support for
> I915_READ, we added the i915.mmio_debug infrastructure and we also
> fixed most (all?) of the unclaimed register problems on HSW. Due
> to this, I don't think the extra register read at every interrupt is
> necesary anymore: we're probably good in terms of debugging.
>
> So let's kill this function in order to make sure it completely
> disappears from perf.
>
> Notice that this only affects HSW.
>

As you have already posted patches for enabling unclaimed
detection for skl, are you sure we want to remove this
completely from interrupt path? I would think this is useful
feature to have on platform enabling.

How about we just bail out on intel_uncore_check_errors if
mmio_debug register is zero?

-Mika


> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  1 -
>  drivers/gpu/drm/i915/i915_irq.c     |  4 ----
>  drivers/gpu/drm/i915/intel_uncore.c | 11 -----------
>  3 files changed, 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1287007..194e864 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2703,7 +2703,6 @@ extern void intel_uncore_sanitize(struct drm_device *dev);
>  extern void intel_uncore_early_sanitize(struct drm_device *dev,
>  					bool restore_forcewake);
>  extern void intel_uncore_init(struct drm_device *dev);
> -extern void intel_uncore_check_errors(struct drm_device *dev);
>  extern void intel_uncore_fini(struct drm_device *dev);
>  extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore);
>  const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2063279..57ec55e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2064,10 +2064,6 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
>  	if (!intel_irqs_enabled(dev_priv))
>  		return IRQ_NONE;
>  
> -	/* We get interrupts on unclaimed registers, so check for this before we
> -	 * do any I915_{READ,WRITE}. */
> -	intel_uncore_check_errors(dev);
> -
>  	/* disable master interrupt before clearing iir  */
>  	de_ier = I915_READ(DEIER);
>  	I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 65e0ea8..8844c314 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1554,14 +1554,3 @@ bool intel_has_gpu_reset(struct drm_device *dev)
>  {
>  	return intel_get_gpu_reset(dev) != NULL;
>  }
> -
> -void intel_uncore_check_errors(struct drm_device *dev)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -	if (HAS_FPGA_DBG_UNCLAIMED(dev) &&
> -	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
> -		DRM_ERROR("Unclaimed register before interrupt\n");
> -		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
> -	}
> -}
> -- 
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx