On Mon, 17 Sep 2012 14:38:14 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > 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). Not even a squeak on my 855gm. On a positive note modeset-rework is able to light the LVDS even when that machine is booted with its lid close, which is a first! It complains vehemently in the process though. > Acked-by: Jesse Barnes <jbarnes at virtuousgeek.org> Tested-by: Chris Wilson <chris at chris-wilson.co.uk> # 855gm with an evil BIOS -Chris -- Chris Wilson, Intel Open Source Technology Centre
