[Intel-gfx] [PATCH 43/66] drm/i915: Disable pipe before ports on ilk

[Paulo Zanoni]

2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> The modeset sequence docs are very clear that we should disable the
> pipe before we switch off any ports, for both pch ports and the cpu
> edp port.
>
> In practice it doesn't seem to matter too much since for non-DP pch
> ports it only matters that the pch transcoder is still on. And for cpu
> edp ports it either doesn't seem to matter or we're quick enough.
>
> But for DP pch ports we have a regular stream of bug reports where the
> cpu pipe seems to be stuck and won't turn off. This change should
> address this.
>
> This should also help with using a nuclear pageflip atomically switch
> off all planes, since it moves that ahead of any other disabling
> action.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=62251
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=52061
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54687
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67462
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

This patch doesn't make us spec-compliant either.

For example, the spec says we need to disable audio, backlight and the
panel power *before* we disable the pipe. This is done by
intel_disable_dp on IVB, which is, today, at the correct order. After
this patch, we will be doing all this *after* we disable the pipe,
which is not what the spec says we should do.

Also, we have ->disable and ->post_disable for a reason, and it kinda
looks like you're making ILK's ->disable do what ->post_disable should
be doing. It seems what we should do if we want to be correct is to
mode stuff from ->disable to ->post_disable.

> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 713563faeafd..82ad84eefc8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3924,14 +3924,14 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>
>         ilk_crtc_disable_planes(crtc);
>
> -       for_each_encoder_on_crtc(dev, crtc, encoder)
> -               encoder->disable(encoder);
> -
>         if (intel_crtc->config.has_pch_encoder)
>                 intel_set_pch_fifo_underrun_reporting(dev, pipe, false);
>
>         intel_disable_pipe(dev_priv, pipe);
>
> +       for_each_encoder_on_crtc(dev, crtc, encoder)
> +               encoder->disable(encoder);
> +
>         ironlake_pfit_disable(intel_crtc);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni