Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
