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

[Daniel Vetter]

On Thu, May 22, 2014 at 04:25:05PM -0300, Paulo Zanoni wrote:
> 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.

Oops, that patch here shouldn't be in this series. Somehow I've screwed up
my branch management, please ignore.
-Daniel

> 
> > ---
> >  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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch