[PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification (v2)

[PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification (v2)

[Hans de Goede]

Hi All,

Here is v2 of my work on making DP over Type-C work on devices where the
Type-C controller does not drive the HPD pin on the GPU, but instead
we need to forward HPD events from the Type-C controller to the DRM driver.

Changes in v2:
- Replace the bogus "drm/connector: Make the drm_sysfs connector->kdev
  device hold a reference to the connector" patch with:
  "drm/connector: Give connector sysfs devices there own device_type"
  the new patch is a dep for patch 2/9 see the patches

- Stop using a class-dev-iter, instead at a global connector list
  to drm_connector.c and use that to find the connector by the fwnode,
  similar to how we already do this in drm_panel.c and drm_bridge.c

- Make drm_connector_oob_hotplug_event() take a fwnode pointer as
  argument, rather then a drm_connector pointer and let it do the
  lookup itself. This allows making drm_connector_find_by_fwnode() a
  drm-internal function and avoids code outside the drm subsystem
  potentially holding on the a drm_connector reference for a longer
  period.

This series not only touches drm subsys files but it also touches
drivers/usb/typec/altmodes/typec_displayport.c, that file usually
does not see a whole lot of changes. So I believe it would be best
to just merge the entire series through drm-misc, Assuming we can
get an ack from Greg for merging the typec_displayport.c changes
this way.

### 

As already mentioned in the v1 cover-letter this series replaces
a previous attempt from quite some time ago. 
For anyone interested here are the old (2019!) patches for this:

https://patchwork.freedesktop.org/patch/288491/
https://patchwork.freedesktop.org/patch/288493/
https://patchwork.freedesktop.org/patch/288495/

Last time I posted this the biggest change requested was for more info to
be included in the event send to the DRM-subsystem, specifically sending
the following info was requested:

1. Which DP connector on the GPU the event is for
2. How many lanes are available
3. Connector orientation

This series is basically an entirely new approach, which no longer
uses the notifier framework at all. Instead the Type-C code looksup
a connector based on a fwnode (this was suggested by Heikki Krogerus)
and then calls a new oob_hotplug_event drm_connector_func directly
on the connector, passing the requested info as argument.

Info such as the orientation and the number of dp-lanes is now passed
to the drm_connector_oob_hotplug_event() function as requested in the
review of the old code, but nothing is done with it for now.
Using this info falls well outside of my knowledge of the i915 driver
so this is left to a follow-up patch (I will be available to test
patches for this).

Regards,

Hans


Hans de Goede (8):
  drm/connector: Give connector sysfs devices there own device_type
  drm/connector: Add a fwnode pointer to drm_connector and register with
    ACPI
  drm/connector: Add drm_connector_find_by_fwnode() function (v2)
  drm/connector: Add support for out-of-band hotplug notification (v2)
  drm/i915/dp: Add support for out-of-bound hotplug events
  usb: typec: altmodes/displayport: Make dp_altmode_notify() more
    generic
  usb: typec: altmodes/displayport: Notify drm subsys of hotplug events
  platform/x86/intel_cht_int33fe: Correct "displayport" fwnode reference

Heikki Krogerus (1):
  drm/i915: Associate ACPI connector nodes with connector entries

 drivers/gpu/drm/drm_connector.c               | 79 +++++++++++++++++
 drivers/gpu/drm/drm_crtc_internal.h           |  1 +
 drivers/gpu/drm/drm_sysfs.c                   | 87 ++++++++++++++++---
 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 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 13 +++
 .../platform/x86/intel_cht_int33fe_typec.c    |  4 +-
 drivers/usb/typec/altmodes/Kconfig            |  1 +
 drivers/usb/typec/altmodes/displayport.c      | 73 +++++++++++-----
 include/drm/drm_connector.h                   | 43 +++++++++
 11 files changed, 308 insertions(+), 37 deletions(-)

-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] drm/connector: Give connector sysfs devices there own device_type

[Hans de Goede]

Give connector sysfs devices there own device_type, this allows us to
check if a device passed to functions dealing with generic devices is
a drm_connector or not.

A check like this is necessary in the drm_connector_acpi_bus_match()
function added in the next patch in this series.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_sysfs.c | 50 +++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index f0336c804639..553024bcda8a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -50,6 +50,10 @@ static struct device_type drm_sysfs_device_minor = {
 	.name = "drm_minor"
 };
 
+static struct device_type drm_sysfs_device_connector = {
+	.name = "drm_connector",
+};
+
 struct class *drm_class;
 
 static char *drm_devnode(struct device *dev, umode_t *mode)
@@ -102,6 +106,11 @@ void drm_sysfs_destroy(void)
 	drm_class = NULL;
 }
 
+static void drm_sysfs_release(struct device *dev)
+{
+	kfree(dev);
+}
+
 /*
  * Connector properties
  */
@@ -274,27 +283,47 @@ static const struct attribute_group *connector_dev_groups[] = {
 int drm_sysfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
+	struct device *kdev;
+	int r;
 
 	if (connector->kdev)
 		return 0;
 
-	connector->kdev =
-		device_create_with_groups(drm_class, dev->primary->kdev, 0,
-					  connector, connector_dev_groups,
-					  "card%d-%s", dev->primary->index,
-					  connector->name);
+	kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
+	if (!kdev)
+		return -ENOMEM;
+
+	device_initialize(kdev);
+	kdev->class = drm_class;
+	kdev->type = &drm_sysfs_device_connector;
+	kdev->parent = dev->primary->kdev;
+	kdev->groups = connector_dev_groups;
+	kdev->release = drm_sysfs_release;
+	dev_set_drvdata(kdev, connector);
+
+	r = dev_set_name(kdev, "card%d-%s", dev->primary->index, connector->name);
+	if (r)
+		goto err_free;
+
 	DRM_DEBUG("adding \"%s\" to sysfs\n",
 		  connector->name);
 
-	if (IS_ERR(connector->kdev)) {
-		DRM_ERROR("failed to register connector device: %ld\n", PTR_ERR(connector->kdev));
-		return PTR_ERR(connector->kdev);
+	r = device_add(kdev);
+	if (r) {
+		DRM_ERROR("failed to register connector device: %d\n", r);
+		goto err_free;
 	}
 
+	connector->kdev = kdev;
+
 	if (connector->ddc)
 		return sysfs_create_link(&connector->kdev->kobj,
 				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
+
+err_free:
+	put_device(kdev);
+	return r;
 }
 
 void drm_sysfs_connector_remove(struct drm_connector *connector)
@@ -375,11 +404,6 @@ void drm_sysfs_connector_status_event(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_sysfs_connector_status_event);
 
-static void drm_sysfs_release(struct device *dev)
-{
-	kfree(dev);
-}
-
 struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
 {
 	const char *minor_str;
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/9] drm/connector: Add a fwnode pointer to drm_connector and register with ACPI

[Hans de Goede]

Add a fwnode pointer to struct drm_connector and register an acpi_bus_type
for the connectors with the ACPI subsystem (when CONFIG_ACPI is enabled).

The adding of the fwnode pointer allows drivers to associate a fwnode
that represents a connector with that connector.

When the new fwnode pointer points to an ACPI-companion, then the new
acpi_bus_type will cause the ACPI subsys to bind the device instantiated
for the connector with the fwnode by calling acpi_bind_one(). This will
result in a firmware_node symlink under /sys/class/card#-<connecter-name>/
which helps to verify that the fwnode-s and connectors are properly
matched.

Co-authored-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_sysfs.c | 37 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h |  2 ++
 2 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 553024bcda8a..12cc649c44f0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -10,6 +10,7 @@
  * Copyright (c) 2003-2004 IBM Corp.
  */
 
+#include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/export.h>
@@ -56,6 +57,39 @@ static struct device_type drm_sysfs_device_connector = {
 
 struct class *drm_class;
 
+#ifdef CONFIG_ACPI
+static bool drm_connector_acpi_bus_match(struct device *dev)
+{
+	return dev->type == &drm_sysfs_device_connector;
+}
+
+static struct acpi_device *drm_connector_acpi_find_companion(struct device *dev)
+{
+	struct drm_connector *connector = to_drm_connector(dev);
+
+	return to_acpi_device_node(connector->fwnode);
+}
+
+static struct acpi_bus_type drm_connector_acpi_bus = {
+	.name = "drm_connector",
+	.match = drm_connector_acpi_bus_match,
+	.find_companion = drm_connector_acpi_find_companion,
+};
+
+static void drm_sysfs_acpi_register(void)
+{
+	register_acpi_bus_type(&drm_connector_acpi_bus);
+}
+
+static void drm_sysfs_acpi_unregister(void)
+{
+	unregister_acpi_bus_type(&drm_connector_acpi_bus);
+}
+#else
+static void drm_sysfs_acpi_register(void) { }
+static void drm_sysfs_acpi_unregister(void) { }
+#endif
+
 static char *drm_devnode(struct device *dev, umode_t *mode)
 {
 	return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev));
@@ -89,6 +123,8 @@ int drm_sysfs_init(void)
 	}
 
 	drm_class->devnode = drm_devnode;
+
+	drm_sysfs_acpi_register();
 	return 0;
 }
 
@@ -101,6 +137,7 @@ void drm_sysfs_destroy(void)
 {
 	if (IS_ERR_OR_NULL(drm_class))
 		return;
+	drm_sysfs_acpi_unregister();
 	class_remove_file(drm_class, &class_attr_version.attr);
 	class_destroy(drm_class);
 	drm_class = NULL;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 0261801af62c..d20bfd7576ed 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1254,6 +1254,8 @@ struct drm_connector {
 	struct device *kdev;
 	/** @attr: sysfs attributes */
 	struct device_attribute *attr;
+	/** @fwnode: associated fwnode supplied by platform firmware */
+	struct fwnode_handle *fwnode;
 
 	/**
 	 * @head:
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/9] drm/connector: Add a fwnode pointer to drm_connector and register with ACPI

[Andy Shevchenko]

[-- Attachment #1.1: Type: text/plain, Size: 3702 bytes --]

On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com> wrote:

> Add a fwnode pointer to struct drm_connector and register an acpi_bus_type
> for the connectors with the ACPI subsystem (when CONFIG_ACPI is enabled).
>
> The adding of the fwnode pointer allows drivers to associate a fwnode
> that represents a connector with that connector.
>
> When the new fwnode pointer points to an ACPI-companion, then the new
> acpi_bus_type will cause the ACPI subsys to bind the device instantiated
> for the connector with the fwnode by calling acpi_bind_one(). This will
> result in a firmware_node symlink under /sys/class/card#-<connecter-name>/
> which helps to verify that the fwnode-s and connectors are properly
> matched.
>
> Co-authored-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>



Official tag is Co-developed-by


> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 37 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h |  2 ++
>  2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 553024bcda8a..12cc649c44f0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -10,6 +10,7 @@
>   * Copyright (c) 2003-2004 IBM Corp.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> @@ -56,6 +57,39 @@ static struct device_type drm_sysfs_device_connector = {
>
>  struct class *drm_class;
>
> +#ifdef CONFIG_ACPI
> +static bool drm_connector_acpi_bus_match(struct device *dev)
> +{
> +       return dev->type == &drm_sysfs_device_connector;
> +}
> +
> +static struct acpi_device *drm_connector_acpi_find_companion(struct
> device *dev)
> +{
> +       struct drm_connector *connector = to_drm_connector(dev);
> +
> +       return to_acpi_device_node(connector->fwnode);
> +}
> +
> +static struct acpi_bus_type drm_connector_acpi_bus = {
> +       .name = "drm_connector",
> +       .match = drm_connector_acpi_bus_match,
> +       .find_companion = drm_connector_acpi_find_companion,
> +};
> +
> +static void drm_sysfs_acpi_register(void)
> +{
> +       register_acpi_bus_type(&drm_connector_acpi_bus);
> +}
> +
> +static void drm_sysfs_acpi_unregister(void)
> +{
> +       unregister_acpi_bus_type(&drm_connector_acpi_bus);
> +}
> +#else
> +static void drm_sysfs_acpi_register(void) { }
> +static void drm_sysfs_acpi_unregister(void) { }
> +#endif
> +
>  static char *drm_devnode(struct device *dev, umode_t *mode)
>  {
>         return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev));
> @@ -89,6 +123,8 @@ int drm_sysfs_init(void)
>         }
>
>         drm_class->devnode = drm_devnode;
> +
> +       drm_sysfs_acpi_register();
>         return 0;
>  }
>
> @@ -101,6 +137,7 @@ void drm_sysfs_destroy(void)
>  {
>         if (IS_ERR_OR_NULL(drm_class))
>                 return;
> +       drm_sysfs_acpi_unregister();
>         class_remove_file(drm_class, &class_attr_version.attr);
>         class_destroy(drm_class);
>         drm_class = NULL;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 0261801af62c..d20bfd7576ed 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1254,6 +1254,8 @@ struct drm_connector {
>         struct device *kdev;
>         /** @attr: sysfs attributes */
>         struct device_attribute *attr;
> +       /** @fwnode: associated fwnode supplied by platform firmware */
> +       struct fwnode_handle *fwnode;
>
>         /**
>          * @head:
> --
> 2.31.1
>
>

-- 
With Best Regards,
Andy Shevchenko

[-- Attachment #1.2: Type: text/html, Size: 4808 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] drm/connector: Add drm_connector_find_by_fwnode() function (v2)

[Hans de Goede]

Add a function to find a connector based on a fwnode.

This will be used by the new drm_connector_oob_hotplug_event()
function which is added by the next patch in this patch-set.

Changes in v2:
- Complete rewrite to use a global connector list in drm_connector.c
  rather then using a class-dev-iter in drm_sysfs.c

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c     | 50 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_internal.h |  1 +
 include/drm/drm_connector.h         |  8 +++++
 3 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 87c68563e6c3..ef759d6add81 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -66,6 +66,14 @@
  * support can instead use e.g. drm_helper_hpd_irq_event().
  */
 
+/*
+ * Global connector list for drm_connector_find_by_fwnode().
+ * Note drm_connector_[un]register() first take connector->lock and then
+ * take the connector_list_lock.
+ */
+static DEFINE_MUTEX(connector_list_lock);
+static LIST_HEAD(connector_list);
+
 struct drm_conn_prop_enum_list {
 	int type;
 	const char *name;
@@ -267,6 +275,7 @@ int drm_connector_init(struct drm_device *dev,
 		goto out_put_type_id;
 	}
 
+	INIT_LIST_HEAD(&connector->global_connector_list_entry);
 	INIT_LIST_HEAD(&connector->probed_modes);
 	INIT_LIST_HEAD(&connector->modes);
 	mutex_init(&connector->mutex);
@@ -540,6 +549,9 @@ int drm_connector_register(struct drm_connector *connector)
 		drm_privacy_screen_register_notifier(connector->privacy_screen,
 					   &connector->privacy_screen_notifier);
 
+	mutex_lock(&connector_list_lock);
+	list_add_tail(&connector->global_connector_list_entry, &connector_list);
+	mutex_unlock(&connector_list_lock);
 	goto unlock;
 
 err_debugfs:
@@ -568,6 +580,10 @@ void drm_connector_unregister(struct drm_connector *connector)
 		return;
 	}
 
+	mutex_lock(&connector_list_lock);
+	list_del_init(&connector->global_connector_list_entry);
+	mutex_unlock(&connector_list_lock);
+
 	if (connector->privacy_screen)
 		drm_privacy_screen_unregister_notifier(
 					connector->privacy_screen,
@@ -2676,6 +2692,40 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	return ret;
 }
 
+/**
+ * drm_connector_find_by_fwnode - Find a connector based on the associated fwnode
+ * @fwnode: fwnode for which to find the matching drm_connector
+ *
+ * This functions looks up a drm_connector based on its associated fwnode. When
+ * a connector is found a reference to the connector is returned. The caller must
+ * call drm_connector_put() to release this reference when it is done with the
+ * connector.
+ *
+ * Returns: A reference to the found connector or an ERR_PTR().
+ */
+struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
+{
+	struct drm_connector *connector, *found = ERR_PTR(-ENODEV);
+
+	if (!fwnode)
+		return ERR_PTR(-ENODEV);
+
+	mutex_lock(&connector_list_lock);
+
+	list_for_each_entry(connector, &connector_list, global_connector_list_entry) {
+		if (connector->fwnode == fwnode ||
+		    (connector->fwnode && connector->fwnode->secondary == fwnode)) {
+			drm_connector_get(connector);
+			found = connector;
+			break;
+		}
+	}
+
+	mutex_unlock(&connector_list_lock);
+
+	return found;
+}
+
 
 /**
  * DOC: Tile group
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 54d4cf1233e9..6e28fc00a740 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -185,6 +185,7 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
 int drm_connector_create_standard_properties(struct drm_device *dev);
 const char *drm_get_connector_force_name(enum drm_connector_force force);
 void drm_connector_free_work_fn(struct work_struct *work);
+struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode);
 
 /* IOCTL */
 int drm_connector_property_set_ioctl(struct drm_device *dev,
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index d20bfd7576ed..ae377354e48e 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1267,6 +1267,14 @@ struct drm_connector {
 	 */
 	struct list_head head;
 
+	/**
+	 * @global_connector_list_entry:
+	 *
+	 * Connector entry in the global connector-list, used by
+	 * drm_connector_find_by_fwnode().
+	 */
+	struct list_head global_connector_list_entry;
+
 	/** @base: base KMS object */
 	struct drm_mode_object base;
 
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/9] drm/connector: Add drm_connector_find_by_fwnode() function (v2)

[Andy Shevchenko]

[-- Attachment #1.1: Type: text/plain, Size: 5513 bytes --]

On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com> wrote:

> Add a function to find a connector based on a fwnode.
>
> This will be used by the new drm_connector_oob_hotplug_event()
> function which is added by the next patch in this patch-set.
>
> Changes in v2:
> - Complete rewrite to use a global connector list in drm_connector.c
>   rather then using a class-dev-iter in drm_sysfs.c
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/drm_connector.c     | 50 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc_internal.h |  1 +
>  include/drm/drm_connector.h         |  8 +++++
>  3 files changed, 59 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_
> connector.c
> index 87c68563e6c3..ef759d6add81 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -66,6 +66,14 @@
>   * support can instead use e.g. drm_helper_hpd_irq_event().
>   */
>
> +/*
> + * Global connector list for drm_connector_find_by_fwnode().
> + * Note drm_connector_[un]register() first take connector->lock and then
> + * take the connector_list_lock.
> + */
> +static DEFINE_MUTEX(connector_list_lock);
> +static LIST_HEAD(connector_list);
> +
>  struct drm_conn_prop_enum_list {
>         int type;
>         const char *name;
> @@ -267,6 +275,7 @@ int drm_connector_init(struct drm_device *dev,
>                 goto out_put_type_id;
>         }
>
> +       INIT_LIST_HEAD(&connector->global_connector_list_entry);
>         INIT_LIST_HEAD(&connector->probed_modes);
>         INIT_LIST_HEAD(&connector->modes);
>         mutex_init(&connector->mutex);
> @@ -540,6 +549,9 @@ int drm_connector_register(struct drm_connector
> *connector)
>                 drm_privacy_screen_register_notifier(connector->privacy_
> screen,
>                                            &connector->privacy_screen_
> notifier);
>
> +       mutex_lock(&connector_list_lock);
> +       list_add_tail(&connector->global_connector_list_entry,
> &connector_list);
> +       mutex_unlock(&connector_list_lock);
>         goto unlock;
>
>  err_debugfs:
> @@ -568,6 +580,10 @@ void drm_connector_unregister(struct drm_connector
> *connector)
>                 return;
>         }
>
> +       mutex_lock(&connector_list_lock);
> +       list_del_init(&connector->global_connector_list_entry);
> +       mutex_unlock(&connector_list_lock);
> +
>         if (connector->privacy_screen)
>                 drm_privacy_screen_unregister_notifier(
>                                         connector->privacy_screen,
> @@ -2676,6 +2692,40 @@ int drm_mode_getconnector(struct drm_device *dev,
> void *data,
>         return ret;
>  }
>
> +/**
> + * drm_connector_find_by_fwnode - Find a connector based on the
> associated fwnode
> + * @fwnode: fwnode for which to find the matching drm_connector
> + *
> + * This functions looks up a drm_connector based on its associated
> fwnode. When
> + * a connector is found a reference to the connector is returned. The
> caller must
> + * call drm_connector_put() to release this reference when it is done
> with the
> + * connector.
> + *
> + * Returns: A reference to the found connector or an ERR_PTR().
> + */
> +struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle
> *fwnode)
> +{
> +       struct drm_connector *connector, *found = ERR_PTR(-ENODEV);
> +
> +       if (!fwnode)
> +               return ERR_PTR(-ENODEV);
> +
> +       mutex_lock(&connector_list_lock);
> +
> +       list_for_each_entry(connector, &connector_list,
> global_connector_list_entry) {
> +               if (connector->fwnode == fwnode ||
> +                   (connector->fwnode && connector->fwnode->secondary ==
> fwnode)) {
> +                       drm_connector_get(connector);
> +                       found = connector;
> +                       break;
> +               }
> +       }
> +
> +       mutex_unlock(&connector_list_lock);
> +
> +       return found;



If I am not mistaken you can replace this with

return list_entry_is_head();

call and remove additional Boolean variable.


> +}
> +
>
>  /**
>   * DOC: Tile group
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> b/drivers/gpu/drm/drm_crtc_internal.h
> index 54d4cf1233e9..6e28fc00a740 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -185,6 +185,7 @@ int drm_connector_set_obj_prop(struct drm_mode_object
> *obj,
>  int drm_connector_create_standard_properties(struct drm_device *dev);
>  const char *drm_get_connector_force_name(enum drm_connector_force force);
>  void drm_connector_free_work_fn(struct work_struct *work);
> +struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle
> *fwnode);
>
>  /* IOCTL */
>  int drm_connector_property_set_ioctl(struct drm_device *dev,
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index d20bfd7576ed..ae377354e48e 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1267,6 +1267,14 @@ struct drm_connector {
>          */
>         struct list_head head;
>
> +       /**
> +        * @global_connector_list_entry:
> +        *
> +        * Connector entry in the global connector-list, used by
> +        * drm_connector_find_by_fwnode().
> +        */
> +       struct list_head global_connector_list_entry;
> +
>         /** @base: base KMS object */
>         struct drm_mode_object base;
>
> --
> 2.31.1
>
>

-- 
With Best Regards,
Andy Shevchenko

[-- Attachment #1.2: Type: text/html, Size: 6971 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/9] drm/connector: Add drm_connector_find_by_fwnode() function (v2)

[Hans de Goede]

Hi,

On 5/4/21 10:00 AM, Andy Shevchenko wrote:
> 
> 
> On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
>     Add a function to find a connector based on a fwnode.
> 
>     This will be used by the new drm_connector_oob_hotplug_event()
>     function which is added by the next patch in this patch-set.
> 
>     Changes in v2:
>     - Complete rewrite to use a global connector list in drm_connector.c
>       rather then using a class-dev-iter in drm_sysfs.c
> 
>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>     ---
>      drivers/gpu/drm/drm_connector.c     | 50 +++++++++++++++++++++++++++++
>      drivers/gpu/drm/drm_crtc_internal.h |  1 +
>      include/drm/drm_connector.h         |  8 +++++
>      3 files changed, 59 insertions(+)
> 
>     diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>     index 87c68563e6c3..ef759d6add81 100644
>     --- a/drivers/gpu/drm/drm_connector.c
>     +++ b/drivers/gpu/drm/drm_connector.c
>     @@ -66,6 +66,14 @@
>       * support can instead use e.g. drm_helper_hpd_irq_event().
>       */
> 
>     +/*
>     + * Global connector list for drm_connector_find_by_fwnode().
>     + * Note drm_connector_[un]register() first take connector->lock and then
>     + * take the connector_list_lock.
>     + */
>     +static DEFINE_MUTEX(connector_list_lock);
>     +static LIST_HEAD(connector_list);
>     +
>      struct drm_conn_prop_enum_list {
>             int type;
>             const char *name;
>     @@ -267,6 +275,7 @@ int drm_connector_init(struct drm_device *dev,
>                     goto out_put_type_id;
>             }
> 
>     +       INIT_LIST_HEAD(&connector->global_connector_list_entry);
>             INIT_LIST_HEAD(&connector->probed_modes);
>             INIT_LIST_HEAD(&connector->modes);
>             mutex_init(&connector->mutex);
>     @@ -540,6 +549,9 @@ int drm_connector_register(struct drm_connector *connector)
>                     drm_privacy_screen_register_notifier(connector->privacy_screen,
>                                                &connector->privacy_screen_notifier);
> 
>     +       mutex_lock(&connector_list_lock);
>     +       list_add_tail(&connector->global_connector_list_entry, &connector_list);
>     +       mutex_unlock(&connector_list_lock);
>             goto unlock;
> 
>      err_debugfs:
>     @@ -568,6 +580,10 @@ void drm_connector_unregister(struct drm_connector *connector)
>                     return;
>             }
> 
>     +       mutex_lock(&connector_list_lock);
>     +       list_del_init(&connector->global_connector_list_entry);
>     +       mutex_unlock(&connector_list_lock);
>     +
>             if (connector->privacy_screen)
>                     drm_privacy_screen_unregister_notifier(
>                                             connector->privacy_screen,
>     @@ -2676,6 +2692,40 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>             return ret;
>      }
> 
>     +/**
>     + * drm_connector_find_by_fwnode - Find a connector based on the associated fwnode
>     + * @fwnode: fwnode for which to find the matching drm_connector
>     + *
>     + * This functions looks up a drm_connector based on its associated fwnode. When
>     + * a connector is found a reference to the connector is returned. The caller must
>     + * call drm_connector_put() to release this reference when it is done with the
>     + * connector.
>     + *
>     + * Returns: A reference to the found connector or an ERR_PTR().
>     + */
>     +struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
>     +{
>     +       struct drm_connector *connector, *found = ERR_PTR(-ENODEV);
>     +
>     +       if (!fwnode)
>     +               return ERR_PTR(-ENODEV);
>     +
>     +       mutex_lock(&connector_list_lock);
>     +
>     +       list_for_each_entry(connector, &connector_list, global_connector_list_entry) {
>     +               if (connector->fwnode == fwnode ||
>     +                   (connector->fwnode && connector->fwnode->secondary == fwnode)) {
>     +                       drm_connector_get(connector);
>     +                       found = connector;
>     +                       break;
>     +               }
>     +       }
>     +
>     +       mutex_unlock(&connector_list_lock);
>     +
>     +       return found;
> 
> 
> 
> If I am not mistaken you can replace this with
> 
> return list_entry_is_head();
> 
> call and remove additional Boolean variable.

Found is not a boolean, it is a pointer to the found connector (or ERR_PTR(-ENODEV)).

Regards,

Hans


>  
> 
>     +}
>     +
> 
>      /**
>       * DOC: Tile group
>     diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>     index 54d4cf1233e9..6e28fc00a740 100644
>     --- a/drivers/gpu/drm/drm_crtc_internal.h
>     +++ b/drivers/gpu/drm/drm_crtc_internal.h
>     @@ -185,6 +185,7 @@ int drm_connector_set_obj_prop(struct drm_mode_object *obj,
>      int drm_connector_create_standard_properties(struct drm_device *dev);
>      const char *drm_get_connector_force_name(enum drm_connector_force force);
>      void drm_connector_free_work_fn(struct work_struct *work);
>     +struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode);
> 
>      /* IOCTL */
>      int drm_connector_property_set_ioctl(struct drm_device *dev,
>     diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>     index d20bfd7576ed..ae377354e48e 100644
>     --- a/include/drm/drm_connector.h
>     +++ b/include/drm/drm_connector.h
>     @@ -1267,6 +1267,14 @@ struct drm_connector {
>              */
>             struct list_head head;
> 
>     +       /**
>     +        * @global_connector_list_entry:
>     +        *
>     +        * Connector entry in the global connector-list, used by
>     +        * drm_connector_find_by_fwnode().
>     +        */
>     +       struct list_head global_connector_list_entry;
>     +
>             /** @base: base KMS object */
>             struct drm_mode_object base;
>      
>     -- 
>     2.31.1
> 
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/9] drm/connector: Add drm_connector_find_by_fwnode() function (v2)

[Andy Shevchenko]

On Tue, May 4, 2021 at 2:53 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/4/21 10:00 AM, Andy Shevchenko wrote:
> > On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:

...

> >     +struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> >     +{
> >     +       struct drm_connector *connector, *found = ERR_PTR(-ENODEV);
> >     +
> >     +       if (!fwnode)
> >     +               return ERR_PTR(-ENODEV);
> >     +
> >     +       mutex_lock(&connector_list_lock);
> >     +
> >     +       list_for_each_entry(connector, &connector_list, global_connector_list_entry) {
> >     +               if (connector->fwnode == fwnode ||
> >     +                   (connector->fwnode && connector->fwnode->secondary == fwnode)) {
> >     +                       drm_connector_get(connector);
> >     +                       found = connector;
> >     +                       break;
> >     +               }
> >     +       }
> >     +
> >     +       mutex_unlock(&connector_list_lock);
> >     +
> >     +       return found;
> >
> > If I am not mistaken you can replace this with
> >
> > return list_entry_is_head();
> >
> > call and remove additional Boolean variable.
>
> Found is not a boolean, it is a pointer to the found connector (or ERR_PTR(-ENODEV)).

Ah, perhaps giving a better name? `match` ?

And to the initial topic, it's either an additional variable or
additional branch in this case. I think additional branch (taking into
account the length of the line or amount of lines) doesn't buy us
anything.

> >     +}

-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification (v2)

[Hans de Goede]

Add a new drm_connector_oob_hotplug_event() function and
oob_hotplug_event drm_connector_funcs member.

On some hardware a hotplug event notification may come from outside the
display driver / device. An example of this is some USB Type-C setups
where the hardware muxes the DisplayPort data and aux-lines but does
not pass the altmode HPD status bit to the GPU's DP HPD pin.

In cases like this the new drm_connector_oob_hotplug_event() function can
be used to report these out-of-band events.

Changes in v2:
- Make drm_connector_oob_hotplug_event() take a fwnode as argument and
  have it call drm_connector_find_by_fwnode() internally. This allows
  making drm_connector_find_by_fwnode() a drm-internal function and
  avoids code outside the drm subsystem potentially holding on the
  a drm_connector reference for a longer period.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/drm_connector.c | 29 +++++++++++++++++++++++++++++
 include/drm/drm_connector.h     | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ef759d6add81..b5e09d751694 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2726,6 +2726,35 @@ struct drm_connector *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
 	return found;
 }
 
+/**
+ * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
+ * @connector: connector to report the event on
+ * @data: data related to the event
+ *
+ * On some hardware a hotplug event notification may come from outside the display
+ * driver / device. An example of this is some USB Type-C setups where the hardware
+ * muxes the DisplayPort data and aux-lines but does not pass the altmode HPD
+ * status bit to the GPU's DP HPD pin.
+ *
+ * This function can be used to report these out-of-band events after obtaining
+ * a drm_connector reference through calling drm_connector_find_by_fwnode().
+ */
+void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
+				     struct drm_connector_oob_hotplug_event_data *data)
+{
+	struct drm_connector *connector;
+
+	connector = drm_connector_find_by_fwnode(connector_fwnode);
+	if (IS_ERR(connector))
+		return;
+
+	if (connector->funcs->oob_hotplug_event)
+		connector->funcs->oob_hotplug_event(connector, data);
+
+	drm_connector_put(connector);
+}
+EXPORT_SYMBOL(drm_connector_oob_hotplug_event);
+
 
 /**
  * DOC: Tile group
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ae377354e48e..bb61aeb9ba2d 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -27,6 +27,7 @@
 #include <linux/llist.h>
 #include <linux/ctype.h>
 #include <linux/hdmi.h>
+#include <linux/usb/typec.h> /* For enum typec_orientation */
 #include <drm/drm_mode_object.h>
 #include <drm/drm_util.h>
 
@@ -649,6 +650,27 @@ struct drm_connector_tv_margins {
 	unsigned int top;
 };
 
+/**
+ * struct drm_connector_oob_hotplug_event_data: OOB hotplug event data
+ *
+ * Contains data about out-of-band hotplug events, signalled through
+ * drm_connector_oob_hotplug_event().
+ */
+struct drm_connector_oob_hotplug_event_data {
+	/**
+	 * @connected: New connected status for the connector.
+	 */
+	bool connected;
+	/**
+	 * @dp_lanes: Number of available displayport lanes, 0 if unknown.
+	 */
+	int dp_lanes;
+	/**
+	 * @orientation: Connector orientation.
+	 */
+	enum typec_orientation orientation;
+};
+
 /**
  * struct drm_tv_connector_state - TV connector related states
  * @subconnector: selected subconnector
@@ -1110,6 +1132,15 @@ struct drm_connector_funcs {
 	 */
 	void (*atomic_print_state)(struct drm_printer *p,
 				   const struct drm_connector_state *state);
+
+	/**
+	 * @oob_hotplug_event:
+	 *
+	 * This will get called when a hotplug-event for a drm-connector
+	 * has been received from a source outside the display driver / device.
+	 */
+	void (*oob_hotplug_event)(struct drm_connector *connector,
+				  struct drm_connector_oob_hotplug_event_data *data);
 };
 
 /**
@@ -1704,6 +1735,8 @@ drm_connector_is_unregistered(struct drm_connector *connector)
 		DRM_CONNECTOR_UNREGISTERED;
 }
 
+void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
+				     struct drm_connector_oob_hotplug_event_data *data);
 const char *drm_get_connector_type_name(unsigned int connector_type);
 const char *drm_get_connector_status_name(enum drm_connector_status status);
 const char *drm_get_subpixel_order_name(enum subpixel_order order);
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification (v2)

[Heikki Krogerus]

> +/**
> + * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
> + * @connector: connector to report the event on
> + * @data: data related to the event
> + *
> + * On some hardware a hotplug event notification may come from outside the display
> + * driver / device. An example of this is some USB Type-C setups where the hardware
> + * muxes the DisplayPort data and aux-lines but does not pass the altmode HPD
> + * status bit to the GPU's DP HPD pin.
> + *
> + * This function can be used to report these out-of-band events after obtaining
> + * a drm_connector reference through calling drm_connector_find_by_fwnode().
> + */
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> +				     struct drm_connector_oob_hotplug_event_data *data)
> +{
> +	struct drm_connector *connector;
> +
> +	connector = drm_connector_find_by_fwnode(connector_fwnode);
> +	if (IS_ERR(connector))
> +		return;
> +
> +	if (connector->funcs->oob_hotplug_event)
> +		connector->funcs->oob_hotplug_event(connector, data);
> +
> +	drm_connector_put(connector);
> +}
> +EXPORT_SYMBOL(drm_connector_oob_hotplug_event);

So it does looks like the "data" parameter is not needed at all:

void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
{
	struct drm_connector *connector;

	connector = drm_connector_find_by_fwnode(connector_fwnode);
	if (IS_ERR(connector))
		return;

	if (connector->funcs->oob_hotplug_event)
		connector->funcs->oob_hotplug_event(connector);

	drm_connector_put(connector);
}

thanks,

-- 
heikki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification (v2)

[Hans de Goede]

Hi,

On 5/4/21 5:10 PM, Heikki Krogerus wrote:
>> +/**
>> + * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
>> + * @connector: connector to report the event on
>> + * @data: data related to the event
>> + *
>> + * On some hardware a hotplug event notification may come from outside the display
>> + * driver / device. An example of this is some USB Type-C setups where the hardware
>> + * muxes the DisplayPort data and aux-lines but does not pass the altmode HPD
>> + * status bit to the GPU's DP HPD pin.
>> + *
>> + * This function can be used to report these out-of-band events after obtaining
>> + * a drm_connector reference through calling drm_connector_find_by_fwnode().
>> + */
>> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
>> +				     struct drm_connector_oob_hotplug_event_data *data)
>> +{
>> +	struct drm_connector *connector;
>> +
>> +	connector = drm_connector_find_by_fwnode(connector_fwnode);
>> +	if (IS_ERR(connector))
>> +		return;
>> +
>> +	if (connector->funcs->oob_hotplug_event)
>> +		connector->funcs->oob_hotplug_event(connector, data);
>> +
>> +	drm_connector_put(connector);
>> +}
>> +EXPORT_SYMBOL(drm_connector_oob_hotplug_event);
> 
> So it does looks like the "data" parameter is not needed at all:

Well Imre did indicate that having the number of lanes is useful, so
for the next version I'll drop the orientation but I plan to keep
the number of lanes if that is ok with you.

Not having passing along this info was one of the reasons why my
previous attempt at this was nacked, so dropping it all together
feels wrong.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification (v2)

[Heikki Krogerus]

On Tue, May 04, 2021 at 05:35:49PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 5/4/21 5:10 PM, Heikki Krogerus wrote:
> >> +/**
> >> + * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
> >> + * @connector: connector to report the event on
> >> + * @data: data related to the event
> >> + *
> >> + * On some hardware a hotplug event notification may come from outside the display
> >> + * driver / device. An example of this is some USB Type-C setups where the hardware
> >> + * muxes the DisplayPort data and aux-lines but does not pass the altmode HPD
> >> + * status bit to the GPU's DP HPD pin.
> >> + *
> >> + * This function can be used to report these out-of-band events after obtaining
> >> + * a drm_connector reference through calling drm_connector_find_by_fwnode().
> >> + */
> >> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> >> +				     struct drm_connector_oob_hotplug_event_data *data)
> >> +{
> >> +	struct drm_connector *connector;
> >> +
> >> +	connector = drm_connector_find_by_fwnode(connector_fwnode);
> >> +	if (IS_ERR(connector))
> >> +		return;
> >> +
> >> +	if (connector->funcs->oob_hotplug_event)
> >> +		connector->funcs->oob_hotplug_event(connector, data);
> >> +
> >> +	drm_connector_put(connector);
> >> +}
> >> +EXPORT_SYMBOL(drm_connector_oob_hotplug_event);
> > 
> > So it does looks like the "data" parameter is not needed at all:
> 
> Well Imre did indicate that having the number of lanes is useful, so
> for the next version I'll drop the orientation but I plan to keep
> the number of lanes if that is ok with you.
> 
> Not having passing along this info was one of the reasons why my
> previous attempt at this was nacked, so dropping it all together
> feels wrong.

If you need to pass any information to the function, then you need to
pass all the information that we have. Don't start with abstraction.
First create a dedicated API, and then, only if we really have another
user for it, we can add an abstraction layer that both users can use.
All cases are going to be different. We don't know how the abstraction
/ generalisation should look like before we have at least two real
users (ideally more than two, actually). Right now we can not even say
for sure that generalising the API is even possible.

I would not make a huge deal out of this unless I wasn't myself being
told by guys like Greg KH in the past to drop my attempts to
"generalize" things from the beginning when I only had a single user.
By doing so you'll not only force all ends, the source of the data
(the typec drivers in this case) as well as the consumer (i915), to be
always changed together, it will also confuse things. We are not
always going to be able to tell the lane count for example like we can
with USB Type-C, so i915 can't really rely on that information.

Right now we also don't know what exact details i915 (or what ever GPU
driver) needs. We can only say for sure that some details are going to
be needed. Trying to guess and cherry-pick the details now does not
makes sense because of that reason too.

So just make this API USB Type-C DP Alt Mode specific API first, and
supply it everything we have.


thanks,

-- 
heikki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/9] drm/connector: Add support for out-of-band hotplug notification (v2)

