2013/7/10 Daniel Vetter <daniel.vetter at ffwll.ch>:
> The current code won't report any fifo underruns on cpt if just one
> pipe has fifo underrun reporting disabled. We can't enable the
> interrupts, but we can still check the per-transcoder bits and so
> report the underrun delayed if:
> - We always clear the transcoder's bit (and none of the other bits)
> when enabling.
> - We check the transcoder's bit after disabling (to avoid racing with
> the interrupt handler).
>
> v2: I've forgotten to actually remove the old SERR_INT clearing.
>
> v3: Use transcoder_name as suggested by Paulo Zanoni. Paulo also
> noticed a logic bug: When an underrun interrupt fires we report it
> both in the interrupt handler and when checking for underruns when
> disabling it in cpt_set_fifo_underrun_reporting. But that second check
> is only required if the interrupt is disabled and we're switching of
> underrun reporting (e.g. because we're disabling the crtc). Hence
> check for that condition.
>
> At first I wanted to rework the code to pass that bit of information
> from the uppper functions down to cpt_set_fifo_underrun_reporting. But
> that turned out too messy. Hence the quick&dirty check whether the
> south error interrupt source is masked off or not.
>
> v4: Streamline the control flow a bit.
>
> v5: s/pipe/pch transcoder/ in the dmesg output, suggested by Paulo.
>
> v6: Review from Paulo:
> - Reorder the was_enabled assignment to only read the register when we
> need it. Also add a comment that we need to do that before updating
> the register.
> - s/%i/%c/ fix for the debug output.
> - Fix the checkpath complaint in the SERR_INT_TRANS_FIFO_UNDERRUN
> #define.
>
> v7: Hopefully put that elusive SERR hunk back into this patch, spotted
> by Paulo.
>
> Cc: Paulo Zanoni <przanoni at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++++++----
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 80b88c8..dd9d999 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -217,16 +217,25 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> if (enable) {
> + I915_WRITE(SERR_INT,
> + SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
> +
> if (!cpt_can_enable_serr_int(dev))
> return;
>
> - I915_WRITE(SERR_INT, SERR_INT_TRANS_A_FIFO_UNDERRUN |
> - SERR_INT_TRANS_B_FIFO_UNDERRUN |
> - SERR_INT_TRANS_C_FIFO_UNDERRUN);
> -
> ibx_enable_display_interrupt(dev_priv, SDE_ERROR_CPT);
> } else {
> + uint32_t tmp = I915_READ(SERR_INT);
> + bool was_enabled = !(I915_READ(SDEIMR) & SDE_ERROR_CPT);
> +
> + /* Change the state _after_ we've read out the current one. */
> ibx_disable_display_interrupt(dev_priv, SDE_ERROR_CPT);
> +
> + if (!was_enabled &&
> + (tmp & SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder))) {
> + DRM_DEBUG_KMS("uncleared pch fifo underrun on pch transcoder %c\n",
> + transcoder_name(pch_transcoder));
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e9c50fa..7e2684f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3882,6 +3882,7 @@
> #define SERR_INT_TRANS_C_FIFO_UNDERRUN (1<<6)
> #define SERR_INT_TRANS_B_FIFO_UNDERRUN (1<<3)
> #define SERR_INT_TRANS_A_FIFO_UNDERRUN (1<<0)
> +#define SERR_INT_TRANS_FIFO_UNDERRUN(pipe) (1<<(pipe*3))
>
> /* digital port hotplug */
> #define PCH_PORT_HOTPLUG 0xc4030 /* SHOTPLUG_CTL */
> --
> 1.8.1.4
>
--
Paulo Zanoni
