[PATCH 0/2] Associate ddc adapters with connectors

[PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

Andrzej Pietrasiewicz (2):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs

 drivers/gpu/drm/drm_sysfs.c          |  9 +++++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 include/drm/drm_connector.h          | 11 +++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

Andrzej Pietrasiewicz (2):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs

 drivers/gpu/drm/drm_sysfs.c          |  9 +++++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 include/drm/drm_connector.h          | 11 +++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

Andrzej Pietrasiewicz (2):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs

 drivers/gpu/drm/drm_sysfs.c          |  9 +++++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 include/drm/drm_connector.h          | 11 +++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_sysfs.c |  9 +++++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..627f8ebfc87a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,10 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj,
+				 connector->ddc->dev.kobj.name);
 	return 0;
 }
 
@@ -301,6 +305,11 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj,
+				  connector->ddc->dev.kobj.name);
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1

[PATCH 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_sysfs.c |  9 +++++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..627f8ebfc87a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,10 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj,
+				 connector->ddc->dev.kobj.name);
 	return 0;
 }
 
@@ -301,6 +305,11 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj,
+				  connector->ddc->dev.kobj.name);
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1

[PATCH 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_sysfs.c |  9 +++++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..627f8ebfc87a 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,10 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj,
+				 connector->ddc->dev.kobj.name);
 	return 0;
 }
 
@@ -301,6 +305,11 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj,
+				  connector->ddc->dev.kobj.name);
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1

[PATCH 2/2] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1

[PATCH 2/2] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> 	-> ../../../../soc/13880000.i2c/i2c-2

Don't you want the symlink name to be "i2c" or something fixed, rather
than the name of the i2c adapter?  Otherwise, you seem to be encumbering
userspace with searching the directory to try and find the symlink.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> 	-> ../../../../soc/13880000.i2c/i2c-2

Don't you want the symlink name to be "i2c" or something fixed, rather
than the name of the i2c adapter?  Otherwise, you seem to be encumbering
userspace with searching the directory to try and find the symlink.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

Hi Russell,

W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
>> It is difficult for a user to know which of the i2c adapters is for which
>> drm connector. This series addresses this problem.
>>
>> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
>>
>> ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
>> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
>> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> Don't you want the symlink name to be "i2c" or something fixed, rather
> than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> userspace with searching the directory to try and find the symlink.
> 

Thank you for your comment. So you imagine something on the lines of:

lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
  	-> ../../../../soc/13880000.i2c/i2c-2

?

This makes sense to me, I will send a v2.

Andrzej

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

Hi Russell,

W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
>> It is difficult for a user to know which of the i2c adapters is for which
>> drm connector. This series addresses this problem.
>>
>> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
>>
>> ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
>> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
>> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> Don't you want the symlink name to be "i2c" or something fixed, rather
> than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> userspace with searching the directory to try and find the symlink.
> 

Thank you for your comment. So you imagine something on the lines of:

lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
  	-> ../../../../soc/13880000.i2c/i2c-2

?

This makes sense to me, I will send a v2.

Andrzej

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 12:14:27PM +0200, Andrzej Pietrasiewicz wrote:
> Hi Russell,
> 
> W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > It is difficult for a user to know which of the i2c adapters is for which
> > > drm connector. This series addresses this problem.
> > > 
> > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > 
> > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > Don't you want the symlink name to be "i2c" or something fixed, rather
> > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > userspace with searching the directory to try and find the symlink.
> > 
> 
> Thank you for your comment. So you imagine something on the lines of:
> 
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>  	-> ../../../../soc/13880000.i2c/i2c-2
> 
> ?

Exactly.

> This makes sense to me, I will send a v2.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 12:14:27PM +0200, Andrzej Pietrasiewicz wrote:
> Hi Russell,
> 
> W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > It is difficult for a user to know which of the i2c adapters is for which
> > > drm connector. This series addresses this problem.
> > > 
> > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > 
> > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > Don't you want the symlink name to be "i2c" or something fixed, rather
> > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > userspace with searching the directory to try and find the symlink.
> > 
> 
> Thank you for your comment. So you imagine something on the lines of:
> 
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>  	-> ../../../../soc/13880000.i2c/i2c-2
> 
> ?

Exactly.

> This makes sense to me, I will send a v2.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

v1..v2:

- used fixed name "ddc" for the symbolic link in order to make it easy for
userspace to find the i2c adapter

Andrzej Pietrasiewicz (2):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs

 drivers/gpu/drm/drm_sysfs.c          |  7 +++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 include/drm/drm_connector.h          | 11 +++++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH v2 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

v1..v2:

- used fixed name "ddc" for the symbolic link in order to make it easy for
userspace to find the i2c adapter

Andrzej Pietrasiewicz (2):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs

 drivers/gpu/drm/drm_sysfs.c          |  7 +++++++
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 include/drm/drm_connector.h          | 11 +++++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_sysfs.c |  7 +++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..26d359b39785 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
 }
 
@@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj, "ddc");
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1

[PATCH v2 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/drm_sysfs.c |  7 +++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..26d359b39785 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
 }
 
@@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj, "ddc");
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/2] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1

[PATCH v2 2/2] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Emil Velikov]

On 2019/06/25, Andrzej Pietrasiewicz wrote:
> Hi Russell,
> 
> W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > It is difficult for a user to know which of the i2c adapters is for which
> > > drm connector. This series addresses this problem.
> > > 
> > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > 
> > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > Don't you want the symlink name to be "i2c" or something fixed, rather
> > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > userspace with searching the directory to try and find the symlink.
> > 
> 
> Thank you for your comment. So you imagine something on the lines of:
> 
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>  	-> ../../../../soc/13880000.i2c/i2c-2
> 
> ?
> 
Fwiw my Intel machine lists a number of i2c devices:
/sys/class/drm/card0-DP-1/i2c-6
/sys/class/drm/card0-DP-2/i2c-7
/sys/class/drm/card0-eDP-1/i2c-5

Note: I haven't looked _if_ they relate to ones you're proposing here.

One thing worth mentioning is, the ones I've seen are not symlinks to
another sysfs entries. And there aren't any i2c nodes in /dev ...

Just a random food for thought :-)

HTH
Emil

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Emil Velikov]

On 2019/06/25, Andrzej Pietrasiewicz wrote:
> Hi Russell,
> 
> W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > It is difficult for a user to know which of the i2c adapters is for which
> > > drm connector. This series addresses this problem.
> > > 
> > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > 
> > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > Don't you want the symlink name to be "i2c" or something fixed, rather
> > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > userspace with searching the directory to try and find the symlink.
> > 
> 
> Thank you for your comment. So you imagine something on the lines of:
> 
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
>  	-> ../../../../soc/13880000.i2c/i2c-2
> 
> ?
> 
Fwiw my Intel machine lists a number of i2c devices:
/sys/class/drm/card0-DP-1/i2c-6
/sys/class/drm/card0-DP-2/i2c-7
/sys/class/drm/card0-eDP-1/i2c-5

Note: I haven't looked _if_ they relate to ones you're proposing here.

One thing worth mentioning is, the ones I've seen are not symlinks to
another sysfs entries. And there aren't any i2c nodes in /dev ...

Just a random food for thought :-)

HTH
Emil





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Daniel Vetter]

On Tue, Jun 25, 2019 at 02:36:39PM +0100, Emil Velikov wrote:
> On 2019/06/25, Andrzej Pietrasiewicz wrote:
> > Hi Russell,
> > 
> > W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > > It is difficult for a user to know which of the i2c adapters is for which
> > > > drm connector. This series addresses this problem.
> > > > 
> > > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > > 
> > > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > > 
> > > Don't you want the symlink name to be "i2c" or something fixed, rather
> > > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > > userspace with searching the directory to try and find the symlink.
> > > 
> > 
> > Thank you for your comment. So you imagine something on the lines of:
> > 
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >  	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > ?
> > 
> Fwiw my Intel machine lists a number of i2c devices:
> /sys/class/drm/card0-DP-1/i2c-6
> /sys/class/drm/card0-DP-2/i2c-7
> /sys/class/drm/card0-eDP-1/i2c-5
> 
> Note: I haven't looked _if_ they relate to ones you're proposing here.
> 
> One thing worth mentioning is, the ones I've seen are not symlinks to
> another sysfs entries. And there aren't any i2c nodes in /dev ...
> 
> Just a random food for thought :-)

Those are the i2c-over-dp-aux controllers. I think we want to list these
too.

Btw to make this more useful maybe some default implementations for
get_modes which automatically dtrt, as a helper? Probably could use that
to squash quite a bit of boilerplate.

Otherwise I like this. Biggest problem I'm seeing here is rolling this out
everywhere, this is a lot of work. And without widespread adoptions it's
not terribly useful for userspace.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Daniel Vetter]

On Tue, Jun 25, 2019 at 02:36:39PM +0100, Emil Velikov wrote:
> On 2019/06/25, Andrzej Pietrasiewicz wrote:
> > Hi Russell,
> > 
> > W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > > It is difficult for a user to know which of the i2c adapters is for which
> > > > drm connector. This series addresses this problem.
> > > > 
> > > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > > 
> > > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > > 
> > > Don't you want the symlink name to be "i2c" or something fixed, rather
> > > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > > userspace with searching the directory to try and find the symlink.
> > > 
> > 
> > Thank you for your comment. So you imagine something on the lines of:
> > 
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >  	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > ?
> > 
> Fwiw my Intel machine lists a number of i2c devices:
> /sys/class/drm/card0-DP-1/i2c-6
> /sys/class/drm/card0-DP-2/i2c-7
> /sys/class/drm/card0-eDP-1/i2c-5
> 
> Note: I haven't looked _if_ they relate to ones you're proposing here.
> 
> One thing worth mentioning is, the ones I've seen are not symlinks to
> another sysfs entries. And there aren't any i2c nodes in /dev ...
> 
> Just a random food for thought :-)

Those are the i2c-over-dp-aux controllers. I think we want to list these
too.

Btw to make this more useful maybe some default implementations for
get_modes which automatically dtrt, as a helper? Probably could use that
to squash quite a bit of boilerplate.

Otherwise I like this. Biggest problem I'm seeing here is rolling this out
everywhere, this is a lot of work. And without widespread adoptions it's
not terribly useful for userspace.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Daniel Vetter]

On Tue, Jun 25, 2019 at 02:36:39PM +0100, Emil Velikov wrote:
> On 2019/06/25, Andrzej Pietrasiewicz wrote:
> > Hi Russell,
> > 
> > W dniu 25.06.2019 o 12:03, Russell King - ARM Linux admin pisze:
> > > On Tue, Jun 25, 2019 at 11:46:34AM +0200, Andrzej Pietrasiewicz wrote:
> > > > It is difficult for a user to know which of the i2c adapters is for which
> > > > drm connector. This series addresses this problem.
> > > > 
> > > > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> > > > 
> > > > ls -l /sys/class/drm/card0-HDMI-A-1/i2c-2
> > > > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/i2c-2 \
> > > > 	-> ../../../../soc/13880000.i2c/i2c-2
> > > 
> > > Don't you want the symlink name to be "i2c" or something fixed, rather
> > > than the name of the i2c adapter?  Otherwise, you seem to be encumbering
> > > userspace with searching the directory to try and find the symlink.
> > > 
> > 
> > Thank you for your comment. So you imagine something on the lines of:
> > 
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >  	-> ../../../../soc/13880000.i2c/i2c-2
> > 
> > ?
> > 
> Fwiw my Intel machine lists a number of i2c devices:
> /sys/class/drm/card0-DP-1/i2c-6
> /sys/class/drm/card0-DP-2/i2c-7
> /sys/class/drm/card0-eDP-1/i2c-5
> 
> Note: I haven't looked _if_ they relate to ones you're proposing here.
> 
> One thing worth mentioning is, the ones I've seen are not symlinks to
> another sysfs entries. And there aren't any i2c nodes in /dev ...
> 
> Just a random food for thought :-)

Those are the i2c-over-dp-aux controllers. I think we want to list these
too.

Btw to make this more useful maybe some default implementations for
get_modes which automatically dtrt, as a helper? Probably could use that
to squash quite a bit of boilerplate.

Otherwise I like this. Biggest problem I'm seeing here is rolling this out
everywhere, this is a lot of work. And without widespread adoptions it's
not terribly useful for userspace.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
> Otherwise I like this. Biggest problem I'm seeing here is rolling this out
> everywhere, this is a lot of work. And without widespread adoptions it's
> not terribly useful for userspace.

There will be cases where it's not possible, because the I2C bus is
hidden behind a chip that doesn't give you direct access to the DDC
bus.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Russell King - ARM Linux admin]

On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
> Otherwise I like this. Biggest problem I'm seeing here is rolling this out
> everywhere, this is a lot of work. And without widespread adoptions it's
> not terribly useful for userspace.

There will be cases where it's not possible, because the I2C bus is
hidden behind a chip that doesn't give you direct access to the DDC
bus.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Daniel Vetter]

On Tue, Jun 25, 2019 at 03:10:32PM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
> > Otherwise I like this. Biggest problem I'm seeing here is rolling this out
> > everywhere, this is a lot of work. And without widespread adoptions it's
> > not terribly useful for userspace.
> 
> There will be cases where it's not possible, because the I2C bus is
> hidden behind a chip that doesn't give you direct access to the DDC
> bus.

Oh sure, plus lots of connectors where there's just not ddc bus at all.
But if we only roll this out for a handful of drivers it's also not great,
that's what I meant. Looking at

$ git grep drm_do_get_edid

there's only very few drivers where the ddc bus is hidden. There's a lot
more where it's not, and I think a big series to tackle those would serve
extremely well to make a case for this sysfs link.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Daniel Vetter]

On Tue, Jun 25, 2019 at 03:10:32PM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
> > Otherwise I like this. Biggest problem I'm seeing here is rolling this out
> > everywhere, this is a lot of work. And without widespread adoptions it's
> > not terribly useful for userspace.
> 
> There will be cases where it's not possible, because the I2C bus is
> hidden behind a chip that doesn't give you direct access to the DDC
> bus.

Oh sure, plus lots of connectors where there's just not ddc bus at all.
But if we only roll this out for a handful of drivers it's also not great,
that's what I meant. Looking at

$ git grep drm_do_get_edid

