[PATCH v2] drm/client: Detect when ACPI lid is closed during initialization
Mario Limonciello
mario.limonciello at amd.com
Wed May 29 16:26:58 UTC 2024
>>>
>>> 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.
More information about the amd-gfx
mailing list