[Hans de Goede]

Hi,

On 5/5/21 11:50 AM, Heikki Krogerus wrote:
> On Tue, May 04, 2021 at 05:35:49PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 5/4/21 5:10 PM, Heikki Krogerus wrote:
>>>> +/**
>>>> + * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to connector
>>>> + * @connector: connector to report the event on
>>>> + * @data: data related to the event
>>>> + *
>>>> + * On some hardware a hotplug event notification may come from outside the display
>>>> + * driver / device. An example of this is some USB Type-C setups where the hardware
>>>> + * muxes the DisplayPort data and aux-lines but does not pass the altmode HPD
>>>> + * status bit to the GPU's DP HPD pin.
>>>> + *
>>>> + * This function can be used to report these out-of-band events after obtaining
>>>> + * a drm_connector reference through calling drm_connector_find_by_fwnode().
>>>> + */
>>>> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
>>>> +				     struct drm_connector_oob_hotplug_event_data *data)
>>>> +{
>>>> +	struct drm_connector *connector;
>>>> +
>>>> +	connector = drm_connector_find_by_fwnode(connector_fwnode);
>>>> +	if (IS_ERR(connector))
>>>> +		return;
>>>> +
>>>> +	if (connector->funcs->oob_hotplug_event)
>>>> +		connector->funcs->oob_hotplug_event(connector, data);
>>>> +
>>>> +	drm_connector_put(connector);
>>>> +}
>>>> +EXPORT_SYMBOL(drm_connector_oob_hotplug_event);
>>>
>>> So it does looks like the "data" parameter is not needed at all:
>>
>> Well Imre did indicate that having the number of lanes is useful, so
>> for the next version I'll drop the orientation but I plan to keep
>> the number of lanes if that is ok with you.
>>
>> Not having passing along this info was one of the reasons why my
>> previous attempt at this was nacked, so dropping it all together
>> feels wrong.
> 
> If you need to pass any information to the function, then you need to
> pass all the information that we have. Don't start with abstraction.
> First create a dedicated API, and then, only if we really have another
> user for it, we can add an abstraction layer that both users can use.
> All cases are going to be different. We don't know how the abstraction
> / generalisation should look like before we have at least two real
> users (ideally more than two, actually). Right now we can not even say
> for sure that generalising the API is even possible.
> 
> I would not make a huge deal out of this unless I wasn't myself being
> told by guys like Greg KH in the past to drop my attempts to
> "generalize" things from the beginning when I only had a single user.
> By doing so you'll not only force all ends, the source of the data
> (the typec drivers in this case) as well as the consumer (i915), to be
> always changed together, it will also confuse things. We are not
> always going to be able to tell the lane count for example like we can
> with USB Type-C, so i915 can't really rely on that information.
> 
> Right now we also don't know what exact details i915 (or what ever GPU
> driver) needs. We can only say for sure that some details are going to
> be needed. Trying to guess and cherry-pick the details now does not
> makes sense because of that reason too.
> 
> So just make this API USB Type-C DP Alt Mode specific API first, and
> supply it everything we have.