there's only very few drivers where the ddc bus is hidden. There's a lot
more where it's not, and I think a big series to tackle those would serve
extremely well to make a case for this sysfs link.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Daniel Vetter]

On Tue, Jun 25, 2019 at 12:24:40PM +0200, Andrzej Pietrasiewicz wrote:
> Add generic code which creates symbolic links in sysfs, pointing to ddc
> interface used by a particular video output. For example:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> This makes it easy for user to associate a display with its ddc adapter
> and use e.g. ddcutil to control the chosen monitor.
> 
> This patch adds an i2c_adapter pointer to struct drm_connector. Particular
> drivers can then use it instead of using their own private instance. If a
> connector contains a ddc, then create a symbolic link in sysfs.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

As mentioned in the other subthread, I think the important bit to sell
this is rolling it out to as many drivers as feasible and collecting all
the acks from driver maintainers.
-Daniel
> ---
>  drivers/gpu/drm/drm_sysfs.c |  7 +++++++
>  include/drm/drm_connector.h | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..26d359b39785 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	/* Let userspace know we have a new connector */
>  	drm_sysfs_hotplug_event(dev);
>  
> +	if (connector->ddc)
> +		return sysfs_create_link(&connector->kdev->kobj,
> +				 &connector->ddc->dev.kobj, "ddc");
>  	return 0;
>  }
>  
> @@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  {
>  	if (!connector->kdev)
>  		return;
> +
> +	if (connector->ddc)
> +		sysfs_remove_link(&connector->kdev->kobj, "ddc");
> +
>  	DRM_DEBUG("removing \"%s\" from sysfs\n",
>  		  connector->name);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ca745d9feaf5..1ad3d1d54ba7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -23,6 +23,7 @@
>  #ifndef __DRM_CONNECTOR_H__
>  #define __DRM_CONNECTOR_H__
>  
> +#include <linux/i2c.h>
>  #include <linux/list.h>
>  #include <linux/llist.h>
>  #include <linux/ctype.h>
> @@ -1308,6 +1309,16 @@ struct drm_connector {
>  	 * [0]: progressive, [1]: interlaced
>  	 */
>  	int audio_latency[2];
> +
> +	/**
> +	 * @ddc: associated ddc adapter.
> +	 * A connector usually has its associated ddc adapter. If a driver uses
> +	 * this field, then an appropriate symbolic link is created in connector
> +	 * sysfs directory to make it easy for the user to tell which i2c
> +	 * adapter is for a particular display.
> +	 */
> +	struct i2c_adapter *ddc;
> +
>  	/**
>  	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
>  	 * Needed to workaround some HW bugs where we get all 0s
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/2] drm: Include ddc adapter pointer in struct drm_connector

[Daniel Vetter]

On Tue, Jun 25, 2019 at 12:24:40PM +0200, Andrzej Pietrasiewicz wrote:
> Add generic code which creates symbolic links in sysfs, pointing to ddc
> interface used by a particular video output. For example:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> This makes it easy for user to associate a display with its ddc adapter
> and use e.g. ddcutil to control the chosen monitor.
> 
> This patch adds an i2c_adapter pointer to struct drm_connector. Particular
> drivers can then use it instead of using their own private instance. If a
> connector contains a ddc, then create a symbolic link in sysfs.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

As mentioned in the other subthread, I think the important bit to sell
this is rolling it out to as many drivers as feasible and collecting all
the acks from driver maintainers.
-Daniel
> ---
>  drivers/gpu/drm/drm_sysfs.c |  7 +++++++
>  include/drm/drm_connector.h | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..26d359b39785 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	/* Let userspace know we have a new connector */
>  	drm_sysfs_hotplug_event(dev);
>  
> +	if (connector->ddc)
> +		return sysfs_create_link(&connector->kdev->kobj,
> +				 &connector->ddc->dev.kobj, "ddc");
>  	return 0;
>  }
>  
> @@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  {
>  	if (!connector->kdev)
>  		return;
> +
> +	if (connector->ddc)
> +		sysfs_remove_link(&connector->kdev->kobj, "ddc");
> +
>  	DRM_DEBUG("removing \"%s\" from sysfs\n",
>  		  connector->name);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ca745d9feaf5..1ad3d1d54ba7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -23,6 +23,7 @@
>  #ifndef __DRM_CONNECTOR_H__
>  #define __DRM_CONNECTOR_H__
>  
> +#include <linux/i2c.h>
>  #include <linux/list.h>
>  #include <linux/llist.h>
>  #include <linux/ctype.h>
> @@ -1308,6 +1309,16 @@ struct drm_connector {
>  	 * [0]: progressive, [1]: interlaced
>  	 */
>  	int audio_latency[2];
> +
> +	/**
> +	 * @ddc: associated ddc adapter.
> +	 * A connector usually has its associated ddc adapter. If a driver uses
> +	 * this field, then an appropriate symbolic link is created in connector
> +	 * sysfs directory to make it easy for the user to tell which i2c
> +	 * adapter is for a particular display.
> +	 */
> +	struct i2c_adapter *ddc;
> +
>  	/**
>  	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
>  	 * Needed to workaround some HW bugs where we get all 0s
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

W dniu 25.06.2019 o 16:20, Daniel Vetter pisze:
> On Tue, Jun 25, 2019 at 03:10:32PM +0100, Russell King - ARM Linux admin wrote:
>> On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
>>> Otherwise I like this. Biggest problem I'm seeing here is rolling this out
>>> everywhere, this is a lot of work. And without widespread adoptions it's
>>> not terribly useful for userspace.
>>
>> There will be cases where it's not possible, because the I2C bus is
>> hidden behind a chip that doesn't give you direct access to the DDC
>> bus.
> 
> Oh sure, plus lots of connectors where there's just not ddc bus at all.
> But if we only roll this out for a handful of drivers it's also not great,
> that's what I meant. Looking at
> 
> $ git grep drm_do_get_edid
> 
> there's only very few drivers where the ddc bus is hidden. There's a lot
> more where it's not, and I think a big series to tackle those would serve
> extremely well to make a case for this sysfs link.
> -Daniel
> 

I will respond with a v3 then, including as many drivers as possible.
Those will be compile-tested only, though.

Andrzej

Re: [PATCH 0/2] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

W dniu 25.06.2019 o 16:20, Daniel Vetter pisze:
> On Tue, Jun 25, 2019 at 03:10:32PM +0100, Russell King - ARM Linux admin wrote:
>> On Tue, Jun 25, 2019 at 04:07:55PM +0200, Daniel Vetter wrote:
>>> Otherwise I like this. Biggest problem I'm seeing here is rolling this out
>>> everywhere, this is a lot of work. And without widespread adoptions it's
>>> not terribly useful for userspace.
>>
>> There will be cases where it's not possible, because the I2C bus is
>> hidden behind a chip that doesn't give you direct access to the DDC
>> bus.
> 
> Oh sure, plus lots of connectors where there's just not ddc bus at all.
> But if we only roll this out for a handful of drivers it's also not great,
> that's what I meant. Looking at
> 
> $ git grep drm_do_get_edid
> 
> there's only very few drivers where the ddc bus is hidden. There's a lot
> more where it's not, and I think a big series to tackle those would serve
> extremely well to make a case for this sysfs link.
> -Daniel
> 

I will respond with a v3 then, including as many drivers as possible.
Those will be compile-tested only, though.

Andrzej

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 00/22] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

It is difficult for a user to know which of the i2c adapters is for which
drm connector. This series addresses this problem.

The idea is to have a symbolic link in connector's sysfs directory, e.g.:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
ddcutil:

ddcutil -b 2 getvcp 0x10
VCP code 0x10 (Brightness                    ): current value =    90, max value =   100

The first patch in the series adds struct i2c_adapter pointer to struct
drm_connector. If the field is used by a particular driver, then an
appropriate symbolic link is created by the generic code, which is also added
by this patch.

The second patch is an example of how to convert a driver to this new scheme.

v1..v2:

- used fixed name "ddc" for the symbolic link in order to make it easy for
userspace to find the i2c adapter

v2..v3:

- converted as many drivers as possible.

PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!

Andrzej Pietrasiewicz (22):
  drm: Include ddc adapter pointer in struct drm_connector
  drm/exynos: Provide ddc symlink in connector's sysfs
  drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
  drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
  drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
  drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
    directory
  drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
  drm/tegra: Provide ddc symlink in output connector sysfs directory
  drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
  drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
  drm/vc4: Provide ddc symlink in connector sysfs directory
  drm: zte: Provide ddc symlink in hdmi connector sysfs directory
  drm: zte: Provide ddc symlink in vga connector sysfs directory
  drm/tilcdc: Provide ddc symlink in connector sysfs directory
  drm: sti: Provide ddc symlink in hdmi connector sysfs directory
  drm/mgag200: Provide ddc symlink in connector sysfs directory
  drm/ast: Provide ddc symlink in connector sysfs directory
  drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
    directory
  drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
  drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
    directory
  drm/amdgpu: Provide ddc symlink in connector sysfs directory
  drm/radeon: Provide ddc symlink in connector sysfs directory

 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 +++++++++++-----
 drivers/gpu/drm/ast/ast_mode.c                |  1 +
 drivers/gpu/drm/bridge/dumb-vga-dac.c         | 19 ++---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 40 ++++-----
 drivers/gpu/drm/bridge/ti-tfp410.c            | 19 ++---
 drivers/gpu/drm/drm_sysfs.c                   |  7 ++
 drivers/gpu/drm/exynos/exynos_hdmi.c          | 11 ++-
 drivers/gpu/drm/imx/imx-ldb.c                 | 13 ++-
 drivers/gpu/drm/imx/imx-tve.c                 |  8 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c           |  9 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c        |  1 +
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c     |  1 +
 drivers/gpu/drm/radeon/radeon_connectors.c    | 82 ++++++++++++++-----
 drivers/gpu/drm/rockchip/inno_hdmi.c          | 17 ++--
 drivers/gpu/drm/rockchip/rk3066_hdmi.c        | 17 ++--
 drivers/gpu/drm/sti/sti_hdmi.c                |  1 +
 drivers/gpu/drm/sun4i/sun4i_hdmi.h            |  1 -
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c        | 14 ++--
 drivers/gpu/drm/tegra/drm.h                   |  1 -
 drivers/gpu/drm/tegra/output.c                | 12 +--
 drivers/gpu/drm/tegra/sor.c                   |  6 +-
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c        |  1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c                | 16 ++--
 drivers/gpu/drm/zte/zx_hdmi.c                 | 25 ++----
 drivers/gpu/drm/zte/zx_vga.c                  | 25 ++----
 include/drm/drm_connector.h                   | 11 +++
 26 files changed, 252 insertions(+), 176 deletions(-)

-- 
2.17.1

[PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Add generic code which creates symbolic links in sysfs, pointing to ddc
interface used by a particular video output. For example:

ls -l /sys/class/drm/card0-HDMI-A-1/ddc
lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
	-> ../../../../soc/13880000.i2c/i2c-2

This makes it easy for user to associate a display with its ddc adapter
and use e.g. ddcutil to control the chosen monitor.

This patch adds an i2c_adapter pointer to struct drm_connector. Particular
drivers can then use it instead of using their own private instance. If a
connector contains a ddc, then create a symbolic link in sysfs.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Acked-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
---
 drivers/gpu/drm/drm_sysfs.c |  7 +++++++
 include/drm/drm_connector.h | 11 +++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ad10810bc972..26d359b39785 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	/* Let userspace know we have a new connector */
 	drm_sysfs_hotplug_event(dev);
 
+	if (connector->ddc)
+		return sysfs_create_link(&connector->kdev->kobj,
+				 &connector->ddc->dev.kobj, "ddc");
 	return 0;
 }
 
@@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 {
 	if (!connector->kdev)
 		return;
+
+	if (connector->ddc)
+		sysfs_remove_link(&connector->kdev->kobj, "ddc");
+
 	DRM_DEBUG("removing \"%s\" from sysfs\n",
 		  connector->name);
 
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ca745d9feaf5..1ad3d1d54ba7 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -23,6 +23,7 @@
 #ifndef __DRM_CONNECTOR_H__
 #define __DRM_CONNECTOR_H__
 
+#include <linux/i2c.h>
 #include <linux/list.h>
 #include <linux/llist.h>
 #include <linux/ctype.h>
@@ -1308,6 +1309,16 @@ struct drm_connector {
 	 * [0]: progressive, [1]: interlaced
 	 */
 	int audio_latency[2];
+
+	/**
+	 * @ddc: associated ddc adapter.
+	 * A connector usually has its associated ddc adapter. If a driver uses
+	 * this field, then an appropriate symbolic link is created in connector
+	 * sysfs directory to make it easy for the user to tell which i2c
+	 * adapter is for a particular display.
+	 */
+	struct i2c_adapter *ddc;
+
 	/**
 	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
 	 * Needed to workaround some HW bugs where we get all 0s
-- 
2.17.1

[PATCH v3 02/22] drm/exynos: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Switch to using the ddc provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 894a99793633..6816e37861b7 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -126,7 +126,6 @@ struct hdmi_context {
 	void __iomem			*regs;
 	void __iomem			*regs_hdmiphy;
 	struct i2c_client		*hdmiphy_port;
-	struct i2c_adapter		*ddc_adpt;
 	struct gpio_desc		*hpd_gpio;
 	int				irq;
 	struct regmap			*pmureg;
@@ -872,10 +871,10 @@ static int hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!hdata->ddc_adpt)
+	if (!connector->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(connector, hdata->ddc_adpt);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1893,7 +1892,7 @@ static int hdmi_get_ddc_adapter(struct hdmi_context *hdata)
 		return -EPROBE_DEFER;
 	}
 
-	hdata->ddc_adpt = adpt;
+	hdata->connector.ddc = adpt;
 
 	return 0;
 }
@@ -2045,7 +2044,7 @@ static int hdmi_probe(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 err_ddc:
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	return ret;
 }
@@ -2072,7 +2071,7 @@ static int hdmi_remove(struct platform_device *pdev)
 	if (hdata->regs_hdmiphy)
 		iounmap(hdata->regs_hdmiphy);
 
-	put_device(&hdata->ddc_adpt->dev);
+	put_device(&hdata->connector.ddc->dev);
 
 	mutex_destroy(&hdata->mutex);
 
-- 
2.17.1

[PATCH v3 03/22] drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/rockchip/rk3066_hdmi.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
index 85fc5f01f761..1f3e630ecdab 100644
--- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
+++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
@@ -49,7 +49,6 @@ struct rk3066_hdmi {
 	struct drm_encoder encoder;
 
 	struct rk3066_hdmi_i2c *i2c;
-	struct i2c_adapter *ddc;
 
 	unsigned int tmdsclk;
 
@@ -470,10 +469,10 @@ static int rk3066_hdmi_connector_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret = 0;
 
-	if (!hdmi->ddc)
+	if (!connector->ddc)
 		return 0;
 
-	edid = drm_get_edid(connector, hdmi->ddc);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (edid) {
 		hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
 		drm_connector_update_edid_property(connector, edid);
@@ -789,10 +788,10 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
 	/* internal hclk = hdmi_hclk / 25 */
 	hdmi_writeb(hdmi, HDMI_INTERNAL_CLK_DIVIDER, 25);
 
-	hdmi->ddc = rk3066_hdmi_i2c_adapter(hdmi);
-	if (IS_ERR(hdmi->ddc)) {
-		ret = PTR_ERR(hdmi->ddc);
-		hdmi->ddc = NULL;
+	hdmi->connector.ddc = rk3066_hdmi_i2c_adapter(hdmi);
+	if (IS_ERR(hdmi->connector.ddc)) {
+		ret = PTR_ERR(hdmi->connector.ddc);
+		hdmi->connector.ddc = NULL;
 		goto err_disable_hclk;
 	}
 
@@ -824,7 +823,7 @@ static int rk3066_hdmi_bind(struct device *dev, struct device *master,
 	hdmi->connector.funcs->destroy(&hdmi->connector);
 	hdmi->encoder.funcs->destroy(&hdmi->encoder);
 err_disable_i2c:
-	i2c_put_adapter(hdmi->ddc);
+	i2c_put_adapter(hdmi->connector.ddc);
 err_disable_hclk:
 	clk_disable_unprepare(hdmi->hclk);
 
@@ -839,7 +838,7 @@ static void rk3066_hdmi_unbind(struct device *dev, struct device *master,
 	hdmi->connector.funcs->destroy(&hdmi->connector);
 	hdmi->encoder.funcs->destroy(&hdmi->encoder);
 
-	i2c_put_adapter(hdmi->ddc);
+	i2c_put_adapter(hdmi->connector.ddc);
 	clk_disable_unprepare(hdmi->hclk);
 }
 
-- 
2.17.1

[PATCH v3 04/22] drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/rockchip/inno_hdmi.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index f8ca98d294d0..d64b119c2649 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -59,7 +59,6 @@ struct inno_hdmi {
 	struct drm_encoder	encoder;
 
 	struct inno_hdmi_i2c *i2c;
-	struct i2c_adapter *ddc;
 
 	unsigned int tmds_rate;
 
@@ -552,10 +551,10 @@ static int inno_hdmi_connector_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret = 0;
 
-	if (!hdmi->ddc)
+	if (!hdmi->connector.ddc)
 		return 0;
 
-	edid = drm_get_edid(connector, hdmi->ddc);
+	edid = drm_get_edid(connector, hdmi->connector.ddc);
 	if (edid) {
 		hdmi->hdmi_data.sink_is_hdmi = drm_detect_hdmi_monitor(edid);
 		hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
@@ -850,10 +849,10 @@ static int inno_hdmi_bind(struct device *dev, struct device *master,
 
 	inno_hdmi_reset(hdmi);
 
-	hdmi->ddc = inno_hdmi_i2c_adapter(hdmi);
-	if (IS_ERR(hdmi->ddc)) {
-		ret = PTR_ERR(hdmi->ddc);
-		hdmi->ddc = NULL;
+	hdmi->connector.ddc = inno_hdmi_i2c_adapter(hdmi);
+	if (IS_ERR(hdmi->connector.ddc)) {
+		ret = PTR_ERR(hdmi->connector.ddc);
+		hdmi->connector.ddc = NULL;
 		goto err_disable_clk;
 	}
 
@@ -886,7 +885,7 @@ static int inno_hdmi_bind(struct device *dev, struct device *master,
 	hdmi->connector.funcs->destroy(&hdmi->connector);
 	hdmi->encoder.funcs->destroy(&hdmi->encoder);
 err_put_adapter:
-	i2c_put_adapter(hdmi->ddc);
+	i2c_put_adapter(hdmi->connector.ddc);
 err_disable_clk:
 	clk_disable_unprepare(hdmi->pclk);
 	return ret;
@@ -900,7 +899,7 @@ static void inno_hdmi_unbind(struct device *dev, struct device *master,
 	hdmi->connector.funcs->destroy(&hdmi->connector);
 	hdmi->encoder.funcs->destroy(&hdmi->encoder);
 
-	i2c_put_adapter(hdmi->ddc);
+	i2c_put_adapter(hdmi->connector.ddc);
 	clk_disable_unprepare(hdmi->pclk);
 }
 
-- 
2.17.1

[PATCH v3 05/22] drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index 07b4cb877d82..4979e3362687 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -461,6 +461,7 @@ struct drm_connector *msm_hdmi_connector_init(struct hdmi *hdmi)
 	connector->doublescan_allowed = 0;
 
 	drm_connector_attach_encoder(connector, hdmi->encoder);
+	connector->ddc = hdmi->i2c;
 
 	return connector;
 }
-- 
2.17.1

[PATCH v3 06/22] drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/sun4i/sun4i_hdmi.h     |  1 -
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 14 +++++++-------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi.h b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
index 7ad3f06c127e..1649273b1493 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi.h
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi.h
@@ -265,7 +265,6 @@ struct sun4i_hdmi {
 	struct clk		*tmds_clk;
 
 	struct i2c_adapter	*i2c;
-	struct i2c_adapter	*ddc_i2c;
 
 	/* Regmap fields for I2C adapter */
 	struct regmap_field	*field_ddc_en;
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 9c3f99339b82..250bec00dc35 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -213,7 +213,7 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	edid = drm_get_edid(connector, hdmi->ddc_i2c ?: hdmi->i2c);
+	edid = drm_get_edid(connector, connector->ddc ?: hdmi->i2c);
 	if (!edid)
 		return 0;
 
@@ -598,11 +598,11 @@ static int sun4i_hdmi_bind(struct device *dev, struct device *master,
 		goto err_disable_mod_clk;
 	}
 
-	hdmi->ddc_i2c = sun4i_hdmi_get_ddc(dev);
-	if (IS_ERR(hdmi->ddc_i2c)) {
-		ret = PTR_ERR(hdmi->ddc_i2c);
+	hdmi->connector.ddc = sun4i_hdmi_get_ddc(dev);
+	if (IS_ERR(hdmi->connector.ddc)) {
+		ret = PTR_ERR(hdmi->connector.ddc);
 		if (ret == -ENODEV)
-			hdmi->ddc_i2c = NULL;
+			hdmi->connector.ddc = NULL;
 		else
 			goto err_del_i2c_adapter;
 	}
@@ -663,7 +663,7 @@ static int sun4i_hdmi_bind(struct device *dev, struct device *master,
 	cec_delete_adapter(hdmi->cec_adap);
 	drm_encoder_cleanup(&hdmi->encoder);
 err_put_ddc_i2c:
-	i2c_put_adapter(hdmi->ddc_i2c);
+	i2c_put_adapter(hdmi->connector.ddc);
 err_del_i2c_adapter:
 	i2c_del_adapter(hdmi->i2c);
 err_disable_mod_clk:
@@ -684,7 +684,7 @@ static void sun4i_hdmi_unbind(struct device *dev, struct device *master,
 	drm_connector_cleanup(&hdmi->connector);
 	drm_encoder_cleanup(&hdmi->encoder);
 	i2c_del_adapter(hdmi->i2c);
-	i2c_put_adapter(hdmi->ddc_i2c);
+	i2c_put_adapter(hdmi->connector.ddc);
 	clk_disable_unprepare(hdmi->mod_clk);
 	clk_disable_unprepare(hdmi->bus_clk);
 }
-- 
2.17.1

[PATCH v3 07/22] drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 5d6a9f094df5..6c5321dcc4b8 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -146,7 +146,6 @@ struct mtk_hdmi {
 	struct device *dev;
 	struct phy *phy;
 	struct device *cec_dev;
-	struct i2c_adapter *ddc_adpt;
 	struct clk *clk[MTK_HDMI_CLK_COUNT];
 	struct drm_display_mode mode;
 	bool dvi_mode;
@@ -1213,10 +1212,10 @@ static int mtk_hdmi_conn_get_modes(struct drm_connector *conn)
 	struct edid *edid;
 	int ret;
 
-	if (!hdmi->ddc_adpt)
+	if (!conn->ddc)
 		return -ENODEV;
 
-	edid = drm_get_edid(conn, hdmi->ddc_adpt);
+	edid = drm_get_edid(conn, conn->ddc);
 	if (!edid)
 		return -ENODEV;
 
@@ -1509,9 +1508,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
 	}
 	of_node_put(remote);
 
-	hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np);
+	hdmi->conn.ddc = of_find_i2c_adapter_by_node(i2c_np);
 	of_node_put(i2c_np);
-	if (!hdmi->ddc_adpt) {
+	if (!hdmi->conn.ddc) {
 		dev_err(dev, "Failed to get ddc i2c adapter by node\n");
 		return -EINVAL;
 	}
-- 
2.17.1

[PATCH v3 08/22] drm/tegra: Provide ddc symlink in output connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/tegra/drm.h    |  1 -
 drivers/gpu/drm/tegra/output.c | 12 ++++++------
 drivers/gpu/drm/tegra/sor.c    |  6 +++---
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 86daa19fcf24..9bf72bcd3ec1 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -120,7 +120,6 @@ struct tegra_output {
 	struct device *dev;
 
 	struct drm_panel *panel;
-	struct i2c_adapter *ddc;
 	const struct edid *edid;
 	struct cec_notifier *cec;
 	unsigned int hpd_irq;
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 274cb955e2e1..0b5037a29c63 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -30,8 +30,8 @@ int tegra_output_connector_get_modes(struct drm_connector *connector)
 
 	if (output->edid)
 		edid = kmemdup(output->edid, sizeof(*edid), GFP_KERNEL);
-	else if (output->ddc)
-		edid = drm_get_edid(connector, output->ddc);
+	else if (connector->ddc)
+		edid = drm_get_edid(connector, connector->ddc);
 
 	cec_notifier_set_phys_addr_from_edid(output->cec, edid);
 	drm_connector_update_edid_property(connector, edid);
@@ -111,8 +111,8 @@ int tegra_output_probe(struct tegra_output *output)
 
 	ddc = of_parse_phandle(output->of_node, "nvidia,ddc-i2c-bus", 0);
 	if (ddc) {
-		output->ddc = of_find_i2c_adapter_by_node(ddc);
-		if (!output->ddc) {
+		output->connector.ddc = of_find_i2c_adapter_by_node(ddc);
+		if (!output->connector.ddc) {
 			err = -EPROBE_DEFER;
 			of_node_put(ddc);
 			return err;
@@ -174,8 +174,8 @@ void tegra_output_remove(struct tegra_output *output)
 	if (output->hpd_gpio)
 		free_irq(output->hpd_irq, output);
 
-	if (output->ddc)
-		put_device(&output->ddc->dev);
+	if (output->connector.ddc)
+		put_device(&output->connector.ddc->dev);
 }
 
 int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 4ffe3794e6d3..77e61f98de07 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2311,7 +2311,7 @@ static void tegra_sor_hdmi_disable_scrambling(struct tegra_sor *sor)
 
 static void tegra_sor_hdmi_scdc_disable(struct tegra_sor *sor)
 {
-	struct i2c_adapter *ddc = sor->output.ddc;
+	struct i2c_adapter *ddc = sor->output.connector.ddc;
 
 	drm_scdc_set_high_tmds_clock_ratio(ddc, false);
 	drm_scdc_set_scrambling(ddc, false);
@@ -2339,7 +2339,7 @@ static void tegra_sor_hdmi_enable_scrambling(struct tegra_sor *sor)
 
 static void tegra_sor_hdmi_scdc_enable(struct tegra_sor *sor)
 {
-	struct i2c_adapter *ddc = sor->output.ddc;
+	struct i2c_adapter *ddc = sor->output.connector.ddc;
 
 	drm_scdc_set_high_tmds_clock_ratio(ddc, true);
 	drm_scdc_set_scrambling(ddc, true);
@@ -2350,7 +2350,7 @@ static void tegra_sor_hdmi_scdc_enable(struct tegra_sor *sor)
 static void tegra_sor_hdmi_scdc_work(struct work_struct *work)
 {
 	struct tegra_sor *sor = container_of(work, struct tegra_sor, scdc.work);
-	struct i2c_adapter *ddc = sor->output.ddc;
+	struct i2c_adapter *ddc = sor->output.connector.ddc;
 
 	if (!drm_scdc_get_scrambling_status(ddc)) {
 		DRM_DEBUG_KMS("SCDC not scrambled\n");
-- 
2.17.1

[PATCH v3 09/22] drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/imx/imx-ldb.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-ldb.c b/drivers/gpu/drm/imx/imx-ldb.c
index 383733302280..44fdb264339e 100644
--- a/drivers/gpu/drm/imx/imx-ldb.c
+++ b/drivers/gpu/drm/imx/imx-ldb.c
@@ -55,7 +55,6 @@ struct imx_ldb_channel {
 	struct drm_bridge *bridge;
 
 	struct device_node *child;
-	struct i2c_adapter *ddc;
 	int chno;
 	void *edid;
 	int edid_len;
@@ -131,8 +130,8 @@ static int imx_ldb_connector_get_modes(struct drm_connector *connector)
 			return num_modes;
 	}
 
-	if (!imx_ldb_ch->edid && imx_ldb_ch->ddc)
-		imx_ldb_ch->edid = drm_get_edid(connector, imx_ldb_ch->ddc);
+	if (!imx_ldb_ch->edid && connector->ddc)
+		imx_ldb_ch->edid = drm_get_edid(connector, connector->ddc);
 
 	if (imx_ldb_ch->edid) {
 		drm_connector_update_edid_property(connector,
@@ -550,15 +549,15 @@ static int imx_ldb_panel_ddc(struct device *dev,
 
 	ddc_node = of_parse_phandle(child, "ddc-i2c-bus", 0);
 	if (ddc_node) {
-		channel->ddc = of_find_i2c_adapter_by_node(ddc_node);
+		channel->connector.ddc = of_find_i2c_adapter_by_node(ddc_node);
 		of_node_put(ddc_node);
-		if (!channel->ddc) {
+		if (!channel->connector.ddc) {
 			dev_warn(dev, "failed to get ddc i2c adapter\n");
 			return -EPROBE_DEFER;
 		}
 	}
 
-	if (!channel->ddc) {
+	if (!channel->connector.ddc) {
 		/* if no DDC available, fallback to hardcoded EDID */
 		dev_dbg(dev, "no ddc available\n");
 
@@ -725,7 +724,7 @@ static void imx_ldb_unbind(struct device *dev, struct device *master,
 			drm_panel_detach(channel->panel);
 
 		kfree(channel->edid);
-		i2c_put_adapter(channel->ddc);
+		i2c_put_adapter(channel->connector.ddc);
 	}
 }
 
-- 
2.17.1

[PATCH v3 10/22] drm/imx: imx-tve: Provide ddc symlink in connector's sysfs

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/imx/imx-tve.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index e725af8a0025..b8bee4e1f169 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -109,7 +109,6 @@ struct imx_tve {
 
 	struct regmap *regmap;
 	struct regulator *dac_reg;
-	struct i2c_adapter *ddc;
 	struct clk *clk;
 	struct clk *di_sel_clk;
 	struct clk_hw clk_hw_di;
@@ -218,14 +217,13 @@ static int tve_setup_vga(struct imx_tve *tve)
 
 static int imx_tve_connector_get_modes(struct drm_connector *connector)
 {
-	struct imx_tve *tve = con_to_tve(connector);
 	struct edid *edid;
 	int ret = 0;
 
-	if (!tve->ddc)
+	if (!connector->ddc)
 		return 0;
 
-	edid = drm_get_edid(connector, tve->ddc);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (edid) {
 		drm_connector_update_edid_property(connector, edid);
 		ret = drm_add_edid_modes(connector, edid);
@@ -551,7 +549,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
 	if (ddc_node) {
-		tve->ddc = of_find_i2c_adapter_by_node(ddc_node);
+		tve->connector.ddc = of_find_i2c_adapter_by_node(ddc_node);
 		of_node_put(ddc_node);
 	}
 
-- 
2.17.1

[PATCH v3 11/22] drm/vc4: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 87ad0879edf3..b46df3aa1798 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -75,7 +75,6 @@ struct vc4_hdmi {
 
 	struct vc4_hdmi_audio audio;
 
-	struct i2c_adapter *ddc;
 	void __iomem *hdmicore_regs;
 	void __iomem *hd_regs;
 	int hpd_gpio;
@@ -206,7 +205,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector, bool force)
 		return connector_status_disconnected;
 	}
 
-	if (drm_probe_ddc(vc4->hdmi->ddc))
+	if (drm_probe_ddc(connector->ddc))
 		return connector_status_connected;
 
 	if (HDMI_READ(VC4_HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED)
@@ -232,7 +231,7 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 	int ret = 0;
 	struct edid *edid;
 
-	edid = drm_get_edid(connector, vc4->hdmi->ddc);
+	edid = drm_get_edid(connector, connector->ddc);
 	cec_s_phys_addr_from_edid(vc4->hdmi->cec_adap, edid);
 	if (!edid)
 		return -ENODEV;
@@ -1287,6 +1286,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	struct vc4_hdmi *hdmi;
 	struct vc4_hdmi_encoder *vc4_hdmi_encoder;
 	struct device_node *ddc_node;
+	struct i2c_adapter *ddc;
 	u32 value;
 	int ret;
 
@@ -1334,9 +1334,9 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 		return -ENODEV;
 	}
 
-	hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node);
+	ddc = of_find_i2c_adapter_by_node(ddc_node);
 	of_node_put(ddc_node);
-	if (!hdmi->ddc) {
+	if (ddc) {
 		DRM_DEBUG("Failed to get ddc i2c adapter by node\n");
 		return -EPROBE_DEFER;
 	}
@@ -1396,6 +1396,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 		ret = PTR_ERR(hdmi->connector);
 		goto err_destroy_encoder;
 	}
+	hdmi->connector->ddc = ddc;
 #ifdef CONFIG_DRM_VC4_HDMI_CEC
 	hdmi->cec_adap = cec_allocate_adapter(&vc4_hdmi_cec_adap_ops,
 					      vc4, "vc4",
@@ -1448,7 +1449,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	clk_disable_unprepare(hdmi->hsm_clock);
 	pm_runtime_disable(dev);
 err_put_i2c:
-	put_device(&hdmi->ddc->dev);
+	put_device(&ddc->dev);
 
 	return ret;
 }
@@ -1459,6 +1460,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master,
 	struct drm_device *drm = dev_get_drvdata(master);
 	struct vc4_dev *vc4 = drm->dev_private;
 	struct vc4_hdmi *hdmi = vc4->hdmi;
+	struct i2c_adapter *ddc = hdmi->connector->ddc;
 
 	cec_unregister_adapter(hdmi->cec_adap);
 	vc4_hdmi_connector_destroy(hdmi->connector);
@@ -1467,7 +1469,7 @@ static void vc4_hdmi_unbind(struct device *dev, struct device *master,
 	clk_disable_unprepare(hdmi->hsm_clock);
 	pm_runtime_disable(dev);
 
-	put_device(&hdmi->ddc->dev);
+	put_device(&ddc->dev);
 
 	vc4->hdmi = NULL;
 }
-- 
2.17.1

[PATCH v3 12/22] drm: zte: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/zte/zx_hdmi.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index bfe918b27c5c..862a855ea14a 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -29,15 +29,11 @@
 #define ZX_HDMI_INFOFRAME_SIZE		31
 #define DDC_SEGMENT_ADDR		0x30
 
-struct zx_hdmi_i2c {
-	struct i2c_adapter adap;
-	struct mutex lock;
-};
-
 struct zx_hdmi {
 	struct drm_connector connector;
 	struct drm_encoder encoder;
-	struct zx_hdmi_i2c *ddc;
+	/* protects ddc access */
+	struct mutex ddc_lock;
 	struct device *dev;
 	struct drm_device *drm;
 	void __iomem *mmio;
@@ -264,7 +260,7 @@ static int zx_hdmi_connector_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	edid = drm_get_edid(connector, &hdmi->ddc->adap);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid)
 		return 0;
 
@@ -562,10 +558,9 @@ static int zx_hdmi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			    int num)
 {
 	struct zx_hdmi *hdmi = i2c_get_adapdata(adap);
-	struct zx_hdmi_i2c *ddc = hdmi->ddc;
 	int i, ret = 0;
 
-	mutex_lock(&ddc->lock);
+	mutex_lock(&hdmi->ddc_lock);
 
 	/* Enable DDC master access */
 	hdmi_writeb_mask(hdmi, TPI_DDC_MASTER_EN, HW_DDC_MASTER, HW_DDC_MASTER);
@@ -590,7 +585,7 @@ static int zx_hdmi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	/* Disable DDC master access */
 	hdmi_writeb_mask(hdmi, TPI_DDC_MASTER_EN, HW_DDC_MASTER, 0);
 
-	mutex_unlock(&ddc->lock);
+	mutex_unlock(&hdmi->ddc_lock);
 
 	return ret;
 }
@@ -608,17 +603,15 @@ static const struct i2c_algorithm zx_hdmi_algorithm = {
 static int zx_hdmi_ddc_register(struct zx_hdmi *hdmi)
 {
 	struct i2c_adapter *adap;
-	struct zx_hdmi_i2c *ddc;
 	int ret;
 
-	ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
-	if (!ddc)
+	adap = devm_kzalloc(hdmi->dev, sizeof(*adap), GFP_KERNEL);
+	if (!adap)
 		return -ENOMEM;
 
-	hdmi->ddc = ddc;
-	mutex_init(&ddc->lock);
+	hdmi->connector.ddc = adap;
+	mutex_init(&hdmi->ddc_lock);
 
-	adap = &ddc->adap;
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DDC;
 	adap->dev.parent = hdmi->dev;
-- 
2.17.1

[PATCH v3 13/22] drm: zte: Provide ddc symlink in vga connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/zte/zx_vga.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_vga.c b/drivers/gpu/drm/zte/zx_vga.c
index 1634a08707fb..a3a4d6982888 100644
--- a/drivers/gpu/drm/zte/zx_vga.c
+++ b/drivers/gpu/drm/zte/zx_vga.c
@@ -23,15 +23,11 @@ struct zx_vga_pwrctrl {
 	u32 mask;
 };
 
-struct zx_vga_i2c {
-	struct i2c_adapter adap;
-	struct mutex lock;
-};
-
 struct zx_vga {
 	struct drm_connector connector;
 	struct drm_encoder encoder;
-	struct zx_vga_i2c *ddc;
+	/* protects ddc access */
+	struct mutex ddc_lock;
 	struct device *dev;
 	void __iomem *mmio;
 	struct clk *i2c_wclk;
@@ -86,7 +82,7 @@ static int zx_vga_connector_get_modes(struct drm_connector *connector)
 	 */
 	zx_writel(vga->mmio + VGA_AUTO_DETECT_SEL, 0);
 
-	edid = drm_get_edid(connector, &vga->ddc->adap);
+	edid = drm_get_edid(connector, connector->ddc);
 	if (!edid) {
 		/*
 		 * If EDID reading fails, we set the device state into
@@ -282,11 +278,10 @@ static int zx_vga_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 			   int num)
 {
 	struct zx_vga *vga = i2c_get_adapdata(adap);
-	struct zx_vga_i2c *ddc = vga->ddc;
 	int ret = 0;
 	int i;
 
-	mutex_lock(&ddc->lock);
+	mutex_lock(&vga->ddc_lock);
 
 	for (i = 0; i < num; i++) {
 		if (msgs[i].flags & I2C_M_RD)
@@ -301,7 +296,7 @@ static int zx_vga_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	if (!ret)
 		ret = num;
 
-	mutex_unlock(&ddc->lock);
+	mutex_unlock(&vga->ddc_lock);
 
 	return ret;
 }
@@ -320,17 +315,15 @@ static int zx_vga_ddc_register(struct zx_vga *vga)
 {
 	struct device *dev = vga->dev;
 	struct i2c_adapter *adap;
-	struct zx_vga_i2c *ddc;
 	int ret;
 
-	ddc = devm_kzalloc(dev, sizeof(*ddc), GFP_KERNEL);
-	if (!ddc)
+	adap = devm_kzalloc(dev, sizeof(*adap), GFP_KERNEL);
+	if (!adap)
 		return -ENOMEM;
 
-	vga->ddc = ddc;
-	mutex_init(&ddc->lock);
+	vga->connector.ddc = adap;
+	mutex_init(&vga->ddc_lock);
 
-	adap = &ddc->adap;
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DDC;
 	adap->dev.parent = dev;
-- 
2.17.1

[PATCH v3 14/22] drm/tilcdc: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
index 62d014c20988..c373edb95666 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
@@ -219,6 +219,7 @@ static struct drm_connector *tfp410_connector_create(struct drm_device *dev,
 	tfp410_connector->mod = mod;
 
 	connector = &tfp410_connector->base;
+	connector->ddc = mod->i2c;
 
 	drm_connector_init(dev, connector, &tfp410_connector_funcs,
 			DRM_MODE_CONNECTOR_DVID);
-- 
2.17.1

[PATCH v3 15/22] drm: sti: Provide ddc symlink in hdmi connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/sti/sti_hdmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f03d617edc4c..c5e6c33ff2cd 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1288,6 +1288,7 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 			&sti_hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA);
 	drm_connector_helper_add(drm_connector,
 			&sti_hdmi_connector_helper_funcs);
+	drm_connector->ddc = hdmi->ddc_adapt;
 
 	/* initialise property */
 	sti_hdmi_connector_init_property(drm_dev, drm_connector);
-- 
2.17.1

[PATCH v3 16/22] drm/mgag200: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index a25054015e8c..a22dbecd4d35 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1712,6 +1712,7 @@ static struct drm_connector *mga_vga_init(struct drm_device *dev)
 	drm_connector_register(connector);
 
 	mga_connector->i2c = mgag200_i2c_create(dev);
+	connector->ddc = &mga_connector->i2c->adapter;
 	if (!mga_connector->i2c)
 		DRM_ERROR("failed to add ddc bus\n");
 
-- 
2.17.1

[PATCH v3 17/22] drm/ast: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/ast/ast_mode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index ffccbef962a4..155c3487a1a7 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -905,6 +905,7 @@ static int ast_connector_init(struct drm_device *dev)
 	drm_connector_attach_encoder(connector, encoder);
 
 	ast_connector->i2c = ast_i2c_create(dev);
+	connector->ddc = &ast_connector->i2c->adapter;
 	if (!ast_connector->i2c)
 		DRM_ERROR("failed to add ddc bus for connector\n");
 
-- 
2.17.1

[PATCH v3 18/22] drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/bridge/dumb-vga-dac.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index d32885b906ae..b4cc3238400a 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -20,7 +20,6 @@ struct dumb_vga {
 	struct drm_bridge	bridge;
 	struct drm_connector	connector;
 
-	struct i2c_adapter	*ddc;
 	struct regulator	*vdd;
 };
 
@@ -42,10 +41,10 @@ static int dumb_vga_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (IS_ERR(vga->ddc))
+	if (IS_ERR(vga->connector.ddc))
 		goto fallback;
 
-	edid = drm_get_edid(connector, vga->ddc);
+	edid = drm_get_edid(connector, vga->connector.ddc);
 	if (!edid) {
 		DRM_INFO("EDID readout failed, falling back to standard modes\n");
 		goto fallback;
@@ -84,7 +83,7 @@ dumb_vga_connector_detect(struct drm_connector *connector, bool force)
 	 * wire the DDC pins, or the I2C bus might not be working at
 	 * all.
 	 */
-	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
+	if (!IS_ERR(vga->connector.ddc) && drm_probe_ddc(vga->connector.ddc))
 		return connector_status_connected;
 
 	return connector_status_unknown;
@@ -190,14 +189,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
 	}
 
-	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
-	if (IS_ERR(vga->ddc)) {
-		if (PTR_ERR(vga->ddc) == -ENODEV) {
+	vga->connector.ddc = dumb_vga_retrieve_ddc(&pdev->dev);
+	if (IS_ERR(vga->connector.ddc)) {
+		if (PTR_ERR(vga->connector.ddc) == -ENODEV) {
 			dev_dbg(&pdev->dev,
 				"No i2c bus specified. Disabling EDID readout\n");
 		} else {
 			dev_err(&pdev->dev, "Couldn't retrieve i2c bus\n");
-			return PTR_ERR(vga->ddc);
+			return PTR_ERR(vga->connector.ddc);
 		}
 	}
 
@@ -216,8 +215,8 @@ static int dumb_vga_remove(struct platform_device *pdev)
 
 	drm_bridge_remove(&vga->bridge);
 
-	if (!IS_ERR(vga->ddc))
-		i2c_put_adapter(vga->ddc);
+	if (!IS_ERR(vga->connector.ddc))
+		i2c_put_adapter(vga->connector.ddc);
 
 	return 0;
 }
-- 
2.17.1

[PATCH v3 19/22] drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 40 +++++++++++------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index c6490949d9db..0b9c9f2619da 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -161,7 +161,6 @@ struct dw_hdmi {
 
 	struct drm_display_mode previous_mode;
 
-	struct i2c_adapter *ddc;
 	void __iomem *regs;
 	bool sink_is_hdmi;
 	bool sink_has_audio;
@@ -1118,7 +1117,7 @@ static bool dw_hdmi_support_scdc(struct dw_hdmi *hdmi)
 		return false;
 
 	/* Disable if no DDC bus */
-	if (!hdmi->ddc)
+	if (!hdmi->connector.ddc)
 		return false;
 
 	/* Disable if SCDC is not supported, or if an HF-VSDB block is absent */
@@ -1156,10 +1155,11 @@ void dw_hdmi_set_high_tmds_clock_ratio(struct dw_hdmi *hdmi)
 
 	/* Control for TMDS Bit Period/TMDS Clock-Period Ratio */
 	if (dw_hdmi_support_scdc(hdmi)) {
+		struct i2c_adapter *ddc = hdmi->connector.ddc;
 		if (mtmdsclock > HDMI14_MAX_TMDSCLK)
-			drm_scdc_set_high_tmds_clock_ratio(hdmi->ddc, 1);
+			drm_scdc_set_high_tmds_clock_ratio(ddc, 1);
 		else
-			drm_scdc_set_high_tmds_clock_ratio(hdmi->ddc, 0);
+			drm_scdc_set_high_tmds_clock_ratio(ddc, 0);
 	}
 }
 EXPORT_SYMBOL_GPL(dw_hdmi_set_high_tmds_clock_ratio);
@@ -1750,6 +1750,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 	if (dw_hdmi_support_scdc(hdmi)) {
 		if (vmode->mtmdsclock > HDMI14_MAX_TMDSCLK ||
 		    hdmi_info->scdc.scrambling.low_rates) {
+			struct i2c_adapter *ddc = hdmi->connector.ddc;
 			/*
 			 * HDMI2.0 Specifies the following procedure:
 			 * After the Source Device has determined that
@@ -1759,13 +1760,12 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 			 * Source Devices compliant shall set the
 			 * Source Version = 1.
 			 */
-			drm_scdc_readb(hdmi->ddc, SCDC_SINK_VERSION,
-				       &bytes);
-			drm_scdc_writeb(hdmi->ddc, SCDC_SOURCE_VERSION,
+			drm_scdc_readb(ddc, SCDC_SINK_VERSION, &bytes);
+			drm_scdc_writeb(ddc, SCDC_SOURCE_VERSION,
 				min_t(u8, bytes, SCDC_MIN_SOURCE_VERSION));
 
 			/* Enabled Scrambling in the Sink */
-			drm_scdc_set_scrambling(hdmi->ddc, 1);
+			drm_scdc_set_scrambling(hdmi->connector.ddc, 1);
 
 			/*
 			 * To activate the scrambler feature, you must ensure
@@ -1781,7 +1781,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
 			hdmi_writeb(hdmi, 0, HDMI_FC_SCRAMBLER_CTRL);
 			hdmi_writeb(hdmi, (u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ,
 				    HDMI_MC_SWRSTZ);
-			drm_scdc_set_scrambling(hdmi->ddc, 0);
+			drm_scdc_set_scrambling(hdmi->connector.ddc, 0);
 		}
 	}
 
@@ -2127,10 +2127,10 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret = 0;
 
-	if (!hdmi->ddc)
+	if (!hdmi->connector.ddc)
 		return 0;
 
-	edid = drm_get_edid(connector, hdmi->ddc);
+	edid = drm_get_edid(connector, hdmi->connector.ddc);
 	if (edid) {
 		dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
 			edid->width_cm, edid->height_cm);
@@ -2548,9 +2548,9 @@ __dw_hdmi_probe(struct platform_device *pdev,
 
 	ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
 	if (ddc_node) {
-		hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
+		hdmi->connector.ddc = of_get_i2c_adapter_by_node(ddc_node);
 		of_node_put(ddc_node);
-		if (!hdmi->ddc) {
+		if (!hdmi->connector.ddc) {
 			dev_dbg(hdmi->dev, "failed to read ddc node\n");
 			return ERR_PTR(-EPROBE_DEFER);
 		}
@@ -2689,7 +2689,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 	hdmi_init_clk_regenerator(hdmi);
 
 	/* If DDC bus is not specified, try to register HDMI I2C bus */
-	if (!hdmi->ddc) {
+	if (!hdmi->connector.ddc) {
 		/* Look for (optional) stuff related to unwedging */
 		hdmi->pinctrl = devm_pinctrl_get(dev);
 		if (!IS_ERR(hdmi->pinctrl)) {
@@ -2708,9 +2708,9 @@ __dw_hdmi_probe(struct platform_device *pdev,
 			}
 		}
 
-		hdmi->ddc = dw_hdmi_i2c_adapter(hdmi);
-		if (IS_ERR(hdmi->ddc))
-			hdmi->ddc = NULL;
+		hdmi->connector.ddc = dw_hdmi_i2c_adapter(hdmi);
+		if (IS_ERR(hdmi->connector.ddc))
+			hdmi->connector.ddc = NULL;
 	}
 
 	hdmi->bridge.driver_private = hdmi;
@@ -2776,7 +2776,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 err_iahb:
 	if (hdmi->i2c) {
 		i2c_del_adapter(&hdmi->i2c->adap);
-		hdmi->ddc = NULL;
+		hdmi->connector.ddc = NULL;
 	}
 
 	if (hdmi->cec_notifier)
@@ -2788,7 +2788,7 @@ __dw_hdmi_probe(struct platform_device *pdev,
 err_isfr:
 	clk_disable_unprepare(hdmi->isfr_clk);
 err_res:
-	i2c_put_adapter(hdmi->ddc);
+	i2c_put_adapter(hdmi->connector.ddc);
 
 	return ERR_PTR(ret);
 }
@@ -2814,7 +2814,7 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 	if (hdmi->i2c)
 		i2c_del_adapter(&hdmi->i2c->adap);
 	else
-		i2c_put_adapter(hdmi->ddc);
+		i2c_put_adapter(hdmi->connector.ddc);
 }
 
 /* -----------------------------------------------------------------------------
-- 
2.17.1

[PATCH v3 20/22] drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index dbf35c7bc85e..e55358f0a5ba 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -26,7 +26,6 @@ struct tfp410 {
 	unsigned int		connector_type;
 
 	u32			bus_format;
-	struct i2c_adapter	*ddc;
 	struct gpio_desc	*hpd;
 	int			hpd_irq;
 	struct delayed_work	hpd_work;
@@ -55,10 +54,10 @@ static int tfp410_get_modes(struct drm_connector *connector)
 	struct edid *edid;
 	int ret;
 
-	if (!dvi->ddc)
+	if (!dvi->connector.ddc)
 		goto fallback;
 
-	edid = drm_get_edid(connector, dvi->ddc);
+	edid = drm_get_edid(connector, dvi->connector.ddc);
 	if (!edid) {
 		DRM_INFO("EDID read failed. Fallback to standard modes\n");
 		goto fallback;
@@ -98,8 +97,8 @@ tfp410_connector_detect(struct drm_connector *connector, bool force)
 			return connector_status_disconnected;
 	}
 
-	if (dvi->ddc) {
-		if (drm_probe_ddc(dvi->ddc))
+	if (dvi->connector.ddc) {
+		if (drm_probe_ddc(dvi->connector.ddc))
 			return connector_status_connected;
 		else
 			return connector_status_disconnected;
@@ -297,8 +296,8 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi)
 	if (!ddc_phandle)
 		goto fail;
 
-	dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
-	if (dvi->ddc)
+	dvi->connector.ddc = of_get_i2c_adapter_by_node(ddc_phandle);
+	if (dvi->connector.ddc)
 		dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
 	else
 		ret = -EPROBE_DEFER;
@@ -367,7 +366,7 @@ static int tfp410_init(struct device *dev, bool i2c)
 
 	return 0;
 fail:
-	i2c_put_adapter(dvi->ddc);
+	i2c_put_adapter(dvi->connector.ddc);
 	if (dvi->hpd)
 		gpiod_put(dvi->hpd);
 	return ret;
@@ -382,8 +381,8 @@ static int tfp410_fini(struct device *dev)
 
 	drm_bridge_remove(&dvi->bridge);
 
-	if (dvi->ddc)
-		i2c_put_adapter(dvi->ddc);
+	if (dvi->connector.ddc)
+		i2c_put_adapter(dvi->connector.ddc);
 	if (dvi->hpd)
 		gpiod_put(dvi->hpd);
 
-- 
2.17.1

[PATCH v3 21/22] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 73b2ede773d3..5f8a7e3818b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			goto failed;
 		amdgpu_connector->con_priv = amdgpu_dig_connector;
 		if (i2c_bus->valid) {
-			amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-			if (amdgpu_connector->ddc_bus)
+			struct amdgpu_connector *acn = amdgpu_connector;
+
+			acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+			if (acn->ddc_bus) {
 				has_aux = true;
-			else
+				connector->ddc = &acn->ddc_bus->adapter;
+			} else {
 				DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			}
 		}
 		switch (connector_type) {
 		case DRM_MODE_CONNECTOR_VGA:
@@ -1662,9 +1666,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			amdgpu_connector->dac_load_detect = true;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1682,9 +1690,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			amdgpu_connector->dac_load_detect = true;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1707,9 +1719,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1757,9 +1773,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      adev->mode_info.coherent_mode_property,
@@ -1799,11 +1819,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dp_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (acn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &acn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&amdgpu_connector->base.base,
@@ -1841,11 +1865,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_edp_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (acn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &acn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      dev->mode_config.scaling_mode_property,
@@ -1862,9 +1890,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
 			drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_lvds_funcs, connector_type);
 			drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_lvds_helper_funcs);
 			if (i2c_bus->valid) {
-				amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
-				if (!amdgpu_connector->ddc_bus)
+				struct amdgpu_connector *acn = amdgpu_connector;
+
+				acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
+				if (!acn->ddc_bus)
 					DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &acn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&amdgpu_connector->base.base,
 						      dev->mode_config.scaling_mode_property,
-- 
2.17.1

[PATCH v3 22/22] drm/radeon: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

Use the ddc pointer provided by the generic connector.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
 drivers/gpu/drm/radeon/radeon_connectors.c | 82 +++++++++++++++++-----
 1 file changed, 63 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index c60d1a44d22a..a876e51d275a 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1946,11 +1946,15 @@ radeon_add_atom_connector(struct drm_device *dev,
 		radeon_dig_connector->igp_lane_info = igp_lane_info;
 		radeon_connector->con_priv = radeon_dig_connector;
 		if (i2c_bus->valid) {
-			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-			if (radeon_connector->ddc_bus)
+			struct radeon_connector *rcn = radeon_connector;
+
+			rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+			if (rcn->ddc_bus) {
 				has_aux = true;
-			else
+				connector->ddc = &rcn->ddc_bus->adapter;
+			} else {
 				DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			}
 		}
 		switch (connector_type) {
 		case DRM_MODE_CONNECTOR_VGA:
@@ -2045,9 +2049,13 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_vga_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_vga_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (!rcn->ddc_bus)
 					DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &rcn->ddc_bus->adapter;
 			}
 			radeon_connector->dac_load_detect = true;
 			drm_object_attach_property(&radeon_connector->base.base,
@@ -2070,9 +2078,13 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_vga_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_vga_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (!rcn->ddc_bus)
 					DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &rcn->ddc_bus->adapter;
 			}
 			radeon_connector->dac_load_detect = true;
 			drm_object_attach_property(&radeon_connector->base.base,
@@ -2101,9 +2113,13 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_dvi_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_dvi_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (!rcn->ddc_bus)
 					DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &rcn->ddc_bus->adapter;
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&radeon_connector->base.base,
@@ -2158,9 +2174,13 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_dvi_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_dvi_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (!rcn->ddc_bus)
 					DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &rcn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&radeon_connector->base.base,
 						      rdev->mode_info.coherent_mode_property,
@@ -2208,11 +2228,15 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_dp_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_dp_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (rcn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &rcn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			subpixel_order = SubPixelHorizontalRGB;
 			drm_object_attach_property(&radeon_connector->base.base,
@@ -2258,11 +2282,15 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_edp_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_dp_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (rcn->ddc_bus) {
 					has_aux = true;
-				else
+					connector->ddc = &rcn->ddc_bus->adapter;
+				} else {
 					DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				}
 			}
 			drm_object_attach_property(&radeon_connector->base.base,
 						      dev->mode_config.scaling_mode_property,
@@ -2297,9 +2325,13 @@ radeon_add_atom_connector(struct drm_device *dev,
 			drm_connector_init(dev, &radeon_connector->base, &radeon_lvds_connector_funcs, connector_type);
 			drm_connector_helper_add(&radeon_connector->base, &radeon_lvds_connector_helper_funcs);
 			if (i2c_bus->valid) {
-				radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
-				if (!radeon_connector->ddc_bus)
+				struct radeon_connector *rcn = radeon_connector;
+
+				rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
+				if (!rcn->ddc_bus)
 					DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+				else
+					connector->ddc = &rcn->ddc_bus->adapter;
 			}
 			drm_object_attach_property(&radeon_connector->base.base,
 						      dev->mode_config.scaling_mode_property,
@@ -2384,6 +2416,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
 				DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			else
+				connector->ddc =
+					&radeon_connector->ddc_bus->adapter;
 		}
 		radeon_connector->dac_load_detect = true;
 		drm_object_attach_property(&radeon_connector->base.base,
@@ -2401,6 +2436,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
 				DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			else
+				connector->ddc =
+					&radeon_connector->ddc_bus->adapter;
 		}
 		radeon_connector->dac_load_detect = true;
 		drm_object_attach_property(&radeon_connector->base.base,
@@ -2419,6 +2457,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
 				DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			else
+				connector->ddc =
+					&radeon_connector->ddc_bus->adapter;
 		}
 		if (connector_type == DRM_MODE_CONNECTOR_DVII) {
 			radeon_connector->dac_load_detect = true;
@@ -2464,6 +2505,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
 			if (!radeon_connector->ddc_bus)
 				DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
+			else
+				connector->ddc =
+					&radeon_connector->ddc_bus->adapter;
 		}
 		drm_object_attach_property(&radeon_connector->base.base,
 					      dev->mode_config.scaling_mode_property,
-- 
2.17.1

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Laurent Pinchart]

Hi Andrzej,

Just FYI, I have a patch series that reworks how bridges and connectors
are handled, and it will heavily conflict with this. The purpose of the
two series isn't the same, so both make sense. I will post the patches
this weekend, and will then review this series in that context.
Hopefully we'll get the best of both worlds :-)

On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> ddcutil:
> 
> ddcutil -b 2 getvcp 0x10
> VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> 
> The first patch in the series adds struct i2c_adapter pointer to struct
> drm_connector. If the field is used by a particular driver, then an
> appropriate symbolic link is created by the generic code, which is also added
> by this patch.
> 
> The second patch is an example of how to convert a driver to this new scheme.
> 
> v1..v2:
> 
> - used fixed name "ddc" for the symbolic link in order to make it easy for
> userspace to find the i2c adapter
> 
> v2..v3:
> 
> - converted as many drivers as possible.
> 
> PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!
> 
> Andrzej Pietrasiewicz (22):
>   drm: Include ddc adapter pointer in struct drm_connector
>   drm/exynos: Provide ddc symlink in connector's sysfs
>   drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
>   drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
>   drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
>   drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
>     directory
>   drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
>   drm/tegra: Provide ddc symlink in output connector sysfs directory
>   drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
>   drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
>   drm/vc4: Provide ddc symlink in connector sysfs directory
>   drm: zte: Provide ddc symlink in hdmi connector sysfs directory
>   drm: zte: Provide ddc symlink in vga connector sysfs directory
>   drm/tilcdc: Provide ddc symlink in connector sysfs directory
>   drm: sti: Provide ddc symlink in hdmi connector sysfs directory
>   drm/mgag200: Provide ddc symlink in connector sysfs directory
>   drm/ast: Provide ddc symlink in connector sysfs directory
>   drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
>     directory
>   drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
>   drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
>     directory
>   drm/amdgpu: Provide ddc symlink in connector sysfs directory
>   drm/radeon: Provide ddc symlink in connector sysfs directory
> 
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 +++++++++++-----
>  drivers/gpu/drm/ast/ast_mode.c                |  1 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c         | 19 ++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 40 ++++-----
>  drivers/gpu/drm/bridge/ti-tfp410.c            | 19 ++---
>  drivers/gpu/drm/drm_sysfs.c                   |  7 ++
>  drivers/gpu/drm/exynos/exynos_hdmi.c          | 11 ++-
>  drivers/gpu/drm/imx/imx-ldb.c                 | 13 ++-
>  drivers/gpu/drm/imx/imx-tve.c                 |  8 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |  9 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c        |  1 +
>  drivers/gpu/drm/msm/hdmi/hdmi_connector.c     |  1 +
>  drivers/gpu/drm/radeon/radeon_connectors.c    | 82 ++++++++++++++-----
>  drivers/gpu/drm/rockchip/inno_hdmi.c          | 17 ++--
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c        | 17 ++--
>  drivers/gpu/drm/sti/sti_hdmi.c                |  1 +
>  drivers/gpu/drm/sun4i/sun4i_hdmi.h            |  1 -
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c        | 14 ++--
>  drivers/gpu/drm/tegra/drm.h                   |  1 -
>  drivers/gpu/drm/tegra/output.c                | 12 +--
>  drivers/gpu/drm/tegra/sor.c                   |  6 +-
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c        |  1 +
>  drivers/gpu/drm/vc4/vc4_hdmi.c                | 16 ++--
>  drivers/gpu/drm/zte/zx_hdmi.c                 | 25 ++----
>  drivers/gpu/drm/zte/zx_vga.c                  | 25 ++----
>  include/drm/drm_connector.h                   | 11 +++
>  26 files changed, 252 insertions(+), 176 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Regards,

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

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Daniel Vetter]

On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> It is difficult for a user to know which of the i2c adapters is for which
> drm connector. This series addresses this problem.
> 
> The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> ddcutil:
> 
> ddcutil -b 2 getvcp 0x10
> VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> 
> The first patch in the series adds struct i2c_adapter pointer to struct
> drm_connector. If the field is used by a particular driver, then an
> appropriate symbolic link is created by the generic code, which is also added
> by this patch.
> 
> The second patch is an example of how to convert a driver to this new scheme.
> 
> v1..v2:
> 
> - used fixed name "ddc" for the symbolic link in order to make it easy for
> userspace to find the i2c adapter
> 
> v2..v3:
> 
> - converted as many drivers as possible.
> 
> PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!

There's a lot more drivers than this I think (i915 is absent as an
example, but there should be tons more). Why are those not possible?
-Daniel

> 
> Andrzej Pietrasiewicz (22):
>   drm: Include ddc adapter pointer in struct drm_connector
>   drm/exynos: Provide ddc symlink in connector's sysfs
>   drm: rockchip: Provide ddc symlink in rk3066_hdmi sysfs directory
>   drm: rockchip: Provide ddc symlink in inno_hdmi sysfs directory
>   drm/msm/hdmi: Provide ddc symlink in hdmi connector sysfs directory
>   drm/sun4i: hdmi: Provide ddc symlink in sun4i hdmi connector sysfs
>     directory
>   drm/mediatek: Provide ddc symlink in hdmi connector sysfs directory
>   drm/tegra: Provide ddc symlink in output connector sysfs directory
>   drm/imx: imx-ldb: Provide ddc symlink in connector's sysfs
>   drm/imx: imx-tve: Provide ddc symlink in connector's sysfs
>   drm/vc4: Provide ddc symlink in connector sysfs directory
>   drm: zte: Provide ddc symlink in hdmi connector sysfs directory
>   drm: zte: Provide ddc symlink in vga connector sysfs directory
>   drm/tilcdc: Provide ddc symlink in connector sysfs directory
>   drm: sti: Provide ddc symlink in hdmi connector sysfs directory
>   drm/mgag200: Provide ddc symlink in connector sysfs directory
>   drm/ast: Provide ddc symlink in connector sysfs directory
>   drm/bridge: dumb-vga-dac: Provide ddc symlink in connector sysfs
>     directory
>   drm/bridge: dw-hdmi: Provide ddc symlink in connector sysfs directory
>   drm/bridge: ti-tfp410: Provide ddc symlink in connector sysfs
>     directory
>   drm/amdgpu: Provide ddc symlink in connector sysfs directory
>   drm/radeon: Provide ddc symlink in connector sysfs directory
> 
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 +++++++++++-----
>  drivers/gpu/drm/ast/ast_mode.c                |  1 +
>  drivers/gpu/drm/bridge/dumb-vga-dac.c         | 19 ++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     | 40 ++++-----
>  drivers/gpu/drm/bridge/ti-tfp410.c            | 19 ++---
>  drivers/gpu/drm/drm_sysfs.c                   |  7 ++
>  drivers/gpu/drm/exynos/exynos_hdmi.c          | 11 ++-
>  drivers/gpu/drm/imx/imx-ldb.c                 | 13 ++-
>  drivers/gpu/drm/imx/imx-tve.c                 |  8 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |  9 +-
>  drivers/gpu/drm/mgag200/mgag200_mode.c        |  1 +
>  drivers/gpu/drm/msm/hdmi/hdmi_connector.c     |  1 +
>  drivers/gpu/drm/radeon/radeon_connectors.c    | 82 ++++++++++++++-----
>  drivers/gpu/drm/rockchip/inno_hdmi.c          | 17 ++--
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c        | 17 ++--
>  drivers/gpu/drm/sti/sti_hdmi.c                |  1 +
>  drivers/gpu/drm/sun4i/sun4i_hdmi.h            |  1 -
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c        | 14 ++--
>  drivers/gpu/drm/tegra/drm.h                   |  1 -
>  drivers/gpu/drm/tegra/output.c                | 12 +--
>  drivers/gpu/drm/tegra/sor.c                   |  6 +-
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c        |  1 +
>  drivers/gpu/drm/vc4/vc4_hdmi.c                | 16 ++--
>  drivers/gpu/drm/zte/zx_hdmi.c                 | 25 ++----
>  drivers/gpu/drm/zte/zx_vga.c                  | 25 ++----
>  include/drm/drm_connector.h                   | 11 +++
>  26 files changed, 252 insertions(+), 176 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Thomas Zimmermann]

Hi

I like the idea, but would prefer a more structured approach.

Setting connector->ddc before calling drm_sysfs_connector_add() seems
error prone. The dependency is not really clear from the code or interfaces.

The other thing is that drivers I mostly work on, ast and mgag200, have
code like this:

  struct ast_i2c_chan {
	struct i2c_adapter adapter;
	struct drm_device *dev;
	struct i2c_algo_bit_data bit;
  };

  struct ast_connector {
	struct drm_connector base;
	struct ast_i2c_chan *i2c;
  };

It already encodes the connection between connector and ddc adapter.

I suggest to introduce struct drm_i2c_adapter

  struct drm_i2c_adapter {
	struct i2c_adapter base;
	struct drm_connector *connector;
  };

and convert drivers over to it. Ast would look like this:

  struct ast_i2c_chan {
	struct drm_i2c_adapter adapter;
	struct i2c_algo_bit_data bit;
  };

Two new sysfs functions would set up and remove the file.

  int drm_sysfs_i2c_adapter_add(struct drm_i2c_adapter *i2c);
  void drm_sysfs_i2c_adapter_remove(struct drm_i2c_adapter *i2c);

The i2c adapter, connector->ddc pointer and sysfs entry would be
initialized by

  drm_i2c_adapter_init(struct drm_i2c_adapter *i2c, struct connector
	*connector, void *algo_data);

and cleaned up by

  drm_i2c_adapter_remove(struct drm_i2c_adapter *i2c);


Thoughts?

Best regards
Thomas

Am 28.06.19 um 18:01 schrieb Andrzej Pietrasiewicz:
> Add generic code which creates symbolic links in sysfs, pointing to ddc
> interface used by a particular video output. For example:
> 
> ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> 	-> ../../../../soc/13880000.i2c/i2c-2
> 
> This makes it easy for user to associate a display with its ddc adapter
> and use e.g. ddcutil to control the chosen monitor.
> 
> This patch adds an i2c_adapter pointer to struct drm_connector. Particular
> drivers can then use it instead of using their own private instance. If a
> connector contains a ddc, then create a symbolic link in sysfs.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_sysfs.c |  7 +++++++
>  include/drm/drm_connector.h | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ad10810bc972..26d359b39785 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -294,6 +294,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	/* Let userspace know we have a new connector */
>  	drm_sysfs_hotplug_event(dev);
>  
> +	if (connector->ddc)
> +		return sysfs_create_link(&connector->kdev->kobj,
> +				 &connector->ddc->dev.kobj, "ddc");
>  	return 0;
>  }
>  
> @@ -301,6 +304,10 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  {
>  	if (!connector->kdev)
>  		return;
> +
> +	if (connector->ddc)
> +		sysfs_remove_link(&connector->kdev->kobj, "ddc");
> +
>  	DRM_DEBUG("removing \"%s\" from sysfs\n",
>  		  connector->name);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ca745d9feaf5..1ad3d1d54ba7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -23,6 +23,7 @@
>  #ifndef __DRM_CONNECTOR_H__
>  #define __DRM_CONNECTOR_H__
>  
> +#include <linux/i2c.h>
>  #include <linux/list.h>
>  #include <linux/llist.h>
>  #include <linux/ctype.h>
> @@ -1308,6 +1309,16 @@ struct drm_connector {
>  	 * [0]: progressive, [1]: interlaced
>  	 */
>  	int audio_latency[2];
> +
> +	/**
> +	 * @ddc: associated ddc adapter.
> +	 * A connector usually has its associated ddc adapter. If a driver uses
> +	 * this field, then an appropriate symbolic link is created in connector
> +	 * sysfs directory to make it easy for the user to tell which i2c
> +	 * adapter is for a particular display.
> +	 */
> +	struct i2c_adapter *ddc;
> +
>  	/**
>  	 * @null_edid_counter: track sinks that give us all zeros for the EDID.
>  	 * Needed to workaround some HW bugs where we get all 0s
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Pietrasiewicz]

Hi Thomas,

Thank you for your comments. Please see inline.

W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze:
> Hi
> 
> I like the idea, but would prefer a more structured approach.
> 
> Setting connector->ddc before calling drm_sysfs_connector_add() seems
> error prone. The dependency is not really clear from the code or interfaces.
> 
> The other thing is that drivers I mostly work on, ast and mgag200, have
> code like this:
> 
>    struct ast_i2c_chan {
> 	struct i2c_adapter adapter;
> 	struct drm_device *dev;
> 	struct i2c_algo_bit_data bit;
>    };
> 
>    struct ast_connector {
> 	struct drm_connector base;
> 	struct ast_i2c_chan *i2c;
>    };
> 
> It already encodes the connection between connector and ddc adapter.
> 
> I suggest to introduce struct drm_i2c_adapter
> 
>    struct drm_i2c_adapter {
> 	struct i2c_adapter base;
> 	struct drm_connector *connector;
>    };
> 
> and convert drivers over to it. Ast would look like this:
> 
>    struct ast_i2c_chan {
> 	struct drm_i2c_adapter adapter;
> 	struct i2c_algo_bit_data bit;
>    };
> 

There are few flavors of these drivers. In some of them
the i2c_adapter for ddc is allocated and initialized by
the driver, which must provide a place in memory to hold
the adapter. ast is an example of this approach.

But there are others, such as for example exynos_hdmi.c.
It gets its ddc adapter with of_find_i2c_adapter_by_node()
and merely stores a pointer to it, while not managing the
memory needed to hold the i2c_adapter.

Do you have any idea how to accommodate these various
flavors of drivers in the scheme you propose?

Andrzej


_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Thomas Zimmermann]

Hi

Am 01.07.19 um 15:27 schrieb Andrzej Pietrasiewicz:
> Hi Thomas,
> 
> Thank you for your comments. Please see inline.
> 
> W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze:
>> Hi
>>
>> I like the idea, but would prefer a more structured approach.
>>
>> Setting connector->ddc before calling drm_sysfs_connector_add() seems
>> error prone. The dependency is not really clear from the code or
>> interfaces.
>>
>> The other thing is that drivers I mostly work on, ast and mgag200, have
>> code like this:
>>
>>    struct ast_i2c_chan {
>>     struct i2c_adapter adapter;
>>     struct drm_device *dev;
>>     struct i2c_algo_bit_data bit;
>>    };
>>
>>    struct ast_connector {
>>     struct drm_connector base;
>>     struct ast_i2c_chan *i2c;
>>    };
>>
>> It already encodes the connection between connector and ddc adapter.
>>
>> I suggest to introduce struct drm_i2c_adapter
>>
>>    struct drm_i2c_adapter {
>>     struct i2c_adapter base;
>>     struct drm_connector *connector;
>>    };
>>
>> and convert drivers over to it. Ast would look like this:
>>
>>    struct ast_i2c_chan {
>>     struct drm_i2c_adapter adapter;
>>     struct i2c_algo_bit_data bit;
>>    };
>>
> 
> There are few flavors of these drivers. In some of them
> the i2c_adapter for ddc is allocated and initialized by
> the driver, which must provide a place in memory to hold
> the adapter. ast is an example of this approach.
> 
> But there are others, such as for example exynos_hdmi.c.
> It gets its ddc adapter with of_find_i2c_adapter_by_node()
> and merely stores a pointer to it, while not managing the
> memory needed to hold the i2c_adapter.

I see.

> Do you have any idea how to accommodate these various
> flavors of drivers in the scheme you propose?

Something like

  struct drm_i2c_adapter {
	struct i2c_adapter *adapter;
	struct drm_connector *connector;
  };

with adapter either being allocated dynamically or acquired via
of_find_i2c_adapter_by_node(); with separate init and finish functions
for each variant. But it looks like over-abstraction to me. Without
prototyping the idea, I cannot say if it's worth the effort.

For something more simple, maybe just have a function to attach an i2c
adapter to a connector:

  drm_connector_attach_i2c_adapter(connector, i2c_adapter)

which sets the connector's ddc pointer and creates the sysfs entry if
the connector's entry is already present.

Best regards
Thomas

> Andrzej
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Emil Velikov]

On Fri, 28 Jun 2019 at 17:45, Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> wrote:
>
> On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> > It is difficult for a user to know which of the i2c adapters is for which
> > drm connector. This series addresses this problem.
> >
> > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> >
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >       -> ../../../../soc/13880000.i2c/i2c-2
> >
> > The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> > ddcutil:
> >
> > ddcutil -b 2 getvcp 0x10
> > VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> >
> > The first patch in the series adds struct i2c_adapter pointer to struct
> > drm_connector. If the field is used by a particular driver, then an
> > appropriate symbolic link is created by the generic code, which is also added
> > by this patch.
> >
> > The second patch is an example of how to convert a driver to this new scheme.
> >
> > v1..v2:
> >
> > - used fixed name "ddc" for the symbolic link in order to make it easy for
> > userspace to find the i2c adapter
> >
> > v2..v3:
> >
> > - converted as many drivers as possible.
> >
> > PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!
>
> There's a lot more drivers than this I think (i915 is absent as an
> example, but there should be tons more). Why are those not possible?

While I fully agree there are more drivers, at the same time I wonder.
Is it a good idea to expect all of those to be fixed in one go and
block patches addressing 15+ drivers?

Personally I think it's reasonable to have this, alongside a TODO
entry for other drivers.

HTH
Emil

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Emil Velikov]

On Fri, 28 Jun 2019 at 17:45, Daniel Vetter <daniel-/w4YWyX8dFk@public.gmane.org> wrote:
>
> On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> > It is difficult for a user to know which of the i2c adapters is for which
> > drm connector. This series addresses this problem.
> >
> > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> >
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >       -> ../../../../soc/13880000.i2c/i2c-2
> >
> > The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> > ddcutil:
> >
> > ddcutil -b 2 getvcp 0x10
> > VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> >
> > The first patch in the series adds struct i2c_adapter pointer to struct
> > drm_connector. If the field is used by a particular driver, then an
> > appropriate symbolic link is created by the generic code, which is also added
> > by this patch.
> >
> > The second patch is an example of how to convert a driver to this new scheme.
> >
> > v1..v2:
> >
> > - used fixed name "ddc" for the symbolic link in order to make it easy for
> > userspace to find the i2c adapter
> >
> > v2..v3:
> >
> > - converted as many drivers as possible.
> >
> > PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!
>
> There's a lot more drivers than this I think (i915 is absent as an
> example, but there should be tons more). Why are those not possible?

While I fully agree there are more drivers, at the same time I wonder.
Is it a good idea to expect all of those to be fixed in one go and
block patches addressing 15+ drivers?

Personally I think it's reasonable to have this, alongside a TODO
entry for other drivers.

HTH
Emil

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Emil Velikov]

On Fri, 28 Jun 2019 at 17:45, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> > It is difficult for a user to know which of the i2c adapters is for which
> > drm connector. This series addresses this problem.
> >
> > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> >
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >       -> ../../../../soc/13880000.i2c/i2c-2
> >
> > The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> > ddcutil:
> >
> > ddcutil -b 2 getvcp 0x10
> > VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> >
> > The first patch in the series adds struct i2c_adapter pointer to struct
> > drm_connector. If the field is used by a particular driver, then an
> > appropriate symbolic link is created by the generic code, which is also added
> > by this patch.
> >
> > The second patch is an example of how to convert a driver to this new scheme.
> >
> > v1..v2:
> >
> > - used fixed name "ddc" for the symbolic link in order to make it easy for
> > userspace to find the i2c adapter
> >
> > v2..v3:
> >
> > - converted as many drivers as possible.
> >
> > PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!
>
> There's a lot more drivers than this I think (i915 is absent as an
> example, but there should be tons more). Why are those not possible?

While I fully agree there are more drivers, at the same time I wonder.
Is it a good idea to expect all of those to be fixed in one go and
block patches addressing 15+ drivers?

Personally I think it's reasonable to have this, alongside a TODO
entry for other drivers.

HTH
Emil
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Emil Velikov]

On Fri, 28 Jun 2019 at 17:45, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Fri, Jun 28, 2019 at 06:01:14PM +0200, Andrzej Pietrasiewicz wrote:
> > It is difficult for a user to know which of the i2c adapters is for which
> > drm connector. This series addresses this problem.
> >
> > The idea is to have a symbolic link in connector's sysfs directory, e.g.:
> >
> > ls -l /sys/class/drm/card0-HDMI-A-1/ddc
> > lrwxrwxrwx 1 root root 0 Jun 24 10:42 /sys/class/drm/card0-HDMI-A-1/ddc \
> >       -> ../../../../soc/13880000.i2c/i2c-2
> >
> > The user then knows that their card0-HDMI-A-1 uses i2c-2 and can e.g. run
> > ddcutil:
> >
> > ddcutil -b 2 getvcp 0x10
> > VCP code 0x10 (Brightness                    ): current value =    90, max value =   100
> >
> > The first patch in the series adds struct i2c_adapter pointer to struct
> > drm_connector. If the field is used by a particular driver, then an
> > appropriate symbolic link is created by the generic code, which is also added
> > by this patch.
> >
> > The second patch is an example of how to convert a driver to this new scheme.
> >
> > v1..v2:
> >
> > - used fixed name "ddc" for the symbolic link in order to make it easy for
> > userspace to find the i2c adapter
> >
> > v2..v3:
> >
> > - converted as many drivers as possible.
> >
> > PATCHES 3/22-22/22 SHOULD BE CONSIDERED RFC!
>
> There's a lot more drivers than this I think (i915 is absent as an
> example, but there should be tons more). Why are those not possible?

While I fully agree there are more drivers, at the same time I wonder.
Is it a good idea to expect all of those to be fixed in one go and
block patches addressing 15+ drivers?

Personally I think it's reasonable to have this, alongside a TODO
entry for other drivers.

HTH
Emil
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 21/22] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Use the ddc pointer provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
>  1 file changed, 51 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 73b2ede773d3..5f8a7e3818b9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         goto failed;
>                 amdgpu_connector->con_priv = amdgpu_dig_connector;
>                 if (i2c_bus->valid) {
> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                       if (amdgpu_connector->ddc_bus)
> +                       struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                       if (acn->ddc_bus) {

This hunk seems pointless unless I'm missing something.  Can you drop
this hunk?  Same comment on each instance of this below.  This also
only covers the legacy modesetting code which is not used by default
on most chips.  The DC code in amd/display/ is probably more relevant.

Alex

>                                 has_aux = true;
> -                       else
> +                               connector->ddc = &acn->ddc_bus->adapter;
> +                       } else {
>                                 DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       }
>                 }
>                 switch (connector_type) {
>                 case DRM_MODE_CONNECTOR_VGA:
> @@ -1662,9 +1666,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         amdgpu_connector->dac_load_detect = true;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1682,9 +1690,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_vga_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_vga_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         amdgpu_connector->dac_load_detect = true;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1707,9 +1719,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1757,9 +1773,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dvi_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dvi_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       adev->mode_info.coherent_mode_property,
> @@ -1799,11 +1819,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_dp_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (acn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&amdgpu_connector->base.base,
> @@ -1841,11 +1865,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_edp_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_dp_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (acn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> @@ -1862,9 +1890,13 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>                         drm_connector_init(dev, &amdgpu_connector->base, &amdgpu_connector_lvds_funcs, connector_type);
>                         drm_connector_helper_add(&amdgpu_connector->base, &amdgpu_connector_lvds_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> -                               if (!amdgpu_connector->ddc_bus)
> +                               struct amdgpu_connector *acn = amdgpu_connector;
> +
> +                               acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> +                               if (!acn->ddc_bus)
>                                         DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &acn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&amdgpu_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 22/22] drm/radeon: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Use the ddc pointer provided by the generic connector.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/radeon/radeon_connectors.c | 82 +++++++++++++++++-----
>  1 file changed, 63 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index c60d1a44d22a..a876e51d275a 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -1946,11 +1946,15 @@ radeon_add_atom_connector(struct drm_device *dev,
>                 radeon_dig_connector->igp_lane_info = igp_lane_info;
>                 radeon_connector->con_priv = radeon_dig_connector;
>                 if (i2c_bus->valid) {
> -                       radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                       if (radeon_connector->ddc_bus)
> +                       struct radeon_connector *rcn = radeon_connector;
> +
> +                       rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                       if (rcn->ddc_bus) {

Same comment as the amdgpu patch.  I think you can drop this hunk.  In
fact, you do in some of the other code paths below.  Same comment on
the other paths below.

Alex

>                                 has_aux = true;
> -                       else
> +                               connector->ddc = &rcn->ddc_bus->adapter;
> +                       } else {
>                                 DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       }
>                 }
>                 switch (connector_type) {
>                 case DRM_MODE_CONNECTOR_VGA:
> @@ -2045,9 +2049,13 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_vga_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_vga_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (!radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (!rcn->ddc_bus)
>                                         DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
>                         }
>                         radeon_connector->dac_load_detect = true;
>                         drm_object_attach_property(&radeon_connector->base.base,
> @@ -2070,9 +2078,13 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_vga_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_vga_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (!radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (!rcn->ddc_bus)
>                                         DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
>                         }
>                         radeon_connector->dac_load_detect = true;
>                         drm_object_attach_property(&radeon_connector->base.base,
> @@ -2101,9 +2113,13 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_dvi_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_dvi_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (!radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (!rcn->ddc_bus)
>                                         DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&radeon_connector->base.base,
> @@ -2158,9 +2174,13 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_dvi_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_dvi_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (!radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (!rcn->ddc_bus)
>                                         DRM_ERROR("HDMI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&radeon_connector->base.base,
>                                                       rdev->mode_info.coherent_mode_property,
> @@ -2208,11 +2228,15 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_dp_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_dp_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (rcn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         subpixel_order = SubPixelHorizontalRGB;
>                         drm_object_attach_property(&radeon_connector->base.base,
> @@ -2258,11 +2282,15 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_edp_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_dp_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (rcn->ddc_bus) {
>                                         has_aux = true;
> -                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
> +                               } else {
>                                         DRM_ERROR("DP: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               }
>                         }
>                         drm_object_attach_property(&radeon_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> @@ -2297,9 +2325,13 @@ radeon_add_atom_connector(struct drm_device *dev,
>                         drm_connector_init(dev, &radeon_connector->base, &radeon_lvds_connector_funcs, connector_type);
>                         drm_connector_helper_add(&radeon_connector->base, &radeon_lvds_connector_helper_funcs);
>                         if (i2c_bus->valid) {
> -                               radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> -                               if (!radeon_connector->ddc_bus)
> +                               struct radeon_connector *rcn = radeon_connector;
> +
> +                               rcn->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
> +                               if (!rcn->ddc_bus)
>                                         DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                               else
> +                                       connector->ddc = &rcn->ddc_bus->adapter;
>                         }
>                         drm_object_attach_property(&radeon_connector->base.base,
>                                                       dev->mode_config.scaling_mode_property,
> @@ -2384,6 +2416,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
>                         radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>                         if (!radeon_connector->ddc_bus)
>                                 DRM_ERROR("VGA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       else
> +                               connector->ddc =
> +                                       &radeon_connector->ddc_bus->adapter;
>                 }
>                 radeon_connector->dac_load_detect = true;
>                 drm_object_attach_property(&radeon_connector->base.base,
> @@ -2401,6 +2436,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
>                         radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>                         if (!radeon_connector->ddc_bus)
>                                 DRM_ERROR("DVIA: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       else
> +                               connector->ddc =
> +                                       &radeon_connector->ddc_bus->adapter;
>                 }
>                 radeon_connector->dac_load_detect = true;
>                 drm_object_attach_property(&radeon_connector->base.base,
> @@ -2419,6 +2457,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
>                         radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>                         if (!radeon_connector->ddc_bus)
>                                 DRM_ERROR("DVI: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       else
> +                               connector->ddc =
> +                                       &radeon_connector->ddc_bus->adapter;
>                 }
>                 if (connector_type == DRM_MODE_CONNECTOR_DVII) {
>                         radeon_connector->dac_load_detect = true;
> @@ -2464,6 +2505,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
>                         radeon_connector->ddc_bus = radeon_i2c_lookup(rdev, i2c_bus);
>                         if (!radeon_connector->ddc_bus)
>                                 DRM_ERROR("LVDS: Failed to assign ddc bus! Check dmesg for i2c errors.\n");
> +                       else
> +                               connector->ddc =
> +                                       &radeon_connector->ddc_bus->adapter;
>                 }
>                 drm_object_attach_property(&radeon_connector->base.base,
>                                               dev->mode_config.scaling_mode_property,
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 21/22] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Andrzej Pietrasiewicz]

W dniu 02.07.2019 o 22:54, Alex Deucher pisze:
> On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>>
>> Use the ddc pointer provided by the generic connector.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>> ---
>>   .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
>>   1 file changed, 51 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> index 73b2ede773d3..5f8a7e3818b9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
>> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
>>                          goto failed;
>>                  amdgpu_connector->con_priv = amdgpu_dig_connector;
>>                  if (i2c_bus->valid) {
>> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
>> -                       if (amdgpu_connector->ddc_bus)
>> +                       struct amdgpu_connector *acn = amdgpu_connector;
>> +
>> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
>> +                       if (acn->ddc_bus) {
> 
> This hunk seems pointless unless I'm missing something.  Can you drop
> this hunk?  Same comment on each instance of this below.  This also
> only covers the legacy modesetting code which is not used by default
> on most chips.  The DC code in amd/display/ is probably more relevant.
> 

If I don't do that checkpatch reports that lines I created exceed 80 characters.

Andrzej
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Andrzej Pietrasiewicz]

W dniu 28.06.2019 o 18:11, Laurent Pinchart pisze:
> Hi Andrzej,
> 
> Just FYI, I have a patch series that reworks how bridges and connectors
> are handled, and it will heavily conflict with this. The purpose of the
> two series isn't the same, so both make sense. I will post the patches
> this weekend, and will then review this series in that context.
> Hopefully we'll get the best of both worlds :-)

Hi Laurent,

Did you have a chance to review my patch series?

Andrzej

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 00/22] Associate ddc adapters with connectors

[Laurent Pinchart]

Hi Andrzej,

On Fri, Jul 05, 2019 at 10:38:27AM +0200, Andrzej Pietrasiewicz wrote:
> W dniu 28.06.2019 o 18:11, Laurent Pinchart pisze:
> > Hi Andrzej,
> > 
> > Just FYI, I have a patch series that reworks how bridges and connectors
> > are handled, and it will heavily conflict with this. The purpose of the
> > two series isn't the same, so both make sense. I will post the patches
> > this weekend, and will then review this series in that context.
> > Hopefully we'll get the best of both worlds :-)
> 
> Hi Laurent,
> 
> Did you have a chance to review my patch series?

Not yet I'm afraid. I've been fairly busy this week, and coupled with
some health issues (but I'm feeling better now, so nothing to worry
about) it delayed my reviews. I'll get to it as soon as possible. Thank
you for pinging me.

-- 
Regards,

Laurent Pinchart
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Hajda]

On 01.07.2019 16:41, Thomas Zimmermann wrote:
> Hi
>
> Am 01.07.19 um 15:27 schrieb Andrzej Pietrasiewicz:
>> Hi Thomas,
>>
>> Thank you for your comments. Please see inline.
>>
>> W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze:
>>> Hi
>>>
>>> I like the idea, but would prefer a more structured approach.
>>>
>>> Setting connector->ddc before calling drm_sysfs_connector_add() seems
>>> error prone. The dependency is not really clear from the code or
>>> interfaces.
>>>
>>> The other thing is that drivers I mostly work on, ast and mgag200, have
>>> code like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct i2c_adapter adapter;
>>>     struct drm_device *dev;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>>>    struct ast_connector {
>>>     struct drm_connector base;
>>>     struct ast_i2c_chan *i2c;
>>>    };
>>>
>>> It already encodes the connection between connector and ddc adapter.
>>>
>>> I suggest to introduce struct drm_i2c_adapter
>>>
>>>    struct drm_i2c_adapter {
>>>     struct i2c_adapter base;
>>>     struct drm_connector *connector;
>>>    };
>>>
>>> and convert drivers over to it. Ast would look like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct drm_i2c_adapter adapter;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>> There are few flavors of these drivers. In some of them
>> the i2c_adapter for ddc is allocated and initialized by
>> the driver, which must provide a place in memory to hold
>> the adapter. ast is an example of this approach.
>>
>> But there are others, such as for example exynos_hdmi.c.
>> It gets its ddc adapter with of_find_i2c_adapter_by_node()
>> and merely stores a pointer to it, while not managing the
>> memory needed to hold the i2c_adapter.
> I see.
>
>> Do you have any idea how to accommodate these various
>> flavors of drivers in the scheme you propose?
> Something like
>
>   struct drm_i2c_adapter {
> 	struct i2c_adapter *adapter;
> 	struct drm_connector *connector;
>   };
>
> with adapter either being allocated dynamically or acquired via
> of_find_i2c_adapter_by_node(); with separate init and finish functions
> for each variant. But it looks like over-abstraction to me. Without
> prototyping the idea, I cannot say if it's worth the effort.
>
> For something more simple, maybe just have a function to attach an i2c
> adapter to a connector:
>
>   drm_connector_attach_i2c_adapter(connector, i2c_adapter)
>
> which sets the connector's ddc pointer and creates the sysfs entry if
> the connector's entry is already present.


I am not sure if such function is really necessary. Assigning ddc field
before connector registration seems to be much simpler and quite
standard - many fields are initialized this way.


Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej



> Best regards
> Thomas
>
>> Andrzej
>>
>>


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Hajda]

On 01.07.2019 16:41, Thomas Zimmermann wrote:
> Hi
>
> Am 01.07.19 um 15:27 schrieb Andrzej Pietrasiewicz:
>> Hi Thomas,
>>
>> Thank you for your comments. Please see inline.
>>
>> W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze:
>>> Hi
>>>
>>> I like the idea, but would prefer a more structured approach.
>>>
>>> Setting connector->ddc before calling drm_sysfs_connector_add() seems
>>> error prone. The dependency is not really clear from the code or
>>> interfaces.
>>>
>>> The other thing is that drivers I mostly work on, ast and mgag200, have
>>> code like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct i2c_adapter adapter;
>>>     struct drm_device *dev;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>>>    struct ast_connector {
>>>     struct drm_connector base;
>>>     struct ast_i2c_chan *i2c;
>>>    };
>>>
>>> It already encodes the connection between connector and ddc adapter.
>>>
>>> I suggest to introduce struct drm_i2c_adapter
>>>
>>>    struct drm_i2c_adapter {
>>>     struct i2c_adapter base;
>>>     struct drm_connector *connector;
>>>    };
>>>
>>> and convert drivers over to it. Ast would look like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct drm_i2c_adapter adapter;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>> There are few flavors of these drivers. In some of them
>> the i2c_adapter for ddc is allocated and initialized by
>> the driver, which must provide a place in memory to hold
>> the adapter. ast is an example of this approach.
>>
>> But there are others, such as for example exynos_hdmi.c.
>> It gets its ddc adapter with of_find_i2c_adapter_by_node()
>> and merely stores a pointer to it, while not managing the
>> memory needed to hold the i2c_adapter.
> I see.
>
>> Do you have any idea how to accommodate these various
>> flavors of drivers in the scheme you propose?
> Something like
>
>   struct drm_i2c_adapter {
> 	struct i2c_adapter *adapter;
> 	struct drm_connector *connector;
>   };
>
> with adapter either being allocated dynamically or acquired via
> of_find_i2c_adapter_by_node(); with separate init and finish functions
> for each variant. But it looks like over-abstraction to me. Without
> prototyping the idea, I cannot say if it's worth the effort.
>
> For something more simple, maybe just have a function to attach an i2c
> adapter to a connector:
>
>   drm_connector_attach_i2c_adapter(connector, i2c_adapter)
>
> which sets the connector's ddc pointer and creates the sysfs entry if
> the connector's entry is already present.


I am not sure if such function is really necessary. Assigning ddc field
before connector registration seems to be much simpler and quite
standard - many fields are initialized this way.


Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej



> Best regards
> Thomas
>
>> Andrzej
>>
>>

Re: [PATCH v3 01/22] drm: Include ddc adapter pointer in struct drm_connector

[Andrzej Hajda]

On 01.07.2019 16:41, Thomas Zimmermann wrote:
> Hi
>
> Am 01.07.19 um 15:27 schrieb Andrzej Pietrasiewicz:
>> Hi Thomas,
>>
>> Thank you for your comments. Please see inline.
>>
>> W dniu 30.06.2019 o 10:12, Thomas Zimmermann pisze:
>>> Hi
>>>
>>> I like the idea, but would prefer a more structured approach.
>>>
>>> Setting connector->ddc before calling drm_sysfs_connector_add() seems
>>> error prone. The dependency is not really clear from the code or
>>> interfaces.
>>>
>>> The other thing is that drivers I mostly work on, ast and mgag200, have
>>> code like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct i2c_adapter adapter;
>>>     struct drm_device *dev;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>>>    struct ast_connector {
>>>     struct drm_connector base;
>>>     struct ast_i2c_chan *i2c;
>>>    };
>>>
>>> It already encodes the connection between connector and ddc adapter.
>>>
>>> I suggest to introduce struct drm_i2c_adapter
>>>
>>>    struct drm_i2c_adapter {
>>>     struct i2c_adapter base;
>>>     struct drm_connector *connector;
>>>    };
>>>
>>> and convert drivers over to it. Ast would look like this:
>>>
>>>    struct ast_i2c_chan {
>>>     struct drm_i2c_adapter adapter;
>>>     struct i2c_algo_bit_data bit;
>>>    };
>>>
>> There are few flavors of these drivers. In some of them
>> the i2c_adapter for ddc is allocated and initialized by
>> the driver, which must provide a place in memory to hold
>> the adapter. ast is an example of this approach.
>>
>> But there are others, such as for example exynos_hdmi.c.
>> It gets its ddc adapter with of_find_i2c_adapter_by_node()
>> and merely stores a pointer to it, while not managing the
>> memory needed to hold the i2c_adapter.
> I see.
>
>> Do you have any idea how to accommodate these various
>> flavors of drivers in the scheme you propose?
> Something like
>
>   struct drm_i2c_adapter {
> 	struct i2c_adapter *adapter;
> 	struct drm_connector *connector;
>   };
>
> with adapter either being allocated dynamically or acquired via
> of_find_i2c_adapter_by_node(); with separate init and finish functions
> for each variant. But it looks like over-abstraction to me. Without
> prototyping the idea, I cannot say if it's worth the effort.
>
> For something more simple, maybe just have a function to attach an i2c
> adapter to a connector:
>
>   drm_connector_attach_i2c_adapter(connector, i2c_adapter)
>
> which sets the connector's ddc pointer and creates the sysfs entry if
> the connector's entry is already present.


I am not sure if such function is really necessary. Assigning ddc field
before connector registration seems to be much simpler and quite
standard - many fields are initialized this way.


Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej



> Best regards
> Thomas
>
>> Andrzej
>>
>>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 21/22] drm/amdgpu: Provide ddc symlink in connector sysfs directory

[Alex Deucher]

On Thu, Jul 4, 2019 at 9:17 AM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> W dniu 02.07.2019 o 22:54, Alex Deucher pisze:
> > On Fri, Jun 28, 2019 at 12:31 PM Andrzej Pietrasiewicz
> > <andrzej.p@collabora.com> wrote:
> >>
> >> Use the ddc pointer provided by the generic connector.
> >>
> >> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> >> ---
> >>   .../gpu/drm/amd/amdgpu/amdgpu_connectors.c    | 70 ++++++++++++++-----
> >>   1 file changed, 51 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> index 73b2ede773d3..5f8a7e3818b9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> >> @@ -1573,11 +1573,15 @@ amdgpu_connector_add(struct amdgpu_device *adev,
> >>                          goto failed;
> >>                  amdgpu_connector->con_priv = amdgpu_dig_connector;
> >>                  if (i2c_bus->valid) {
> >> -                       amdgpu_connector->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> >> -                       if (amdgpu_connector->ddc_bus)
> >> +                       struct amdgpu_connector *acn = amdgpu_connector;
> >> +
> >> +                       acn->ddc_bus = amdgpu_i2c_lookup(adev, i2c_bus);
> >> +                       if (acn->ddc_bus) {
> >
> > This hunk seems pointless unless I'm missing something.  Can you drop
> > this hunk?  Same comment on each instance of this below.  This also
> > only covers the legacy modesetting code which is not used by default
> > on most chips.  The DC code in amd/display/ is probably more relevant.
> >
>
> If I don't do that checkpatch reports that lines I created exceed 80 characters.

Don't worry about that.

Alex
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno