If you don't hook into some lid notify event how is one supposed to get
the display back to life after opening the lid?
I guess in my mind it's a tangential to the "initial modeset". The DRM
master can issue a modeset to enable the combination as desired.
This code is run whenever there's a hotplug/etc. Not sure why you're
only thinking about the initial modeset.
Got it; so in that case adding a notification chain for lid events to
run it again should do the trick.
When I tested I did confirm that with mutter such an event is received
and it does the modeset to enable the eDP when lid is opened.
This code isn't relevant when you have a userspace drm master
calling the shots.
Right.
Let me ask this - what happens if no DRM master running and you hotplug
a DP cable? Does a "new" clone configuration get done?
Yes, this code reprobes the displays and comes up with a new
config to suit the new situation.
Got it; in this case you're right we should have some notification
chain. Do you think it should be in the initial patch or a follow up?
The other potential issue here is whether acpi_lid_open() is actually
trustworthy. Some kms drivers have/had some lid handling in their own
code, and I'm pretty sure those have often needed quirks/modparams
to actually do sensible things on certain machines.
FWIW I ripped out all the lid crap from i915 long ago since it was
half backed, mostly broken, and ugly, and I'm not looking to add it
back there. But I do think handling that in drm_client does seem
somewhat sane, as that should more or less match what userspace
clients would do. Just a question of how bad the quirk situation
will get...
If the lid reporting is wrong it's not just drm_client that would
falter. There are other parts of the kernel that rely upon
acpi_lid_open() being accurate and IMO it would be best to put any
quirks to the effect in drivers/acpi/button.c.
If it can't be relied upon then it's best to just report -EINVAL or -ENODEV.
Also a direct acpi_lid_open() call seems a bit iffy. But I guess if
someone needs this to work on non-ACPI system they get to figure out
how to abstract it better. acpi_lid_open() does seem to return != 0
when ACPI is not supported, so at least it would err on the side
of enabling everything.
Yeah acpi_lid_open() seemed fine to me specifically because non ACPI
hardcodes to open.