Hmm, ok I'll just drop the data argument all together for now (as you
already suggested); and then we can see what is best once an actual user
for the info shows up.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Hans de Goede]

From: Heikki Krogerus <heikki.krogerus@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@linux.intel.com>
[hdegoede@redhat.com: Move intel_acpi_assign_connector_fwnodes() to
 intel_acpi.c]
Signed-off-by: Hans de Goede <hdegoede@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;
+
+		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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Andy Shevchenko]

[-- Attachment #1.1: Type: text/plain, Size: 5359 bytes --]

On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com> wrote:

> From: Heikki Krogerus <heikki.krogerus@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@linux.intel.com>
> [hdegoede@redhat.com: Move intel_acpi_assign_connector_fwnodes() to
>  intel_acpi.c]
> Signed-off-by: Hans de Goede <hdegoede@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 ?

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.


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

[-- Attachment #1.2: Type: text/html, Size: 6705 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Heikki Krogerus]

Hi Andy,

> > +/* 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 ?
> 
> 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.

Sorry Andy. This patch is from time before the software nodes
implementation of the get_next_child callback handled the ref counting
properly.

Br,

-- 
heikki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Hans de Goede]

Hi,

On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> 
> 
> On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
>     From: Heikki Krogerus <heikki.krogerus@linux.intel.com <mailto:heikki.krogerus@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@linux.intel.com <mailto:heikki.krogerus@linux.intel.com>>
>     [hdegoede@redhat.com <mailto:hdegoede@redhat.com>: Move intel_acpi_assign_connector_fwnodes() to
>      intel_acpi.c]
>     Signed-off-by: Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@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
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Andy Shevchenko]

On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> > On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:

...

> >     +               fwnode = device_get_next_child_node(kdev, fwnode);

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

Yes, ACPI currently is exceptional, but fwnode API is not.
If you may guarantee that this case won't ever be outside of ACPI and
even though if ACPI won't ever gain a reference counting for fwnodes,
we can leave it as is.

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

It's possible. I dunno if it had to be done earlier. Sakari?

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

We have already issues, so I prefer not to wait for a documentation
update, because for old kernels it will still be an issue.

In any case most of my fixes are against LED subsystem (drivers) and
they are valid due to use in the OF environment.

-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Hans de Goede]

Hi,

On 5/5/21 11:17 AM, Andy Shevchenko wrote:
> On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 5/4/21 9:52 AM, Andy Shevchenko wrote:
>>> On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
> ...
> 
>>>     +               fwnode = device_get_next_child_node(kdev, fwnode);
> 
>>> 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.
> 
> Yes, ACPI currently is exceptional, but fwnode API is not.
> If you may guarantee that this case won't ever be outside of ACPI

Yes I can guarantee that currently this code (which is for the i915
driver only) only deals with ACPI fwnodes.

> and
> even though if ACPI won't ever gain a reference counting for fwnodes,
> we can leave it as is.

Would it not be better to add fake ref-counting to the ACPI fwnode
next_child_node() op though. I believe just getting a reference
on the return value there should work fine; and then all fwnode
implementations would be consistent ?

(note I did not check that the of and swnode code do return
a reference but I would assume so).

>>> 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.
> 
> It's possible. I dunno if it had to be done earlier. Sakari?
> 
>> 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.
> 
> We have already issues, so I prefer not to wait for a documentation
> update, because for old kernels it will still be an issue.

I wonder if we really have issues though, in practice fwnodes are
generated from an devicetree or ACPI tables (or by platform codes
adding swnodes) and then these pretty much stick around for ever.

IOW the initial refcount of 1 is never dropped at least for of-nodes
and ACPI nodes.  I know there are some exceptions like device-tree
overlays which I guess may also be dynamically removed again, but those
exceptions are not widely used.

And if we forget to drop a reference in the worst case we have a small
non-re-occuring (so not growing) memleak. Where as if we start adding
put() calls everywhere we may end up freeing things which are still
in use; or dropping refcounts below 0 triggering WARNs in various
places (IIRC).

So it seems the cure is potentially worse then the disease in this
case.

So if you want to work on this, then IMHO it would be best to first make
sure that all the fwnode implementations behave in the same way wrt
ref-counting, before adding the missing put() calls in various
places.

And once the behavior is consistent then we can also document this
properly making it easier for other people to do the right thing
when using these functions.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Andy Shevchenko]

On Wed, May 5, 2021 at 12:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/5/21 11:17 AM, Andy Shevchenko wrote:
> > On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> >>> On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> >
> > ...
> >
> >>>     +               fwnode = device_get_next_child_node(kdev, fwnode);
> >
> >>> 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.
> >
> > Yes, ACPI currently is exceptional, but fwnode API is not.
> > If you may guarantee that this case won't ever be outside of ACPI
>
> Yes I can guarantee that currently this code (which is for the i915
> driver only) only deals with ACPI fwnodes.
>
> > and
> > even though if ACPI won't ever gain a reference counting for fwnodes,
> > we can leave it as is.
>
> Would it not be better to add fake ref-counting to the ACPI fwnode
> next_child_node() op though. I believe just getting a reference
> on the return value there should work fine; and then all fwnode
> implementations would be consistent ?

But it's already there by absent put/get callbacks. On fwnode level it
is like you described. So, talking for a good pattern we have to call
the fwnode_handle_put() independently and always for for_each_child
and get_next_child usages.

> (note I did not check that the of and swnode code do return
> a reference but I would assume so).

Yes, it's only ACPI that survives w/o reference counting.

> >>> 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.
> >
> > It's possible. I dunno if it had to be done earlier. Sakari?
> >
> >> 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.
> >
> > We have already issues, so I prefer not to wait for a documentation
> > update, because for old kernels it will still be an issue.
>
> I wonder if we really have issues though, in practice fwnodes are
> generated from an devicetree or ACPI tables (or by platform codes
> adding swnodes) and then these pretty much stick around for ever.

Overlays. Not for ever.

> IOW the initial refcount of 1 is never dropped at least for of-nodes
> and ACPI nodes.

>  I know there are some exceptions like device-tree
> overlays which I guess may also be dynamically removed again, but those
> exceptions are not widely used.

ACPI overlays are quite used (at least by two people I know and a few
more that asked questions about them here and there), but luckily it
doesn't require refcounting (yet?).

> And if we forget to drop a reference in the worst case we have a small
> non-re-occuring (so not growing) memleak.

And is it good to provoke all kinds of tools (kmemleak, *SANs, etc)? I
do not think so. If we are writing good code it should be good enough.

> Where as if we start adding
> put() calls everywhere we may end up freeing things which are still
> in use; or dropping refcounts below 0 triggering WARNs in various
> places (IIRC).

Which is good. Then we will discover real issues.

> So it seems the cure is potentially worse then the disease in this
> case.

I tend to disagree with you. How in this case we can go below 0 in
case we know that we took a counter? If somewhere else the code will
do that, it is a problem that has to be fixed on case-by-case basis.

> So if you want to work on this, then IMHO it would be best to first make
> sure that all the fwnode implementations behave in the same way wrt
> ref-counting, before adding the missing put() calls in various
> places.
>
> And once the behavior is consistent

It's consistent now independently of the beneath layer from fwnode API p.o.v.

> then we can also document this
> properly making it easier for other people to do the right thing
> when using these functions.



-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Hans de Goede]

Hi,

On 5/5/21 12:02 PM, Andy Shevchenko wrote:
> On Wed, May 5, 2021 at 12:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> On 5/5/21 11:17 AM, Andy Shevchenko wrote:
>>> On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>> On 5/4/21 9:52 AM, Andy Shevchenko wrote:
>>>>> On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
>>>
>>> ...
>>>
>>>>>     +               fwnode = device_get_next_child_node(kdev, fwnode);
>>>
>>>>> 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.
>>>
>>> Yes, ACPI currently is exceptional, but fwnode API is not.
>>> If you may guarantee that this case won't ever be outside of ACPI
>>
>> Yes I can guarantee that currently this code (which is for the i915
>> driver only) only deals with ACPI fwnodes.
>>
>>> and
>>> even though if ACPI won't ever gain a reference counting for fwnodes,
>>> we can leave it as is.
>>
>> Would it not be better to add fake ref-counting to the ACPI fwnode
>> next_child_node() op though. I believe just getting a reference
>> on the return value there should work fine; and then all fwnode
>> implementations would be consistent ?
> 
> But it's already there by absent put/get callbacks.

Ah, I completely missed that the put/get-s are actually done
through function pointers in fwnode_operations. I assumed that there
was a kref embedded inside the fwnode_handle struct and that they
operated directly on that.

So this whole discussion is entirely based on that misunderstanding,
my bad, sorry.

So yes you are right, things are already consistent thanks to the
absent put/get callbacks.

But we do really need to document the behavior better here
in the kdoc for fwnode_get_next_child_node() and
device_get_next_child_node().

of_get_next_child has this bit, which applies to those too:

 *      Returns a node pointer with refcount incremented, use of_node_put() on
 *      it when done. Returns NULL when prev is the last child. Decrements the
 *      refcount of prev.

I'll prepare a patch to add this to the kdoc for fwnode_get_next_child_node()
and device_get_next_child_node() once I'm done with readying v3 of this series.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Andy Shevchenko]

On Wed, May 5, 2021 at 1:30 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/5/21 12:02 PM, Andy Shevchenko wrote:

...

> But we do really need to document the behavior better here
> in the kdoc for fwnode_get_next_child_node() and
> device_get_next_child_node().

Totally agree!

> of_get_next_child has this bit, which applies to those too:
>
>  *      Returns a node pointer with refcount incremented, use of_node_put() on
>  *      it when done. Returns NULL when prev is the last child. Decrements the
>  *      refcount of prev.
>
> I'll prepare a patch to add this to the kdoc for fwnode_get_next_child_node()
> and device_get_next_child_node() once I'm done with readying v3 of this series.

Thanks!

-- 
With Best Regards,
Andy Shevchenko
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Sakari Ailus]

Hi Andy, Hans,

On Wed, May 05, 2021 at 12:17:14PM +0300, Andy Shevchenko wrote:
> On Wed, May 5, 2021 at 12:07 PM Hans de Goede <hdegoede@redhat.com> wrote:
> > On 5/4/21 9:52 AM, Andy Shevchenko wrote:
> > > On Monday, May 3, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
> 
> ...
> 
> > >     +               fwnode = device_get_next_child_node(kdev, fwnode);
> 
> > > 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.
> 
> Yes, ACPI currently is exceptional, but fwnode API is not.
> If you may guarantee that this case won't ever be outside of ACPI and
> even though if ACPI won't ever gain a reference counting for fwnodes,
> we can leave it as is.
> 
> > > 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.
> 
> It's possible. I dunno if it had to be done earlier. Sakari?

The fwnode API has worked with references for a long time, looks like from
the very beginning of it, as in patch
8a0662d9ed2968e1186208336a8e1fab3fdfea63 .

If you're expecting an fwnode family of function returning another node,
then that function has to have taken a reference to that node before
returning it. Otherwise there's no guarantee that node is still there when
it is returned.

It may be this is not documented for every function doing so. That should
probably be added.

-- 
Kind regards,

Sakari Ailus
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/9] drm/i915/dp: Add support for out-of-bound hotplug events

[Hans de Goede]

On some Cherry Trail devices, DisplayPort over Type-C is supported through
a USB-PD microcontroller (e.g. a fusb302) + a mux to switch the superspeed
datalines between USB-3 and DP (e.g. a pi3usb30532). The kernel in this
case does the PD/alt-mode negotiation itself, rather then everything being
handled in firmware.

So the kernel itself picks an alt-mode, tells the Type-C "dongle" to switch
to DP mode and sets the mux accordingly. In this setup the HPD pin is not
connected, so the i915 driver needs to respond to a software event and scan
the DP port for changes manually.

This commit adds support for this. Together with the recent addition of
DP alt-mode support to the Type-C subsystem this makes DP over Type-C
work on these devices.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 64a73b3ced94..1029720cc945 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5846,6 +5846,18 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn,
 	return intel_modeset_synced_crtcs(state, conn);
 }
 
+static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
+				       struct drm_connector_oob_hotplug_event_data *data)
+{
+	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
+	struct drm_i915_private *i915 = to_i915(connector->dev);
+
+	spin_lock_irq(&i915->irq_lock);
+	i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
+	spin_unlock_irq(&i915->irq_lock);
+	queue_delayed_work(system_wq, &i915->hotplug.hotplug_work, 0);
+}
+
 static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.force = intel_dp_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
@@ -5856,6 +5868,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.destroy = intel_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
+	.oob_hotplug_event = intel_dp_oob_hotplug_event,
 };
 
 static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/9] usb: typec: altmodes/displayport: Make dp_altmode_notify() more generic

[Hans de Goede]

Make dp_altmode_notify() handle the dp->data.conf == 0 case too,
rather then having separate code-paths for this in various places
which call it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/typec/altmodes/displayport.c | 35 +++++++++---------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index b7f094435b00..aa669b9cf70e 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -66,10 +66,17 @@ struct dp_altmode {
 
 static int dp_altmode_notify(struct dp_altmode *dp)
 {
-	u8 state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
+	unsigned long conf;
+	u8 state;
+
+	if (dp->data.conf) {
+		state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
+		conf = TYPEC_MODAL_STATE(state);
+	} else {
+		conf = TYPEC_STATE_USB;
+	}
 
-	return typec_altmode_notify(dp->alt, TYPEC_MODAL_STATE(state),
-				   &dp->data);
+	return typec_altmode_notify(dp->alt, conf, &dp->data);
 }
 
 static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
@@ -137,21 +144,10 @@ static int dp_altmode_status_update(struct dp_altmode *dp)
 
 static int dp_altmode_configured(struct dp_altmode *dp)
 {
-	int ret;
-
 	sysfs_notify(&dp->alt->dev.kobj, "displayport", "configuration");
-
-	if (!dp->data.conf)
-		return typec_altmode_notify(dp->alt, TYPEC_STATE_USB,
-					    &dp->data);
-
-	ret = dp_altmode_notify(dp);
-	if (ret)
-		return ret;
-
 	sysfs_notify(&dp->alt->dev.kobj, "displayport", "pin_assignment");
 
-	return 0;
+	return dp_altmode_notify(dp);
 }
 
 static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
@@ -172,13 +168,8 @@ static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
 	}
 
 	ret = typec_altmode_vdm(dp->alt, header, &conf, 2);
-	if (ret) {
-		if (DP_CONF_GET_PIN_ASSIGN(dp->data.conf))
-			dp_altmode_notify(dp);
-		else
-			typec_altmode_notify(dp->alt, TYPEC_STATE_USB,
-					     &dp->data);
-	}
+	if (ret)
+		dp_altmode_notify(dp);
 
 	return ret;
 }
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] usb: typec: altmodes/displayport: Notify drm subsys of hotplug events

[Hans de Goede]

Use the new drm_connector_find_by_fwnode() and
drm_connector_oob_hotplug_event() functions to let drm/kms drivers
know about DisplayPort over Type-C hotplug events.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Add missing depends on DRM to TYPEC_DP_ALTMODE Kconfig entry
---
 drivers/usb/typec/altmodes/Kconfig       |  1 +
 drivers/usb/typec/altmodes/displayport.c | 40 +++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/altmodes/Kconfig b/drivers/usb/typec/altmodes/Kconfig
index 60d375e9c3c7..1a6b5e872b0d 100644
--- a/drivers/usb/typec/altmodes/Kconfig
+++ b/drivers/usb/typec/altmodes/Kconfig
@@ -4,6 +4,7 @@ menu "USB Type-C Alternate Mode drivers"
 
 config TYPEC_DP_ALTMODE
 	tristate "DisplayPort Alternate Mode driver"
+	depends on DRM
 	help
 	  DisplayPort USB Type-C Alternate Mode allows DisplayPort
 	  displays and adapters to be attached to the USB Type-C
diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index aa669b9cf70e..f00dfc5c14b6 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -11,8 +11,10 @@
 #include <linux/delay.h>
 #include <linux/mutex.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/usb/pd_vdo.h>
 #include <linux/usb/typec_dp.h>
+#include <drm/drm_connector.h>
 #include "displayport.h"
 
 #define DP_HEADER(_dp, ver, cmd)	(VDO((_dp)->alt->svid, 1, ver, cmd)	\
@@ -62,12 +64,30 @@ struct dp_altmode {
 	struct work_struct work;
 	struct typec_altmode *alt;
 	const struct typec_altmode *port;
+	struct fwnode_handle *connector_fwnode;
 };
 
+static void dp_altmode_notify_connector(struct dp_altmode *dp)
+{
+	struct drm_connector_oob_hotplug_event_data data = { };
+	u8 pin_assign = DP_CONF_GET_PIN_ASSIGN(dp->data.conf);
+
+	data.connected = dp->data.status & DP_STATUS_HPD_STATE;
+	data.orientation = typec_altmode_get_orientation(dp->alt);
+
+	if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
+		data.dp_lanes = 4;
+	else if (pin_assign & DP_PIN_ASSIGN_MULTI_FUNC_MASK)
+		data.dp_lanes = 2;
+
+	drm_connector_oob_hotplug_event(dp->connector_fwnode, &data);
+}
+
 static int dp_altmode_notify(struct dp_altmode *dp)
 {
 	unsigned long conf;
 	u8 state;
+	int ret;
 
 	if (dp->data.conf) {
 		state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
@@ -76,7 +96,12 @@ static int dp_altmode_notify(struct dp_altmode *dp)
 		conf = TYPEC_STATE_USB;
 	}
 
-	return typec_altmode_notify(dp->alt, conf, &dp->data);
+	ret = typec_altmode_notify(dp->alt, conf, &dp->data);
+	if (ret)
+		return ret;
+
+	dp_altmode_notify_connector(dp);
+	return 0;
 }
 
 static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
@@ -512,6 +537,7 @@ static const struct attribute_group dp_altmode_group = {
 int dp_altmode_probe(struct typec_altmode *alt)
 {
 	const struct typec_altmode *port = typec_altmode_get_partner(alt);
+	struct fwnode_handle *fwnode;
 	struct dp_altmode *dp;
 	int ret;
 
@@ -540,6 +566,11 @@ int dp_altmode_probe(struct typec_altmode *alt)
 	alt->desc = "DisplayPort";
 	alt->ops = &dp_altmode_ops;
 
+	fwnode = dev_fwnode(alt->dev.parent->parent); /* typec_port fwnode */
+	dp->connector_fwnode = fwnode_find_reference(fwnode, "displayport", 0);
+	if (IS_ERR(dp->connector_fwnode))
+		dp->connector_fwnode = NULL;
+
 	typec_altmode_set_drvdata(alt, dp);
 
 	dp->state = DP_STATE_ENTER;
@@ -555,6 +586,13 @@ void dp_altmode_remove(struct typec_altmode *alt)
 
 	sysfs_remove_group(&alt->dev.kobj, &dp_altmode_group);
 	cancel_work_sync(&dp->work);
+
+	if (dp->connector_fwnode) {
+		dp->data.conf = 0;
+		dp->data.status = 0;
+		dp_altmode_notify_connector(dp);
+		fwnode_handle_put(dp->connector_fwnode);
+	}
 }
 EXPORT_SYMBOL_GPL(dp_altmode_remove);
 
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] usb: typec: altmodes/displayport: Notify drm subsys of hotplug events

[Heikki Krogerus]

Hi Hans,

On Mon, May 03, 2021 at 05:46:46PM +0200, Hans de Goede wrote:
> Use the new drm_connector_find_by_fwnode() and
> drm_connector_oob_hotplug_event() functions to let drm/kms drivers
> know about DisplayPort over Type-C hotplug events.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - Add missing depends on DRM to TYPEC_DP_ALTMODE Kconfig entry
> ---
>  drivers/usb/typec/altmodes/Kconfig       |  1 +
>  drivers/usb/typec/altmodes/displayport.c | 40 +++++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/altmodes/Kconfig b/drivers/usb/typec/altmodes/Kconfig
> index 60d375e9c3c7..1a6b5e872b0d 100644
> --- a/drivers/usb/typec/altmodes/Kconfig
> +++ b/drivers/usb/typec/altmodes/Kconfig
> @@ -4,6 +4,7 @@ menu "USB Type-C Alternate Mode drivers"
>  
>  config TYPEC_DP_ALTMODE
>  	tristate "DisplayPort Alternate Mode driver"
> +	depends on DRM
>  	help
>  	  DisplayPort USB Type-C Alternate Mode allows DisplayPort
>  	  displays and adapters to be attached to the USB Type-C
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index aa669b9cf70e..f00dfc5c14b6 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -11,8 +11,10 @@
>  #include <linux/delay.h>
>  #include <linux/mutex.h>
>  #include <linux/module.h>
> +#include <linux/property.h>
>  #include <linux/usb/pd_vdo.h>
>  #include <linux/usb/typec_dp.h>
> +#include <drm/drm_connector.h>
>  #include "displayport.h"
>  
>  #define DP_HEADER(_dp, ver, cmd)	(VDO((_dp)->alt->svid, 1, ver, cmd)	\
> @@ -62,12 +64,30 @@ struct dp_altmode {
>  	struct work_struct work;
>  	struct typec_altmode *alt;
>  	const struct typec_altmode *port;
> +	struct fwnode_handle *connector_fwnode;
>  };
>  
> +static void dp_altmode_notify_connector(struct dp_altmode *dp)
> +{
> +	struct drm_connector_oob_hotplug_event_data data = { };
> +	u8 pin_assign = DP_CONF_GET_PIN_ASSIGN(dp->data.conf);
> +
> +	data.connected = dp->data.status & DP_STATUS_HPD_STATE;
> +	data.orientation = typec_altmode_get_orientation(dp->alt);
> +
> +	if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
> +		data.dp_lanes = 4;
> +	else if (pin_assign & DP_PIN_ASSIGN_MULTI_FUNC_MASK)
> +		data.dp_lanes = 2;
> +
> +	drm_connector_oob_hotplug_event(dp->connector_fwnode, &data);
> +}
> +
>  static int dp_altmode_notify(struct dp_altmode *dp)
>  {
>  	unsigned long conf;
>  	u8 state;
> +	int ret;
>  
>  	if (dp->data.conf) {
>  		state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
> @@ -76,7 +96,12 @@ static int dp_altmode_notify(struct dp_altmode *dp)
>  		conf = TYPEC_STATE_USB;
>  	}
>  
> -	return typec_altmode_notify(dp->alt, conf, &dp->data);
> +	ret = typec_altmode_notify(dp->alt, conf, &dp->data);
> +	if (ret)
> +		return ret;
> +
> +	dp_altmode_notify_connector(dp);

That is now going to be always called after configuration, right? The
HPD is not necessarily issued at that point.

I think that should be called from dp_altmode_status_update(), and
probable only if there really is HPD IRQ or HPD State changes... I
think?

> +	return 0;
>  }

thanks,

-- 
heikki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 8/9] usb: typec: altmodes/displayport: Notify drm subsys of hotplug events

[Hans de Goede]

Hi Heikki,

On 5/5/21 12:17 PM, Heikki Krogerus wrote:
> Hi Hans,
> 
> On Mon, May 03, 2021 at 05:46:46PM +0200, Hans de Goede wrote:
>> Use the new drm_connector_find_by_fwnode() and
>> drm_connector_oob_hotplug_event() functions to let drm/kms drivers
>> know about DisplayPort over Type-C hotplug events.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> - Add missing depends on DRM to TYPEC_DP_ALTMODE Kconfig entry
>> ---
>>  drivers/usb/typec/altmodes/Kconfig       |  1 +
>>  drivers/usb/typec/altmodes/displayport.c | 40 +++++++++++++++++++++++-
>>  2 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/typec/altmodes/Kconfig b/drivers/usb/typec/altmodes/Kconfig
>> index 60d375e9c3c7..1a6b5e872b0d 100644
>> --- a/drivers/usb/typec/altmodes/Kconfig
>> +++ b/drivers/usb/typec/altmodes/Kconfig
>> @@ -4,6 +4,7 @@ menu "USB Type-C Alternate Mode drivers"
>>  
>>  config TYPEC_DP_ALTMODE
>>  	tristate "DisplayPort Alternate Mode driver"
>> +	depends on DRM
>>  	help
>>  	  DisplayPort USB Type-C Alternate Mode allows DisplayPort
>>  	  displays and adapters to be attached to the USB Type-C
>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>> index aa669b9cf70e..f00dfc5c14b6 100644
>> --- a/drivers/usb/typec/altmodes/displayport.c
>> +++ b/drivers/usb/typec/altmodes/displayport.c
>> @@ -11,8 +11,10 @@
>>  #include <linux/delay.h>
>>  #include <linux/mutex.h>
>>  #include <linux/module.h>
>> +#include <linux/property.h>
>>  #include <linux/usb/pd_vdo.h>
>>  #include <linux/usb/typec_dp.h>
>> +#include <drm/drm_connector.h>
>>  #include "displayport.h"
>>  
>>  #define DP_HEADER(_dp, ver, cmd)	(VDO((_dp)->alt->svid, 1, ver, cmd)	\
>> @@ -62,12 +64,30 @@ struct dp_altmode {
>>  	struct work_struct work;
>>  	struct typec_altmode *alt;
>>  	const struct typec_altmode *port;
>> +	struct fwnode_handle *connector_fwnode;
>>  };
>>  
>> +static void dp_altmode_notify_connector(struct dp_altmode *dp)
>> +{
>> +	struct drm_connector_oob_hotplug_event_data data = { };
>> +	u8 pin_assign = DP_CONF_GET_PIN_ASSIGN(dp->data.conf);
>> +
>> +	data.connected = dp->data.status & DP_STATUS_HPD_STATE;
>> +	data.orientation = typec_altmode_get_orientation(dp->alt);
>> +
>> +	if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
>> +		data.dp_lanes = 4;
>> +	else if (pin_assign & DP_PIN_ASSIGN_MULTI_FUNC_MASK)
>> +		data.dp_lanes = 2;
>> +
>> +	drm_connector_oob_hotplug_event(dp->connector_fwnode, &data);
>> +}
>> +
>>  static int dp_altmode_notify(struct dp_altmode *dp)
>>  {
>>  	unsigned long conf;
>>  	u8 state;
>> +	int ret;
>>  
>>  	if (dp->data.conf) {
>>  		state = get_count_order(DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
>> @@ -76,7 +96,12 @@ static int dp_altmode_notify(struct dp_altmode *dp)
>>  		conf = TYPEC_STATE_USB;
>>  	}
>>  
>> -	return typec_altmode_notify(dp->alt, conf, &dp->data);
>> +	ret = typec_altmode_notify(dp->alt, conf, &dp->data);
>> +	if (ret)
>> +		return ret;
>> +
>> +	dp_altmode_notify_connector(dp);
> 
> That is now going to be always called after configuration, right? The
> HPD is not necessarily issued at that point.
> 
> I think that should be called from dp_altmode_status_update(), and
> probable only if there really is HPD IRQ or HPD State changes... I
> think?

I did see this triggering a bit more often then necessary on the initial
plugin of a DP-altmode capable Type-C "dongle", so what you are suggesting
makes sense. I'll come up with a better approach for the next version.

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] platform/x86/intel_cht_int33fe: Correct "displayport" fwnode reference

[Hans de Goede]

The Type-C connector on these devices is connected to DP-2 not DP-1,
so the reference must be to the DD04 child-node of the GPU, rather
then the DD02 child-node.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/intel_cht_int33fe_typec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe_typec.c b/drivers/platform/x86/intel_cht_int33fe_typec.c
index b61bad9cc8d2..d59544167430 100644
--- a/drivers/platform/x86/intel_cht_int33fe_typec.c
+++ b/drivers/platform/x86/intel_cht_int33fe_typec.c
@@ -168,8 +168,8 @@ static int cht_int33fe_setup_dp(struct cht_int33fe_data *data)
 		return -ENODEV;
 	}
 
-	/* Then the DP child device node */
-	data->dp = device_get_named_child_node(&pdev->dev, "DD02");
+	/* Then the DP-2 child device node */
+	data->dp = device_get_named_child_node(&pdev->dev, "DD04");
 	pci_dev_put(pdev);
 	if (!data->dp)
 		return -ENODEV;
-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 9/9] platform/x86/intel_cht_int33fe: Correct "displayport" fwnode reference

[Hans de Goede]

Hi,

On 5/3/21 5:46 PM, Hans de Goede wrote:
> The Type-C connector on these devices is connected to DP-2 not DP-1,
> so the reference must be to the DD04 child-node of the GPU, rather
> then the DD02 child-node.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Since this is pretty much independent from the rest of the series,
I'll take this upstream through the pdx86 tree.

I've added this to my review-hans branch now, and it will get added
to for-next from there.

Regards,

Hans



> ---
>  drivers/platform/x86/intel_cht_int33fe_typec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_cht_int33fe_typec.c b/drivers/platform/x86/intel_cht_int33fe_typec.c
> index b61bad9cc8d2..d59544167430 100644
> --- a/drivers/platform/x86/intel_cht_int33fe_typec.c
> +++ b/drivers/platform/x86/intel_cht_int33fe_typec.c
> @@ -168,8 +168,8 @@ static int cht_int33fe_setup_dp(struct cht_int33fe_data *data)
>  		return -ENODEV;
>  	}
>  
> -	/* Then the DP child device node */
> -	data->dp = device_get_named_child_node(&pdev->dev, "DD02");
> +	/* Then the DP-2 child device node */
> +	data->dp = device_get_named_child_node(&pdev->dev, "DD04");
>  	pci_dev_put(pdev);
>  	if (!data->dp)
>  		return -ENODEV;
> 

Re: [PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification (v2)

[Heikki Krogerus]

On Mon, May 03, 2021 at 05:46:38PM +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of my work on making DP over Type-C work on devices where the
> Type-C controller does not drive the HPD pin on the GPU, but instead
> we need to forward HPD events from the Type-C controller to the DRM driver.
> 
> Changes in v2:
> - Replace the bogus "drm/connector: Make the drm_sysfs connector->kdev
>   device hold a reference to the connector" patch with:
>   "drm/connector: Give connector sysfs devices there own device_type"
>   the new patch is a dep for patch 2/9 see the patches
> 
> - Stop using a class-dev-iter, instead at a global connector list
>   to drm_connector.c and use that to find the connector by the fwnode,
>   similar to how we already do this in drm_panel.c and drm_bridge.c
> 
> - Make drm_connector_oob_hotplug_event() take a fwnode pointer as
>   argument, rather then a drm_connector pointer and let it do the
>   lookup itself. This allows making drm_connector_find_by_fwnode() a
>   drm-internal function and avoids code outside the drm subsystem
>   potentially holding on the a drm_connector reference for a longer
>   period.
> 
> This series not only touches drm subsys files but it also touches
> drivers/usb/typec/altmodes/typec_displayport.c, that file usually
> does not see a whole lot of changes. So I believe it would be best
> to just merge the entire series through drm-misc, Assuming we can
> get an ack from Greg for merging the typec_displayport.c changes
> this way.
> 
> ### 
> 
> As already mentioned in the v1 cover-letter this series replaces
> a previous attempt from quite some time ago. 
> For anyone interested here are the old (2019!) patches for this:
> 
> https://patchwork.freedesktop.org/patch/288491/
> https://patchwork.freedesktop.org/patch/288493/
> https://patchwork.freedesktop.org/patch/288495/
> 
> Last time I posted this the biggest change requested was for more info to
> be included in the event send to the DRM-subsystem, specifically sending
> the following info was requested:
> 
> 1. Which DP connector on the GPU the event is for
> 2. How many lanes are available
> 3. Connector orientation
> 
> This series is basically an entirely new approach, which no longer
> uses the notifier framework at all. Instead the Type-C code looksup
> a connector based on a fwnode (this was suggested by Heikki Krogerus)
> and then calls a new oob_hotplug_event drm_connector_func directly
> on the connector, passing the requested info as argument.
> 
> Info such as the orientation and the number of dp-lanes is now passed
> to the drm_connector_oob_hotplug_event() function as requested in the
> review of the old code, but nothing is done with it for now.
> Using this info falls well outside of my knowledge of the i915 driver
> so this is left to a follow-up patch (I will be available to test
> patches for this).

Thanks for taking care of these! It's really great that you spent the
time to do this series. I'm already thinking about what we can add
after these are in. I think support for re-configuration, so support
for changing the pin-configuration in runtime is going to be needed
soon after these. But first things first (sorry, I'm getting ahead of
myself).


thanks,

-- 
heikki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/9] drm + usb-type-c: Add support for out-of-band hotplug notification

[Hans de Goede]

Hi All,

Here is a new attempt to make DP over Type-C work on devices where the
Type-C controller does not drive the HPD pin on the GPU, but instead
we need to forward HPD events from the Type-C controller to the DRM driver.

For anyone interested here are the old (2019!) patches for this:

https://patchwork.freedesktop.org/patch/288491/
https://patchwork.freedesktop.org/patch/288493/
https://patchwork.freedesktop.org/patch/288495/

Last time I posted this the biggest change requested was for more info to
be included in the event send to the DRM-subsystem, specifically sending
the following info was requested:

1. Which DP connector on the GPU the event is for
2. How many lanes are available
3. Connector orientation

This series is basically an entirely new approach, which no longer
uses the notifier framework at all. Instead the Type-C code looksup
a connector based on a fwnode (this was suggested by Heikki Krogerus)
and then calls a new oob_hotplug_event drm_connector_func directly
on the connector, passing the requested info as argument.

This series not only touches drm subsys files but it also touches
drivers/usb/typec/altmodes/typec_displayport.c, that file usually
does not see a whole lot of changes. So I believe it would be best
to just merge the entire series through drm-misc, Assuming we can
get an ack from Greg for merging the typec_displayport.c changes
this way.

Regards,

Hans


Hans de Goede (8):
  drm/connector: Make the drm_sysfs connector->kdev device hold a
    reference to the connector
  drm/connector: Add a fwnode pointer to drm_connector and register with
    ACPI
  drm/connector: Add drm_connector_find_by_fwnode() function
  drm/connector: Add support for out-of-band hotplug notification
  drm/i915/dp: Add support for out-of-bound hotplug events
  usb: typec: altmodes/displayport: Make dp_altmode_notify() more
    generic
  usb: typec: altmodes/displayport: Notify drm subsys of hotplug events
  platform/x86/intel_cht_int33fe: Correct "displayport" fwnode reference

Heikki Krogerus (1):
  drm/i915: Associate ACPI connector nodes with connector entries

 drivers/gpu/drm/drm_connector.c               |  20 +++
 drivers/gpu/drm/drm_sysfs.c                   | 129 ++++++++++++++++--
 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 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  13 ++
 .../platform/x86/intel_cht_int33fe_typec.c    |   4 +-
 drivers/usb/typec/altmodes/displayport.c      |  78 ++++++++---
 include/drm/drm_connector.h                   |  36 +++++
 9 files changed, 292 insertions(+), 32 deletions(-)

-- 
2.31.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] drm/i915: Associate ACPI connector nodes with connector entries

[Hans de Goede]

From: Heikki Krogerus <heikki.krogerus@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@linux.intel.com>
[hdegoede@redhat.com: Move intel_acpi_assign_connector_fwnodes() to
 intel_acpi.c]
Signed-off-by: Hans de Goede <hdegoede@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;
+
+		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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel