[Intel-gfx] [PATCH 5/5] drm/i915: Shuffle fifo underrun disable/enable points for gmch platforms

[Thomas Wood]

On 16 May 2014 17:40,  <ville.syrjala at linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Gen2 reports FIFO underruns whenever no planes are enabled on the pipe.
> So in order to avoid false positives we must enable the FIFO underrun
> reporting only when at least one plane is enabled on the pipe. For
> now just move the underrun reporting enable/disable points to the
> other side of the plane enable/disable point. That doesn't cover cases
> when we turn off all the planes for the pipe but leave the pipe running
> on purpose, but it's better than the current situation.
>
> On gen4+ we can actually move the underrun reporting enable/disable to
> the opposite ends of the crtc enable/disable hooks. I suppose in theory
> we could leave the underrun reporting enabled all the time, except on
> VLV where PIPESTAT stops working when the display power well is down.
> If we ever get around to unifying the PIPESTAT irq handling for all
> gmch platforms, we should still follow the VLV route for other platforms.
> It would also micro-optimize the irq handler a bit since we could then
> skip the PIPESTAT reads for all disabled pipes.
>
> Gen3 is still a mystery, but for now I'm going to assume it behaves
> like gen4+.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>


This doesn't apply to drm-intel-nightly, but looks fine in principle:

Reviewed-by: Thomas Wood <thomas.wood at intel.com>




> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1b5164c..7f61047 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4514,6 +4514,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
>         intel_crtc->active = true;
>
> +       intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> +
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_pll_enable)
>                         encoder->pre_pll_enable(encoder);
> @@ -4537,7 +4539,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>
>         intel_update_watermarks(crtc);
>         intel_enable_pipe(intel_crtc);
> -       intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
> @@ -4564,6 +4565,9 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
>         intel_crtc->active = true;
>
> +       if (!IS_GEN2(dev))
> +               intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> +
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_enable)
>                         encoder->pre_enable(encoder);
> @@ -4576,13 +4580,22 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>
>         intel_update_watermarks(crtc);
>         intel_enable_pipe(intel_crtc);
> -       intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->enable(encoder);
>
>         intel_crtc_enable_planes(crtc);
>
> +       /*
> +        * Gen2 reports pipe underruns whenever all planes are disabled.
> +        * So don't enable underrun reporting before at least some planes
> +        * are enabled.
> +        * FIXME: Need to fix the logic to work when we turn off all planes
> +        * but leave the pipe running.
> +        */
> +       if (IS_GEN2(dev))
> +               intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> +
>         drm_vblank_on(dev, pipe);
>
>         /* Underruns don't raise interrupts, so check manually. */
> @@ -4615,12 +4628,20 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>         if (!intel_crtc->active)
>                 return;
>
> +       /*
> +        * Gen2 reports pipe underruns whenever all planes are disabled.
> +        * So diasble underrun reporting before all the planes get disabled.
> +        * FIXME: Need to fix the logic to work when we turn off all planes
> +        * but leave the pipe running.
> +        */
> +       if (IS_GEN2(dev))
> +               intel_set_cpu_fifo_underrun_reporting(dev, pipe, false);
> +
>         intel_crtc_disable_planes(crtc);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 encoder->disable(encoder);
>
> -       intel_set_cpu_fifo_underrun_reporting(dev, pipe, false);
>         intel_disable_pipe(dev_priv, pipe);
>
>         i9xx_pfit_disable(intel_crtc);
> @@ -4638,6 +4659,9 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>                         i9xx_disable_pll(dev_priv, pipe);
>         }
>
> +       if (!IS_GEN2(dev))
> +               intel_set_cpu_fifo_underrun_reporting(dev, pipe, false);
> +
>         intel_crtc->active = false;
>         intel_update_watermarks(crtc);
>
> --
> 1.8.5.5