Re: [PATCH v2] drm/client: Detect when ACPI lid is closed during initialization

[Mario Limonciello <mario.limonciello@amd.com>]

>>>
>>> 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.