| Message ID | 20200305012338.219746-3-rajatja@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,1/3] intel_acpi: Rename drm_dev local variable to dev | expand |
On Wed, 04 Mar 2020, Rajat Jain <rajatja@google.com> wrote: > Lookup and attach ACPI nodes for intel connectors. The lookup is done > in compliance with ACPI Spec 6.3 > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf > (Ref: Pages 1119 - 1123). > > This can be useful for any connector specific platform properties. (This > will be used for privacy screen in next patch). > > Signed-off-by: Rajat Jain <rajatja@google.com> > --- > v6: Addressed minor comments from Jani at > https://lkml.org/lkml/2020/1/24/1143 > - local variable renamed. > - used drm_dbg_kms() > - used acpi_device_handle() > - Used opaque type acpi_handle instead of void* > v5: same as v4 > v4: Same as v3 > v3: fold the code into existing acpi_device_id_update() function > v2: formed by splitting the original patch into ACPI lookup, and privacy > screen property. Also move it into i915 now that I found existing code > in i915 that can be re-used. > > drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++++++++++++++++++ > .../drm/i915/display/intel_display_types.h | 5 ++++ > drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c > index 3e6831cca4ac1..870c1ad98df92 100644 > --- a/drivers/gpu/drm/i915/display/intel_acpi.c > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c > @@ -222,11 +222,22 @@ static u32 acpi_display_type(struct intel_connector *connector) > return display_type; > } > > +/* > + * Ref: ACPI Spec 6.3 > + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf > + * Pages 1119 - 1123 describe, what I believe, a standard way of > + * identifying / addressing "display panels" in the ACPI. It provides > + * a way for the ACPI to define devices for the display panels attached > + * to the system. It thus provides a way for the BIOS to export any panel > + * specific properties to the system via ACPI (like device trees). > + */ > void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) > { > struct drm_device *dev = &dev_priv->drm; > struct intel_connector *connector; > struct drm_connector_list_iter conn_iter; > + struct acpi_device *conn_dev; > + u64 conn_addr; > u8 display_index[16] = {}; > > /* Populate the ACPI IDs for all connectors for a given drm_device */ > @@ -242,6 +253,19 @@ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) > device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT; > > connector->acpi_device_id = device_id; > + > + /* Build the _ADR to look for */ > + conn_addr = device_id | ACPI_DEVICE_ID_SCHEME | > + ACPI_BIOS_CAN_DETECT; > + > + drm_dbg_kms(dev, "Checking connector ACPI node at _ADR=%llX\n", > + conn_addr); > + > + /* Look up the connector device, under the PCI device */ > + conn_dev = acpi_find_child_device( > + ACPI_COMPANION(&dev->pdev->dev), > + conn_addr, false); > + connector->acpi_handle = acpi_device_handle(conn_dev); > } > drm_connector_list_iter_end(&conn_iter); > } > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > index 5e00e611f077f..d70612cc1ba2a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -411,9 +411,14 @@ struct intel_connector { > */ > struct intel_encoder *encoder; > > +#ifdef CONFIG_ACPI > /* ACPI device id for ACPI and driver cooperation */ > u32 acpi_device_id; > > + /* ACPI handle corresponding to this connector display, if found */ > + acpi_handle acpi_handle; > +#endif > + > /* Reads out the current hw, returning true if the connector is enabled > * and active (i.e. dpms ON state). */ > bool (*get_hw_state)(struct intel_connector *); > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 0a417cd2af2bc..171821113d362 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -44,6 +44,7 @@ > #include "i915_debugfs.h" > #include "i915_drv.h" > #include "i915_trace.h" > +#include "intel_acpi.h" > #include "intel_atomic.h" > #include "intel_audio.h" > #include "intel_connector.h" > @@ -6868,6 +6869,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT; > > + /* Lookup the ACPI node corresponding to the connector */ > + intel_acpi_device_id_update(dev_priv); I find this part problematic. Normally, we call the function at probe via i915_driver_register() -> intel_opregion_register() -> intel_opregion_resume() -> intel_didl_outputs() -> intel_acpi_device_id_update(). It gets called *once* at probe, after we have all the outputs (and thus connectors) figured out. This in turn calls it for every DP connector, before we even have all connectors registered. But it also re-iterates the previously handled connectors again and again. The problem, of course, is that you'll need connector->acpi_handle to figure out whether the feature is present and whether the property is needed. Figuring out acpi_handle also requires connector->acpi_device_id. It's a bit of a catch-22. I think the options are: 1) 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.) 2) 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 (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.) BR, Jani. > } > }
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: > > Lookup and attach ACPI nodes for intel connectors. The lookup is done > > in compliance with ACPI Spec 6.3 > > https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf > > (Ref: Pages 1119 - 1123). > > > > This can be useful for any connector specific platform properties. (This > > will be used for privacy screen in next patch). > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > --- > > v6: Addressed minor comments from Jani at > > https://lkml.org/lkml/2020/1/24/1143 > > - local variable renamed. > > - used drm_dbg_kms() > > - used acpi_device_handle() > > - Used opaque type acpi_handle instead of void* > > v5: same as v4 > > v4: Same as v3 > > v3: fold the code into existing acpi_device_id_update() function > > v2: formed by splitting the original patch into ACPI lookup, and privacy > > screen property. Also move it into i915 now that I found existing code > > in i915 that can be re-used. > > > > drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++++++++++++++++++ > > .../drm/i915/display/intel_display_types.h | 5 ++++ > > drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ > > 3 files changed, 32 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c > > index 3e6831cca4ac1..870c1ad98df92 100644 > > --- a/drivers/gpu/drm/i915/display/intel_acpi.c > > +++ b/drivers/gpu/drm/i915/display/intel_acpi.c > > @@ -222,11 +222,22 @@ static u32 acpi_display_type(struct intel_connector *connector) > > return display_type; > > } > > > > +/* > > + * Ref: ACPI Spec 6.3 > > + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf > > + * Pages 1119 - 1123 describe, what I believe, a standard way of > > + * identifying / addressing "display panels" in the ACPI. It provides > > + * a way for the ACPI to define devices for the display panels attached > > + * to the system. It thus provides a way for the BIOS to export any panel > > + * specific properties to the system via ACPI (like device trees). > > + */ > > void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = &dev_priv->drm; > > struct intel_connector *connector; > > struct drm_connector_list_iter conn_iter; > > + struct acpi_device *conn_dev; > > + u64 conn_addr; > > u8 display_index[16] = {}; > > > > /* Populate the ACPI IDs for all connectors for a given drm_device */ > > @@ -242,6 +253,19 @@ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) > > device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT; > > > > connector->acpi_device_id = device_id; > > + > > + /* Build the _ADR to look for */ > > + conn_addr = device_id | ACPI_DEVICE_ID_SCHEME | > > + ACPI_BIOS_CAN_DETECT; > > + > > + drm_dbg_kms(dev, "Checking connector ACPI node at _ADR=%llX\n", > > + conn_addr); > > + > > + /* Look up the connector device, under the PCI device */ > > + conn_dev = acpi_find_child_device( > > + ACPI_COMPANION(&dev->pdev->dev), > > + conn_addr, false); > > + connector->acpi_handle = acpi_device_handle(conn_dev); > > } > > drm_connector_list_iter_end(&conn_iter); > > } > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 5e00e611f077f..d70612cc1ba2a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -411,9 +411,14 @@ struct intel_connector { > > */ > > struct intel_encoder *encoder; > > > > +#ifdef CONFIG_ACPI > > /* ACPI device id for ACPI and driver cooperation */ > > u32 acpi_device_id; > > > > + /* ACPI handle corresponding to this connector display, if found */ > > + acpi_handle acpi_handle; > > +#endif > > + > > /* Reads out the current hw, returning true if the connector is enabled > > * and active (i.e. dpms ON state). */ > > bool (*get_hw_state)(struct intel_connector *); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > > index 0a417cd2af2bc..171821113d362 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -44,6 +44,7 @@ > > #include "i915_debugfs.h" > > #include "i915_drv.h" > > #include "i915_trace.h" > > +#include "intel_acpi.h" > > #include "intel_atomic.h" > > #include "intel_audio.h" > > #include "intel_connector.h" > > @@ -6868,6 +6869,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect > > > > connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT; > > > > + /* Lookup the ACPI node corresponding to the connector */ > > + intel_acpi_device_id_update(dev_priv); > > I find this part problematic. > > Normally, we call the function at probe via i915_driver_register() -> > intel_opregion_register() -> intel_opregion_resume() -> > intel_didl_outputs() -> intel_acpi_device_id_update(). It gets called > *once* at probe, after we have all the outputs (and thus connectors) > figured out. > > This in turn calls it for every DP connector, before we even have all > connectors registered. But it also re-iterates the previously handled > connectors again and again. > > The problem, of course, is that you'll need connector->acpi_handle to > figure out whether the feature is present and whether the property is > needed. Figuring out acpi_handle also requires > connector->acpi_device_id. > > It's a bit of a catch-22. > > I think the options are: > > 1) 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: [ 1.223163] WARNING: CPU: 2 PID: 1 at drivers/gpu/drm/drm_mode_object.c:45 __drm_mode_object_add+0xab/0xaf .... <snip> ..... [ 1.223540] Call Trace: [ 1.223540] drm_property_create+0xcc/0x155 [ 1.223540] drm_property_create_enum+0x26/0x79 [ 1.223540] intel_attach_privacy_screen_property+0x3a/0x5d [ 1.223540] intel_dp_connector_register+0x6a/0xa8 [ 1.223540] drm_connector_register+0x61/0xaa [ 1.223540] drm_connector_register_all+0x46/0x8b [ 1.223540] drm_modeset_register_all+0x3e/0x67 [ 1.223540] drm_dev_register+0x124/0x187 [ 1.223540] i915_driver_probe+0xa18/0xda0 [ 1.223540] i915_pci_probe+0x145/0x168 > > 2) 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. > (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? I can write the code up and send as my next patch iteration. Thanks, Rajat > > > } > > } > > -- > Jani Nikula, Intel Open Source Graphics Center
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: >> 1) 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. >> 2) 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. >> (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.
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: > >> 1) 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. > > >> 2) 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: 1) 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
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: > > >> 1) 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 > > > > > >> 2) 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: > > 1) 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
diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c index 3e6831cca4ac1..870c1ad98df92 100644 --- a/drivers/gpu/drm/i915/display/intel_acpi.c +++ b/drivers/gpu/drm/i915/display/intel_acpi.c @@ -222,11 +222,22 @@ static u32 acpi_display_type(struct intel_connector *connector) return display_type; } +/* + * Ref: ACPI Spec 6.3 + * https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf + * Pages 1119 - 1123 describe, what I believe, a standard way of + * identifying / addressing "display panels" in the ACPI. It provides + * a way for the ACPI to define devices for the display panels attached + * to the system. It thus provides a way for the BIOS to export any panel + * specific properties to the system via ACPI (like device trees). + */ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) { struct drm_device *dev = &dev_priv->drm; struct intel_connector *connector; struct drm_connector_list_iter conn_iter; + struct acpi_device *conn_dev; + u64 conn_addr; u8 display_index[16] = {}; /* Populate the ACPI IDs for all connectors for a given drm_device */ @@ -242,6 +253,19 @@ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv) device_id |= display_index[type]++ << ACPI_DISPLAY_INDEX_SHIFT; connector->acpi_device_id = device_id; + + /* Build the _ADR to look for */ + conn_addr = device_id | ACPI_DEVICE_ID_SCHEME | + ACPI_BIOS_CAN_DETECT; + + drm_dbg_kms(dev, "Checking connector ACPI node at _ADR=%llX\n", + conn_addr); + + /* Look up the connector device, under the PCI device */ + conn_dev = acpi_find_child_device( + ACPI_COMPANION(&dev->pdev->dev), + conn_addr, false); + connector->acpi_handle = acpi_device_handle(conn_dev); } drm_connector_list_iter_end(&conn_iter); } diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 5e00e611f077f..d70612cc1ba2a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -411,9 +411,14 @@ struct intel_connector { */ struct intel_encoder *encoder; +#ifdef CONFIG_ACPI /* ACPI device id for ACPI and driver cooperation */ u32 acpi_device_id; + /* ACPI handle corresponding to this connector display, if found */ + acpi_handle acpi_handle; +#endif + /* Reads out the current hw, returning true if the connector is enabled * and active (i.e. dpms ON state). */ bool (*get_hw_state)(struct intel_connector *); diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0a417cd2af2bc..171821113d362 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -44,6 +44,7 @@ #include "i915_debugfs.h" #include "i915_drv.h" #include "i915_trace.h" +#include "intel_acpi.h" #include "intel_atomic.h" #include "intel_audio.h" #include "intel_connector.h" @@ -6868,6 +6869,8 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect connector->state->scaling_mode = DRM_MODE_SCALE_ASPECT; + /* Lookup the ACPI node corresponding to the connector */ + intel_acpi_device_id_update(dev_priv); } }
Lookup and attach ACPI nodes for intel connectors. The lookup is done in compliance with ACPI Spec 6.3 https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf (Ref: Pages 1119 - 1123). This can be useful for any connector specific platform properties. (This will be used for privacy screen in next patch). Signed-off-by: Rajat Jain <rajatja@google.com> --- v6: Addressed minor comments from Jani at https://lkml.org/lkml/2020/1/24/1143 - local variable renamed. - used drm_dbg_kms() - used acpi_device_handle() - Used opaque type acpi_handle instead of void* v5: same as v4 v4: Same as v3 v3: fold the code into existing acpi_device_id_update() function v2: formed by splitting the original patch into ACPI lookup, and privacy screen property. Also move it into i915 now that I found existing code in i915 that can be re-used. drivers/gpu/drm/i915/display/intel_acpi.c | 24 +++++++++++++++++++ .../drm/i915/display/intel_display_types.h | 5 ++++ drivers/gpu/drm/i915/display/intel_dp.c | 3 +++ 3 files changed, 32 insertions(+)