On Mon, 17 Sep 2012 22:27:21 +0200 Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > This goes back to > > commit c1c7af60892070e4b82ad63bbfb95ae745056de0 > Author: Jesse Barnes <jbarnes at virtuousgeek.org> > Date: Thu Sep 10 15:28:03 2009 -0700 > > drm/i915: force mode set at lid open time > > It was used to fix an issue on a i915GM based Thinkpad X41, which > somehow clobbered the modeset state at lid close time. Since then > massive amounts of things changed: Tons of fixes to the modeset > sequence, OpRegion support, better integration with the acpi code. > Especially OpRegion /should/ allow us to control the display hw > cooperatively with the firmware, without the firmware clobbering the > hw state behind our backs. > > So it's dubious whether we still need this. > > The second issue is that it's unclear who's responsibility it actually > is to restore the mode - Chris Wilson suggests to just emit a hotplug > event and let userspace figure things out. > > The real reason I've stumbled over this is that the new modeset code > breaks drm_helper_resume_force_mode - it OOPSes derefing a NULL vfunc > pointer. The reason this wasn't caught in testing earlier is that in > > commit c9354c85c1c7bac788ce57d3c17f2016c1c45b1d > Author: Linus Torvalds <torvalds at linux-foundation.org> > Date: Mon Nov 2 09:29:55 2009 -0800 > > i915: fix intel graphics suspend breakage due to resume/lid event > confusion > > logic was added to _not_ restore the modeset state after a resume. And > since most machines are configured to auto-suspend on lid-close, this > neatly papered over the issue. > > Summarizing, this shouldn't be required on any platform supporting > OpRegion. And none of the really old machines I have here seem to > require it either. Hence I'm inclined to just rip it out. > > But in case that there are really firmwares out there that clobber the > hw state, replace it with a call to intel_modset_check_state. This > will ensure that we catch any issues as soon as they happen. > > Cc: Chris Wilson <chris at chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > drivers/gpu/drm/i915/intel_lvds.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index b1b36de..4e22d46 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -581,7 +581,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val, > dev_priv->modeset_on_lid = 0; > > mutex_lock(&dev->mode_config.mutex); > - drm_helper_resume_force_mode(dev); > + intel_modeset_check_state(dev); > mutex_unlock(&dev->mode_config.mutex); > > return NOTIFY_OK; Yeah, I *hope* we don't need this anymore. Needs some good tested-by activity on old machines especially (some of them definitely do mess with display regs across lid events). Acked-by: Jesse Barnes <jbarnes at virtuousgeek.org> -- Jesse Barnes, Intel Open Source Technology Center
