2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>: > 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@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx