[Intel-gfx] [PATCH 13/24] drm/i915: kill lpt pch transcoder->crtc mapping code for fifo underruns

[Paulo Zanoni]

2013/6/12 Daniel Vetter <daniel.vetter at ffwll.ch>:
> It's racy: There's no guarantee that we won't walk this code (due to a
> pch fifo underrun interrupt) while someone is changing the pointers
> around.
>
> The only reason we do this is to use the righ crtc for the pch fifo
> underrun accounting. But we never expose this to userspace, so
> essentially no one really cares if we use the "wrong" crtc.
>
> So let's just rip it out.
>
> With this patch fifo underrun code will always use crtc A for tracking
> underruns on the (only) pch transcoder on LPT.

Please add a big comment in the source code explaining the hack and its reason.

>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 33 +++++++--------------------------
>  1 file changed, 7 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bb26555..b98ea4e 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -193,13 +193,13 @@ static void ibx_display_interrupt_update(struct drm_i915_private *dev_priv,
>         POSTING_READ(SDEIMR);
>  }
>
> -static void ibx_set_fifo_underrun_reporting(struct intel_crtc *crtc,
> +static void ibx_set_fifo_underrun_reporting(struct drm_device *dev,
> +                                           enum transcoder pch_transcoder,
>                                             bool enable)
>  {
> -       struct drm_device *dev = crtc->base.dev;
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       uint32_t bit = (crtc->pipe == PIPE_A) ? SDE_TRANSA_FIFO_UNDER :
> -                                               SDE_TRANSB_FIFO_UNDER;
> +       uint32_t bit = (pch_transcoder == TRANSCODER_A) ?
> +                      SDE_TRANSA_FIFO_UNDER : SDE_TRANSB_FIFO_UNDER;
>
>         ibx_display_interrupt_update(dev_priv, bit,
>                                      enable ? bit : 0);
> @@ -292,30 +292,11 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>                                            bool enable)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> -       enum pipe p;
> -       struct drm_crtc *crtc;
> -       struct intel_crtc *intel_crtc;
> +       struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
> +       struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>         unsigned long flags;
>         bool ret;
>
> -       if (HAS_PCH_LPT(dev)) {
> -               crtc = NULL;
> -               for_each_pipe(p) {
> -                       struct drm_crtc *c = dev_priv->pipe_to_crtc_mapping[p];
> -                       if (intel_pipe_has_type(c, INTEL_OUTPUT_ANALOG)) {
> -                               crtc = c;
> -                               break;
> -                       }
> -               }
> -               if (!crtc) {
> -                       DRM_ERROR("PCH FIFO underrun, but no CRTC using the PCH found\n");
> -                       return false;
> -               }
> -       } else {
> -               crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
> -       }
> -       intel_crtc = to_intel_crtc(crtc);
> -
>         spin_lock_irqsave(&dev_priv->irq_lock, flags);
>
>         ret = !intel_crtc->pch_fifo_underrun_disabled;
> @@ -326,7 +307,7 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>         intel_crtc->pch_fifo_underrun_disabled = !enable;
>
>         if (HAS_PCH_IBX(dev))
> -               ibx_set_fifo_underrun_reporting(intel_crtc, enable);
> +               ibx_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
>         else
>                 cpt_set_fifo_underrun_reporting(dev, pch_transcoder, enable);
>
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni