On Wed, Sep 5, 2012 at 8:31 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > On Wed, 29 Aug 2012 23:13:29 +0200 > Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > >> Since this only calls crtc helper functions, of which a shocking >> amount are NULL. >> >> Now the curious thing is how the new modeset code worked with this >> function call still present: >> >> Thanks to the hw state readout and the suspend fixes to properly >> quiescent the register state, nothing is actually enabled at resume >> (if the bios doesn't set up anything). Which means resume_force_mode >> doesn't actually do anything and hence nothing blows up at resume >> time. >> >> The other reason things do work is that the fbcon layer has it's own >> resume notifier callback, which restores the mode. And thanks to the >> force vt switch at suspend/resume, that then forces X to restore it's >> own mode. >> >> Hence everything still worked (as long as the bios doesn't enable >> anything). And we can just kill the call to resume_force_mode. >> >> The upside of both this patch and the preceeding patch to quiescent >> the modeset state is that our resume path is much simpler: >> - We now longer restore bogus register values (which most often would >> enable the backlight a bit and a few ports), causing flickering. >> - We now longer call resume_force_mode to restore a mode that the >> fbcon layer would overwrite right away anyway. >> >> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index fe7512a..cd6697c 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -549,11 +549,6 @@ static int i915_drm_thaw(struct drm_device *dev) >> intel_modeset_setup_hw_state(dev); >> drm_mode_config_reset(dev); >> drm_irq_install(dev); >> - >> - /* Resume the modeset for every activated CRTC */ >> - mutex_lock(&dev->mode_config.mutex); >> - drm_helper_resume_force_mode(dev); >> - mutex_unlock(&dev->mode_config.mutex); >> } >> >> intel_opregion_init(dev); > > Wouldn't the fb layer's modeset end up being a no-op if the suspended > mode was the same as the fb mode (often the case)? Or at the very > least just a flip rather than a full mode set. I guess most of the flicker was because the register restoring restored a bunch of crap (since the old modeset state wasn't properly cleared before suspending). > Though we do need to deal with non-fb, non-X resumes as well. kmscon > and wayland will expect to be restored at resume time even if CONFIG_VT > and the fb layer aren't compiled into the kernel. Tbh I was rather surprised that when I've noticed this little issue here the restore still worked - until I've noticed by looking at the logs that both the fbcon and the X server restore their modes. I'm not sure what exactly we should do here, since even with the current code the concept of a controlling node isn't really defined in the kms interface (fbcon uses a bunch of funky checks to ensure it doesn't clobber the output state of someone else). But for now (with fbcon pretty much being non-optional) things keep on working, and afaict actually work a bit better overall. -Daniel -- Daniel Vetter daniel.vetter at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch
