On Fri, Mar 6, 2020 at 5:38 PM Rajat Jain rajatja@google.com wrote:
On Fri, Mar 6, 2020 at 1:42 AM Jani Nikula jani.nikula@linux.intel.com wrote:
On Thu, 05 Mar 2020, Rajat Jain rajatja@google.com wrote:
On Thu, Mar 5, 2020 at 1:41 AM Jani Nikula jani.nikula@linux.intel.com wrote:
On Wed, 04 Mar 2020, Rajat Jain rajatja@google.com wrote:
- See if we can postpone creating and attaching properties to connector
->late_register hook. (I didn't have the time to look into it yet, at all.)
Apparently not. The drm core doesn't like to add properties in late_register() callback. I just tried it and get this warning:
I kind of had a feeling this would be the case, thanks for checking.
Thinking about it again, it looks like there is a difference in creating a property and attaching a property. I'm wondering if drm would let me (unconditionally) create a property before registering, and attach it in late_register() only in case a privacy screen is detected. (If not present, I can destroy the property in late_register()). If this approach sound more promising, I can try it out.
I tried out this approach, and it worked. The new patchset is posted at:
https://patchwork.freedesktop.org/series/74473/#rev1
Thanks!
Rajat
- Provide a way to populate connector->acpi_device_id and
connector->acpi_handle on a per-connector basis. At least the device id remains constant for the lifetime of the drm_device
Are you confirming that the connector->acpi_device_id remains constant for the lifetime of the drm_device, as calculated in intel_acpi_device_id_update()? Even in the face of external displays (monitors) being connected and disconnected during the lifetime of the system? If so, then I think we can have a solution.
First I thought so. Alas it does not hold for DP MST, where you can have connectors added and removed dynamically. I think we could ensure they stay the same for all other connectors though. I'm pretty sure this is already the case; they get added/removed after all others.
Another thought, from the ACPI perspective, I'm not sure the dynamically added/removed DP MST connectors should even have acpi handles. But again, tying all this together with ACPI stuff is not something I am an expert on.
I propose that we:
- Maintain a display_index[] array within the drm_dev, and increment
as connectors are added. 2) Initialize connector->acpi_device_id and and connector->acpi_handle while registering (one time per connector). 3) Remove the code to update acpi_device_id on every resume.
It doesn't look like anyone on the DP MST side has cared for ACPI so far, so I doubt if we can do anything that might break MST currently. In other words, the above should not make things any worse for MST, if not better. For connectors other than MST, this should allow them to get ACPI handle and play with it, if they need.
WDYT?
Thanks,
Rajat
(why do we keep updating it at every resume?!) but can we be sure ->acpi_handle does too? (I don't really know my way around ACPI.)
I don't understand why this was being updated on every resume in that case (this existed even before my patchset). I believe we do not need it. Yes, the ->acpi_handle will not change if the ->acpi_device_id does not change. I believe the way forward should then be to populate connector->acpi_device_id and connector->acpi_handle ONE TIME at the time of connector init (and not update it on every resume). Does this sound ok?
If a DP MST connector gets removed, should the other ACPI display indexes after that shift, or remain the same? I really don't know.
BR, Jani.
-- Jani Nikula, Intel Open Source Graphics Center