[Intel-gfx] [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries
Hans de Goede
hdegoede at redhat.com
Wed May 5 09:07:28 UTC 2021
Hi,
On 5/4/21 9:52 AM, Andy Shevchenko wrote:
>
>
> On Monday, May 3, 2021, Hans de Goede <hdegoede at redhat.com <mailto:hdegoede at redhat.com>> wrote:
>
> From: Heikki Krogerus <heikki.krogerus at linux.intel.com <mailto:heikki.krogerus at linux.intel.com>>
>
> On Intel platforms we know that the ACPI connector device
> node order will follow the order the driver (i915) decides.
> The decision is made using the custom Intel ACPI OpRegion
> (intel_opregion.c), though the driver does not actually know
> that the values it sends to ACPI there are used for
> associating a device node for the connectors, and assigning
> address for them.
>
> In reality that custom Intel ACPI OpRegion actually violates
> ACPI specification (we supply dynamic information to objects
> that are defined static, for example _ADR), however, it
> makes assigning correct connector node for a connector entry
> straightforward (it's one-on-one mapping).
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus at linux.intel.com <mailto:heikki.krogerus at linux.intel.com>>
> [hdegoede at redhat.com <mailto:hdegoede at redhat.com>: Move intel_acpi_assign_connector_fwnodes() to
> intel_acpi.c]
> Signed-off-by: Hans de Goede <hdegoede at redhat.com <mailto:hdegoede at redhat.com>>
> ---
> drivers/gpu/drm/i915/display/intel_acpi.c | 40 ++++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_acpi.h | 3 ++
> drivers/gpu/drm/i915/display/intel_display.c | 1 +
> 3 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c b/drivers/gpu/drm/i915/display/intel_acpi.c
> index 833d0c1be4f1..9f266dfda7dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> @@ -263,3 +263,43 @@ void intel_acpi_device_id_update(struct drm_i915_private *dev_priv)
> }
> drm_connector_list_iter_end(&conn_iter);
> }
> +
> +/* NOTE: The connector order must be final before this is called. */
> +void intel_acpi_assign_connector_fwnodes(struct drm_i915_private *i915)
> +{
> + struct drm_connector_list_iter conn_iter;
> + struct drm_device *drm_dev = &i915->drm;
> + struct device *kdev = &drm_dev->pdev->dev;
> + struct fwnode_handle *fwnode = NULL;
> + struct drm_connector *connector;
> + struct acpi_device *adev;
> +
> + drm_connector_list_iter_begin(drm_dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + /* Always getting the next, even when the last was not used. */
> + fwnode = device_get_next_child_node(kdev, fwnode);
> + if (!fwnode)
> + break;
>
>
>
> Who is dropping reference counting on fwnode ?
We are dealing with ACPI fwnode-s here and those are not ref-counted, they
are embedded inside a struct acpi_device and their lifetime is tied to
that struct. They should probably still be ref-counted (with the count
never dropping to 0) so that the generic fwnode functions behave the same
anywhere but atm the ACPI nodes are not refcounted, see: acpi_get_next_subnode()
in drivers/acpi/property.c which is the get_next_child_node() implementation
for ACPI fwnode-s.
> I’m in the middle of a pile of fixes for fwnode refcounting when for_each_child or get_next_child is used. So, please double check you drop a reference.
The kdoc comments on device_get_next_child_node() / fwnode_get_next_child_node()
do not mention anything about these functions returning a reference.
So I think we need to first make up our mind here how we want this all to
work and then fix the actual implementation and docs before fixing callers.
Regards,
Hans
>
>
> +
> + switch (connector->connector_type) {
> + case DRM_MODE_CONNECTOR_LVDS:
> + case DRM_MODE_CONNECTOR_eDP:
> + case DRM_MODE_CONNECTOR_DSI:
> + /*
> + * Integrated displays have a specific address 0x1f on
> + * most Intel platforms, but not on all of them.
> + */
> + adev = acpi_find_child_device(ACPI_COMPANION(kdev),
> + 0x1f, 0);
> + if (adev) {
> + connector->fwnode = acpi_fwnode_handle(adev);
> + break;
> + }
> + fallthrough;
> + default:
> + connector->fwnode = fwnode;
> + break;
> + }
> + }
> + drm_connector_list_iter_end(&conn_iter);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h b/drivers/gpu/drm/i915/display/intel_acpi.h
> index e8b068661d22..d2435691f4b5 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.h
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.h
> @@ -12,11 +12,14 @@ struct drm_i915_private;
> void intel_register_dsm_handler(void);
> void intel_unregister_dsm_handler(void);
> void intel_acpi_device_id_update(struct drm_i915_private *i915);
> +void intel_acpi_assign_connector_fwnodes(struct drm_i915_private *i915);
> #else
> static inline void intel_register_dsm_handler(void) { return; }
> static inline void intel_unregister_dsm_handler(void) { return; }
> static inline
> void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; }
> +static inline
> +void intel_acpi_assign_connector_fwnodes(struct drm_i915_private *i915) { return; }
> #endif /* CONFIG_ACPI */
>
> #endif /* __INTEL_ACPI_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 828ef4c5625f..87cad549632c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14970,6 +14970,7 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
>
> drm_modeset_lock_all(dev);
> intel_modeset_setup_hw_state(dev, dev->mode_config.acquire_ctx);
> + intel_acpi_assign_connector_fwnodes(i915);
> drm_modeset_unlock_all(dev);
>
> for_each_intel_crtc(dev, crtc) {
> --
> 2.31.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
More information about the Intel-gfx
mailing list