On Mon, Jan 28, 2013 at 3:36 AM, Zhang Rui <rui.zhang at intel.com> wrote: >> Given that this essentially requires users to manually set this module >> option to make stuff work I don't like this. >> > sorry, I do not understand. > this patch just disables modeset_on_lid during suspend. Pardon me, the misunderstanding is on my side - I've mixed up dev_priv->modeset_on_lid with the corresponding module option. Is my understanding correct that only with the new SCI pm state this can happen, since that still allows acpi events to be processed (and so our lid_notifier being called? >> I see a few possible options: >> - plug the races between all the different parts - I've never really >> understood why acpi sends us events before the resume code has >> completed ... If that's indeed intentional, we could delay the >> handling a bit until at least the i915 resume code completed. > > IMO, the real question is that, during a suspend/resume cycle, if we > need to take care of the lid open event or not. > To me, the answer is no, because when resuming from STR, i915 is not > aware of such an event, and the i915 resume code works well, right? > so I do not think it is a problem to ignore the lid event for another > lightweight suspend state, which is introduced in Patch 1/3 in this > series. > >> - Judging from the diff context you're not on the latest 3.8-rc >> codebase - we've applied a few fixes to this lid hack recently. Please >> retest on a kernel with >> > the patch is based on 3.8.0-rc3, which already includes the commit > below. > And yes, the problem still exists. Ok, I think I see the issue now. But I suspect that we need some additional locking to make this solid, since currently dev_priv->modeset_on_lid updates can race with our lid notifier handler. Let me think about this a bit more. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
