(unknown),

(unknown),

[Maxime Ripard]

Subject: [PATCH v5 0/5] drm: Add Support for Passive RGB to VGA bridges

Hi,

This serie is about adding support for the RGB to VGA bridge found in
the A13-Olinuxino and the CHIP VGA adapter.

Both these boards rely on an entirely passive bridge made out of
resitor ladders that do not require any initialisation. The only thing
needed is to get the timings from the screen if available (and if not,
fall back on XGA standards), set up the display pipeline to output on
the RGB bus with the proper timings, and you're done.

This serie also fixes a bunch of bugs uncovered when trying to
increase the resolution, and hence the pixel clock, of our
pipeline. It also fixes a few bugs in the DRM driver itself that went
unnoticed before.

Let me know what you think,
Maxime

Changes from v4:
  - Removed unused functions

Changes from v3:
  - Depends on OF in Kconfig
  - Fixed typos in the driver comments
  - Removed the mention of a "passive" bridge in the bindings doc
  - Made the strcuture const
  - Removed the nops and best_encoders implementations
  - Removed the call to drm_bridge_enable in the sun4i driver

Changes from v2:
  - Changed the compatible as suggested
  - Rebased on top 4.8

Changes from v1:
  - Switch to using a vga-connector
  - Use drm_encoder bridge pointer instead of doing our own
  - Report the connector status as unknown instead of connected by
    default, and as connected only if we can retrieve the EDID.
  - Switch to of_i2c_get_adapter by node, and put the reference when done
  - Rebased on linux-next	      

Maxime Ripard (5):
  drm/sun4i: rgb: Remove the bridge enable/disable functions
  drm/bridge: Add RGB to VGA bridge support
  ARM: sun5i: a13-olinuxino: Enable VGA bridge
  ARM: multi_v7: enable VGA bridge
  ARM: sunxi: Enable VGA bridge

 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts          |  54 +++++
 arch/arm/configs/multi_v7_defconfig                |   1 +
 arch/arm/configs/sunxi_defconfig                   |   1 +
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_rgb.c                  |   6 -
 8 files changed, 341 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

-- 
2.9.3

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

No subject

[Maxime Ripard]

Subject: [PATCH v5 0/5] drm: Add Support for Passive RGB to VGA bridges

Hi,

This serie is about adding support for the RGB to VGA bridge found in
the A13-Olinuxino and the CHIP VGA adapter.

Both these boards rely on an entirely passive bridge made out of
resitor ladders that do not require any initialisation. The only thing
needed is to get the timings from the screen if available (and if not,
fall back on XGA standards), set up the display pipeline to output on
the RGB bus with the proper timings, and you're done.

This serie also fixes a bunch of bugs uncovered when trying to
increase the resolution, and hence the pixel clock, of our
pipeline. It also fixes a few bugs in the DRM driver itself that went
unnoticed before.

Let me know what you think,
Maxime

Changes from v4:
  - Removed unused functions

Changes from v3:
  - Depends on OF in Kconfig
  - Fixed typos in the driver comments
  - Removed the mention of a "passive" bridge in the bindings doc
  - Made the strcuture const
  - Removed the nops and best_encoders implementations
  - Removed the call to drm_bridge_enable in the sun4i driver

Changes from v2:
  - Changed the compatible as suggested
  - Rebased on top 4.8

Changes from v1:
  - Switch to using a vga-connector
  - Use drm_encoder bridge pointer instead of doing our own
  - Report the connector status as unknown instead of connected by
    default, and as connected only if we can retrieve the EDID.
  - Switch to of_i2c_get_adapter by node, and put the reference when done
  - Rebased on linux-next	      

Maxime Ripard (5):
  drm/sun4i: rgb: Remove the bridge enable/disable functions
  drm/bridge: Add RGB to VGA bridge support
  ARM: sun5i: a13-olinuxino: Enable VGA bridge
  ARM: multi_v7: enable VGA bridge
  ARM: sunxi: Enable VGA bridge

 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts          |  54 +++++
 arch/arm/configs/multi_v7_defconfig                |   1 +
 arch/arm/configs/sunxi_defconfig                   |   1 +
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
 drivers/gpu/drm/sun4i/sun4i_rgb.c                  |   6 -
 8 files changed, 341 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

-- 
2.9.3

[PATCH v5 1/5] drm/sun4i: rgb: Remove the bridge enable/disable functions

[Maxime Ripard]

The atomic helpers already call the drm_bridge_enable on our behalf,
there's no need to do it a second time.

Reported-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index 4e4bea6f395c..d198ad7e5323 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -155,9 +155,6 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
 	if (!IS_ERR(tcon->panel))
 		drm_panel_prepare(tcon->panel);
 
-	/* encoder->bridge can be NULL; drm_bridge_enable checks for it */
-	drm_bridge_enable(encoder->bridge);
-
 	sun4i_tcon_channel_enable(tcon, 0);
 
 	if (!IS_ERR(tcon->panel))
@@ -177,9 +174,6 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
 
 	sun4i_tcon_channel_disable(tcon, 0);
 
-	/* encoder->bridge can be NULL; drm_bridge_disable checks for it */
-	drm_bridge_disable(encoder->bridge);
-
 	if (!IS_ERR(tcon->panel))
 		drm_panel_unprepare(tcon->panel);
 }
-- 
2.9.3

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

[PATCH v5 1/5] drm/sun4i: rgb: Remove the bridge enable/disable functions

[Maxime Ripard]

The atomic helpers already call the drm_bridge_enable on our behalf,
there's no need to do it a second time.

Reported-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_rgb.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index 4e4bea6f395c..d198ad7e5323 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -155,9 +155,6 @@ static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
 	if (!IS_ERR(tcon->panel))
 		drm_panel_prepare(tcon->panel);
 
-	/* encoder->bridge can be NULL; drm_bridge_enable checks for it */
-	drm_bridge_enable(encoder->bridge);
-
 	sun4i_tcon_channel_enable(tcon, 0);
 
 	if (!IS_ERR(tcon->panel))
@@ -177,9 +174,6 @@ static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
 
 	sun4i_tcon_channel_disable(tcon, 0);
 
-	/* encoder->bridge can be NULL; drm_bridge_disable checks for it */
-	drm_bridge_disable(encoder->bridge);
-
 	if (!IS_ERR(tcon->panel))
 		drm_panel_unprepare(tcon->panel);
 }
-- 
2.9.3

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

Some boards have an entirely passive RGB to VGA bridge, based on either
DACs or resistor ladders.

Those might or might not have an i2c bus routed to the VGA connector in
order to access the screen EDIDs.

Add a bridge that doesn't do anything but expose the modes available on the
screen, either based on the EDIDs if available, or based on the XGA
standards.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
new file mode 100644
index 000000000000..a8375bc1f9cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
@@ -0,0 +1,48 @@
+Dumb RGB to VGA bridge
+----------------------
+
+This binding is aimed for dumb RGB to VGA bridges that do not require
+any configuration.
+
+Required properties:
+
+- compatible: Must be "rgb-to-vga-bridge"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+
+Example
+-------
+
+bridge {
+	compatible = "rgb-to-vga-bridge";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			vga_bridge_in: endpoint {
+				remote-endpoint = <&tcon0_out_vga>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			vga_bridge_out: endpoint {
+				remote-endpoint = <&vga_con_in>;
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b590e678052d..d690398c541c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
 	  the HDMI output of an application processor to MyDP
 	  or DisplayPort.
 
+config DRM_RGB_TO_VGA
+	tristate "Dumb RGB to VGA Bridge support"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  Support for passive RGB to VGA bridges
+
 config DRM_DW_HDMI
 	tristate
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index efdb07e878f5..3bb8cbe09fe9 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
 ccflags-y := -Iinclude/drm
 
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
+obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c b/drivers/gpu/drm/bridge/rgb-to-vga.c
new file mode 100644
index 000000000000..5ff4d4f3598f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2015-2016 Free Electrons
+ * Copyright (C) 2015-2016 NextThing Co
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/of_graph.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+struct dumb_vga {
+	struct drm_bridge	bridge;
+	struct drm_connector	connector;
+
+	struct i2c_adapter	*ddc;
+};
+
+static inline struct dumb_vga *
+drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct dumb_vga, bridge);
+}
+
+static inline struct dumb_vga *
+drm_connector_to_dumb_vga(struct drm_connector *connector)
+{
+	return container_of(connector, struct dumb_vga, connector);
+}
+
+static int dumb_vga_get_modes(struct drm_connector *connector)
+{
+	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
+	struct edid *edid;
+	int ret;
+
+	if (IS_ERR(vga->ddc))
+		goto fallback;
+
+	edid = drm_get_edid(connector, vga->ddc);
+	if (!edid) {
+		DRM_INFO("EDID readout failed, falling back to standard modes\n");
+		goto fallback;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	return drm_add_edid_modes(connector, edid);
+
+fallback:
+	/*
+	 * In case we cannot retrieve the EDIDs (broken or missing i2c
+	 * bus), fallback on the XGA standards
+	 */
+	ret = drm_add_modes_noedid(connector, 1920, 1200);
+
+	/* And prefer a mode pretty much anyone can handle */
+	drm_set_preferred_mode(connector, 1024, 768);
+
+	return ret;
+}
+
+static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs = {
+	.get_modes	= dumb_vga_get_modes,
+};
+
+static enum drm_connector_status
+dumb_vga_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
+
+	/*
+	 * Even if we have an I2C bus, we can't assume that the cable
+	 * is disconnected if drm_probe_ddc fails. Some cables don't
+	 * wire the DDC pins, or the I2C bus might not be working at
+	 * all.
+	 */
+	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
+		return connector_status_connected;
+
+	return connector_status_unknown;
+}
+
+static void
+dumb_vga_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs dumb_vga_con_funcs = {
+	.dpms			= drm_atomic_helper_connector_dpms,
+	.detect			= dumb_vga_connector_detect,
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= dumb_vga_connector_destroy,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+static int dumb_vga_attach(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Missing encoder\n");
+		return -ENODEV;
+	}
+
+	drm_connector_helper_add(&vga->connector,
+				 &dumb_vga_con_helper_funcs);
+	ret = drm_connector_init(bridge->dev, &vga->connector,
+				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&vga->connector,
+					  bridge->encoder);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
+	.attach		= dumb_vga_attach,
+};
+
+static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
+{
+	struct device_node *end_node, *phandle, *remote;
+	struct i2c_adapter *ddc;
+
+	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (!end_node) {
+		dev_err(dev, "Missing connector endpoint\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	remote = of_graph_get_remote_port_parent(end_node);
+	of_node_put(end_node);
+	if (!remote) {
+		dev_err(dev, "Enable to parse remote node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
+	of_node_put(remote);
+	if (!phandle)
+		return ERR_PTR(-ENODEV);
+
+	ddc = of_get_i2c_adapter_by_node(phandle);
+	of_node_put(phandle);
+	if (!ddc)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return ddc;
+}
+
+static int dumb_vga_probe(struct platform_device *pdev)
+{
+	struct dumb_vga *vga;
+	int ret;
+
+	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
+	if (!vga)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, vga);
+
+	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
+	if (IS_ERR(vga->ddc)) {
+		if (PTR_ERR(vga->ddc) == -ENODEV) {
+			dev_info(&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);
+		}
+	}
+
+	vga->bridge.funcs = &dumb_vga_bridge_funcs;
+	vga->bridge.of_node = pdev->dev.of_node;
+
+	ret = drm_bridge_add(&vga->bridge);
+	if (ret && !IS_ERR(vga->ddc))
+		i2c_put_adapter(vga->ddc);
+
+	return ret;
+}
+
+static int dumb_vga_remove(struct platform_device *pdev)
+{
+	struct dumb_vga *vga = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&vga->bridge);
+
+	if (!IS_ERR(vga->ddc))
+		i2c_put_adapter(vga->ddc);
+
+	return 0;
+}
+
+static const struct of_device_id dumb_vga_match[] = {
+	{ .compatible = "rgb-to-vga-bridge" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dumb_vga_match);
+
+struct platform_driver dumb_vga_driver = {
+	.probe	= dumb_vga_probe,
+	.remove	= dumb_vga_remove,
+	.driver		= {
+		.name		= "rgb-to-vga-bridge",
+		.of_match_table	= dumb_vga_match,
+	},
+};
+module_platform_driver(dumb_vga_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
+MODULE_LICENSE("GPL");
-- 
2.9.3

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

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

Some boards have an entirely passive RGB to VGA bridge, based on either
DACs or resistor ladders.

Those might or might not have an i2c bus routed to the VGA connector in
order to access the screen EDIDs.

Add a bridge that doesn't do anything but expose the modes available on the
screen, either based on the EDIDs if available, or based on the XGA
standards.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
new file mode 100644
index 000000000000..a8375bc1f9cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
@@ -0,0 +1,48 @@
+Dumb RGB to VGA bridge
+----------------------
+
+This binding is aimed for dumb RGB to VGA bridges that do not require
+any configuration.
+
+Required properties:
+
+- compatible: Must be "rgb-to-vga-bridge"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+
+Example
+-------
+
+bridge {
+	compatible = "rgb-to-vga-bridge";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port at 0 {
+			reg = <0>;
+
+			vga_bridge_in: endpoint {
+				remote-endpoint = <&tcon0_out_vga>;
+			};
+		};
+
+		port at 1 {
+			reg = <1>;
+
+			vga_bridge_out: endpoint {
+				remote-endpoint = <&vga_con_in>;
+			};
+		};
+	};
+};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b590e678052d..d690398c541c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
 	  the HDMI output of an application processor to MyDP
 	  or DisplayPort.
 
+config DRM_RGB_TO_VGA
+	tristate "Dumb RGB to VGA Bridge support"
+	depends on OF
+	select DRM_KMS_HELPER
+	help
+	  Support for passive RGB to VGA bridges
+
 config DRM_DW_HDMI
 	tristate
 	select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index efdb07e878f5..3bb8cbe09fe9 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
 ccflags-y := -Iinclude/drm
 
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
+obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c b/drivers/gpu/drm/bridge/rgb-to-vga.c
new file mode 100644
index 000000000000..5ff4d4f3598f
--- /dev/null
+++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2015-2016 Free Electrons
+ * Copyright (C) 2015-2016 NextThing Co
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/of_graph.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+struct dumb_vga {
+	struct drm_bridge	bridge;
+	struct drm_connector	connector;
+
+	struct i2c_adapter	*ddc;
+};
+
+static inline struct dumb_vga *
+drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct dumb_vga, bridge);
+}
+
+static inline struct dumb_vga *
+drm_connector_to_dumb_vga(struct drm_connector *connector)
+{
+	return container_of(connector, struct dumb_vga, connector);
+}
+
+static int dumb_vga_get_modes(struct drm_connector *connector)
+{
+	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
+	struct edid *edid;
+	int ret;
+
+	if (IS_ERR(vga->ddc))
+		goto fallback;
+
+	edid = drm_get_edid(connector, vga->ddc);
+	if (!edid) {
+		DRM_INFO("EDID readout failed, falling back to standard modes\n");
+		goto fallback;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	return drm_add_edid_modes(connector, edid);
+
+fallback:
+	/*
+	 * In case we cannot retrieve the EDIDs (broken or missing i2c
+	 * bus), fallback on the XGA standards
+	 */
+	ret = drm_add_modes_noedid(connector, 1920, 1200);
+
+	/* And prefer a mode pretty much anyone can handle */
+	drm_set_preferred_mode(connector, 1024, 768);
+
+	return ret;
+}
+
+static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs = {
+	.get_modes	= dumb_vga_get_modes,
+};
+
+static enum drm_connector_status
+dumb_vga_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
+
+	/*
+	 * Even if we have an I2C bus, we can't assume that the cable
+	 * is disconnected if drm_probe_ddc fails. Some cables don't
+	 * wire the DDC pins, or the I2C bus might not be working at
+	 * all.
+	 */
+	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
+		return connector_status_connected;
+
+	return connector_status_unknown;
+}
+
+static void
+dumb_vga_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs dumb_vga_con_funcs = {
+	.dpms			= drm_atomic_helper_connector_dpms,
+	.detect			= dumb_vga_connector_detect,
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= dumb_vga_connector_destroy,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+static int dumb_vga_attach(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+	int ret;
+
+	if (!bridge->encoder) {
+		DRM_ERROR("Missing encoder\n");
+		return -ENODEV;
+	}
+
+	drm_connector_helper_add(&vga->connector,
+				 &dumb_vga_con_helper_funcs);
+	ret = drm_connector_init(bridge->dev, &vga->connector,
+				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&vga->connector,
+					  bridge->encoder);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
+	.attach		= dumb_vga_attach,
+};
+
+static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
+{
+	struct device_node *end_node, *phandle, *remote;
+	struct i2c_adapter *ddc;
+
+	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (!end_node) {
+		dev_err(dev, "Missing connector endpoint\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	remote = of_graph_get_remote_port_parent(end_node);
+	of_node_put(end_node);
+	if (!remote) {
+		dev_err(dev, "Enable to parse remote node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
+	of_node_put(remote);
+	if (!phandle)
+		return ERR_PTR(-ENODEV);
+
+	ddc = of_get_i2c_adapter_by_node(phandle);
+	of_node_put(phandle);
+	if (!ddc)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return ddc;
+}
+
+static int dumb_vga_probe(struct platform_device *pdev)
+{
+	struct dumb_vga *vga;
+	int ret;
+
+	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
+	if (!vga)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, vga);
+
+	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
+	if (IS_ERR(vga->ddc)) {
+		if (PTR_ERR(vga->ddc) == -ENODEV) {
+			dev_info(&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);
+		}
+	}
+
+	vga->bridge.funcs = &dumb_vga_bridge_funcs;
+	vga->bridge.of_node = pdev->dev.of_node;
+
+	ret = drm_bridge_add(&vga->bridge);
+	if (ret && !IS_ERR(vga->ddc))
+		i2c_put_adapter(vga->ddc);
+
+	return ret;
+}
+
+static int dumb_vga_remove(struct platform_device *pdev)
+{
+	struct dumb_vga *vga = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&vga->bridge);
+
+	if (!IS_ERR(vga->ddc))
+		i2c_put_adapter(vga->ddc);
+
+	return 0;
+}
+
+static const struct of_device_id dumb_vga_match[] = {
+	{ .compatible = "rgb-to-vga-bridge" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dumb_vga_match);
+
+struct platform_driver dumb_vga_driver = {
+	.probe	= dumb_vga_probe,
+	.remove	= dumb_vga_remove,
+	.driver		= {
+		.name		= "rgb-to-vga-bridge",
+		.of_match_table	= dumb_vga_match,
+	},
+};
+module_platform_driver(dumb_vga_driver);
+
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
+MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
+MODULE_LICENSE("GPL");
-- 
2.9.3

[PATCH v5 3/5] ARM: sun5i: a13-olinuxino: Enable VGA bridge

[Maxime Ripard]

Now that we have support for the VGA bridges using our DRM driver, enable
the display engine for the Olimex A13-Olinuxino.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 54 +++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index b3c234c65ea1..01ce7ea9032d 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -72,6 +72,47 @@
 			default-state = "on";
 		};
 	};
+
+	bridge {
+		compatible = "rgb-to-vga-bridge";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				vga_bridge_in: endpoint {
+					remote-endpoint = <&tcon0_out_vga>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				vga_bridge_out: endpoint {
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
+};
+
+&be0 {
+	status = "okay";
 };
 
 &ehci0 {
@@ -211,6 +252,19 @@
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_rgb666_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_vga: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&vga_bridge_in>;
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart1_pins_b>;
-- 
2.9.3

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

[PATCH v5 3/5] ARM: sun5i: a13-olinuxino: Enable VGA bridge

[Maxime Ripard]

Now that we have support for the VGA bridges using our DRM driver, enable
the display engine for the Olimex A13-Olinuxino.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 54 +++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index b3c234c65ea1..01ce7ea9032d 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -72,6 +72,47 @@
 			default-state = "on";
 		};
 	};
+
+	bridge {
+		compatible = "rgb-to-vga-bridge";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				reg = <0>;
+
+				vga_bridge_in: endpoint {
+					remote-endpoint = <&tcon0_out_vga>;
+				};
+			};
+
+			port at 1 {
+				reg = <1>;
+
+				vga_bridge_out: endpoint {
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	vga {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_bridge_out>;
+			};
+		};
+	};
+};
+
+&be0 {
+	status = "okay";
 };
 
 &ehci0 {
@@ -211,6 +252,19 @@
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_rgb666_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_vga: endpoint at 0 {
+		reg = <0>;
+		remote-endpoint = <&vga_bridge_in>;
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart1_pins_b>;
-- 
2.9.3

[PATCH v5 4/5] ARM: multi_v7: enable VGA bridge

[Maxime Ripard]

Enable the RGB to VGA bridge driver in the defconfig

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 2c8665cd9dc5..22ef41afc658 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -567,6 +567,7 @@ CONFIG_DRM=y
 CONFIG_DRM_I2C_ADV7511=m
 # CONFIG_DRM_I2C_CH7006 is not set
 # CONFIG_DRM_I2C_SIL164 is not set
+CONFIG_DRM_RGB_TO_VGA=m
 CONFIG_DRM_NXP_PTN3460=m
 CONFIG_DRM_PARADE_PS8622=m
 CONFIG_DRM_NOUVEAU=m
-- 
2.9.3

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

[PATCH v5 4/5] ARM: multi_v7: enable VGA bridge

[Maxime Ripard]

Enable the RGB to VGA bridge driver in the defconfig

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 2c8665cd9dc5..22ef41afc658 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -567,6 +567,7 @@ CONFIG_DRM=y
 CONFIG_DRM_I2C_ADV7511=m
 # CONFIG_DRM_I2C_CH7006 is not set
 # CONFIG_DRM_I2C_SIL164 is not set
+CONFIG_DRM_RGB_TO_VGA=m
 CONFIG_DRM_NXP_PTN3460=m
 CONFIG_DRM_PARADE_PS8622=m
 CONFIG_DRM_NOUVEAU=m
-- 
2.9.3

[PATCH v5 5/5] ARM: sunxi: Enable VGA bridge

[Maxime Ripard]

Enable the VGA bridge used on the A13-Olinuxino in the sunxi defconfig

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 714da336ec86..d830e258db59 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -98,6 +98,7 @@ CONFIG_MEDIA_RC_SUPPORT=y
 CONFIG_RC_DEVICES=y
 CONFIG_IR_SUNXI=y
 CONFIG_DRM=y
+CONFIG_DRM_RGB_TO_VGA=y
 CONFIG_DRM_SUN4I=y
 CONFIG_FB=y
 CONFIG_FB_SIMPLE=y
-- 
2.9.3

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

[PATCH v5 5/5] ARM: sunxi: Enable VGA bridge

[Maxime Ripard]

Enable the VGA bridge used on the A13-Olinuxino in the sunxi defconfig

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 714da336ec86..d830e258db59 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -98,6 +98,7 @@ CONFIG_MEDIA_RC_SUPPORT=y
 CONFIG_RC_DEVICES=y
 CONFIG_IR_SUNXI=y
 CONFIG_DRM=y
+CONFIG_DRM_RGB_TO_VGA=y
 CONFIG_DRM_SUN4I=y
 CONFIG_FB=y
 CONFIG_FB_SIMPLE=y
-- 
2.9.3

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

Hi Maxime,

On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> Some boards have an entirely passive RGB to VGA bridge, based on either
> DACs or resistor ladders.
>
> Those might or might not have an i2c bus routed to the VGA connector in
> order to access the screen EDIDs.
>
> Add a bridge that doesn't do anything but expose the modes available on the
> screen, either based on the EDIDs if available, or based on the XGA
> standards.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
>  4 files changed, 285 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>  create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> new file mode 100644
> index 000000000000..a8375bc1f9cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> @@ -0,0 +1,48 @@
> +Dumb RGB to VGA bridge
> +----------------------
> +
> +This binding is aimed for dumb RGB to VGA bridges that do not require
> +any configuration.
> +
> +Required properties:
> +
> +- compatible: Must be "rgb-to-vga-bridge"

I'd talked to Laurent on IRC if he's okay with this. And I guess you to
had discussed it during XDC too. He's suggested that it'd be better to
have the compatible string as "simple-vga-dac".

Some of the reasons behind having this:

- We don't need to specify "rgb" in the compatible string since most
simple VGA DACs can only work with an RGB input.
- Also, with "dac" specified in the string, we don't need to
specifically mention "bridge" in the string. Also, bridge is a drm
specific term.
- "simple" is considered because it's an unconfigurable bridge, and it
might be misleading for other VGA DACs to not use "vga-dac".

What do you think about this? If you think it's good, would it be
possible for you to change this? I guess it's okay for the rest of
the patch to stay the same.

Sorry about the churn.

Thanks,
Archit

> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +
> +Example
> +-------
> +
> +bridge {
> +	compatible = "rgb-to-vga-bridge";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			vga_bridge_in: endpoint {
> +				remote-endpoint = <&tcon0_out_vga>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			vga_bridge_out: endpoint {
> +				remote-endpoint = <&vga_con_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e678052d..d690398c541c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
>  	  the HDMI output of an application processor to MyDP
>  	  or DisplayPort.
>
> +config DRM_RGB_TO_VGA
> +	tristate "Dumb RGB to VGA Bridge support"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for passive RGB to VGA bridges
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index efdb07e878f5..3bb8cbe09fe9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
>  ccflags-y := -Iinclude/drm
>
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> +obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c b/drivers/gpu/drm/bridge/rgb-to-vga.c
> new file mode 100644
> index 000000000000..5ff4d4f3598f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
> @@ -0,0 +1,229 @@
> +/*
> + * Copyright (C) 2015-2016 Free Electrons
> + * Copyright (C) 2015-2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct dumb_vga {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +};
> +
> +static inline struct dumb_vga *
> +drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct dumb_vga, bridge);
> +}
> +
> +static inline struct dumb_vga *
> +drm_connector_to_dumb_vga(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct dumb_vga, connector);
> +}
> +
> +static int dumb_vga_get_modes(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (IS_ERR(vga->ddc))
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, vga->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID readout failed, falling back to standard modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	return drm_add_edid_modes(connector, edid);
> +
> +fallback:
> +	/*
> +	 * In case we cannot retrieve the EDIDs (broken or missing i2c
> +	 * bus), fallback on the XGA standards
> +	 */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anyone can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs = {
> +	.get_modes	= dumb_vga_get_modes,
> +};
> +
> +static enum drm_connector_status
> +dumb_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	/*
> +	 * Even if we have an I2C bus, we can't assume that the cable
> +	 * is disconnected if drm_probe_ddc fails. Some cables don't
> +	 * wire the DDC pins, or the I2C bus might not be working at
> +	 * all.
> +	 */
> +	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
> +		return connector_status_connected;
> +
> +	return connector_status_unknown;
> +}
> +
> +static void
> +dumb_vga_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs dumb_vga_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= dumb_vga_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= dumb_vga_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int dumb_vga_attach(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&vga->connector,
> +				 &dumb_vga_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &vga->connector,
> +				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&vga->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> +	.attach		= dumb_vga_attach,
> +};
> +
> +static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> +{
> +	struct device_node *end_node, *phandle, *remote;
> +	struct i2c_adapter *ddc;
> +
> +	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!end_node) {
> +		dev_err(dev, "Missing connector endpoint\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(end_node);
> +	of_node_put(end_node);
> +	if (!remote) {
> +		dev_err(dev, "Enable to parse remote node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +	of_node_put(remote);
> +	if (!phandle)
> +		return ERR_PTR(-ENODEV);
> +
> +	ddc = of_get_i2c_adapter_by_node(phandle);
> +	of_node_put(phandle);
> +	if (!ddc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return ddc;
> +}
> +
> +static int dumb_vga_probe(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga;
> +	int ret;
> +
> +	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, vga);
> +
> +	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> +	if (IS_ERR(vga->ddc)) {
> +		if (PTR_ERR(vga->ddc) == -ENODEV) {
> +			dev_info(&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);
> +		}
> +	}
> +
> +	vga->bridge.funcs = &dumb_vga_bridge_funcs;
> +	vga->bridge.of_node = pdev->dev.of_node;
> +
> +	ret = drm_bridge_add(&vga->bridge);
> +	if (ret && !IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return ret;
> +}
> +
> +static int dumb_vga_remove(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&vga->bridge);
> +
> +	if (!IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dumb_vga_match[] = {
> +	{ .compatible = "rgb-to-vga-bridge" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dumb_vga_match);
> +
> +struct platform_driver dumb_vga_driver = {
> +	.probe	= dumb_vga_probe,
> +	.remove	= dumb_vga_remove,
> +	.driver		= {
> +		.name		= "rgb-to-vga-bridge",
> +		.of_match_table	= dumb_vga_match,
> +	},
> +};
> +module_platform_driver(dumb_vga_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
> +MODULE_LICENSE("GPL");
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

Hi Maxime,

On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> Some boards have an entirely passive RGB to VGA bridge, based on either
> DACs or resistor ladders.
>
> Those might or might not have an i2c bus routed to the VGA connector in
> order to access the screen EDIDs.
>
> Add a bridge that doesn't do anything but expose the modes available on the
> screen, either based on the EDIDs if available, or based on the XGA
> standards.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
>  4 files changed, 285 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>  create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> new file mode 100644
> index 000000000000..a8375bc1f9cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> @@ -0,0 +1,48 @@
> +Dumb RGB to VGA bridge
> +----------------------
> +
> +This binding is aimed for dumb RGB to VGA bridges that do not require
> +any configuration.
> +
> +Required properties:
> +
> +- compatible: Must be "rgb-to-vga-bridge"

I'd talked to Laurent on IRC if he's okay with this. And I guess you to
had discussed it during XDC too. He's suggested that it'd be better to
have the compatible string as "simple-vga-dac".

Some of the reasons behind having this:

- We don't need to specify "rgb" in the compatible string since most
simple VGA DACs can only work with an RGB input.
- Also, with "dac" specified in the string, we don't need to
specifically mention "bridge" in the string. Also, bridge is a drm
specific term.
- "simple" is considered because it's an unconfigurable bridge, and it
might be misleading for other VGA DACs to not use "vga-dac".

What do you think about this? If you think it's good, would it be
possible for you to change this? I guess it's okay for the rest of
the patch to stay the same.

Sorry about the churn.

Thanks,
Archit

> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +
> +Example
> +-------
> +
> +bridge {
> +	compatible = "rgb-to-vga-bridge";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port at 0 {
> +			reg = <0>;
> +
> +			vga_bridge_in: endpoint {
> +				remote-endpoint = <&tcon0_out_vga>;
> +			};
> +		};
> +
> +		port at 1 {
> +			reg = <1>;
> +
> +			vga_bridge_out: endpoint {
> +				remote-endpoint = <&vga_con_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e678052d..d690398c541c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
>  	  the HDMI output of an application processor to MyDP
>  	  or DisplayPort.
>
> +config DRM_RGB_TO_VGA
> +	tristate "Dumb RGB to VGA Bridge support"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for passive RGB to VGA bridges
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index efdb07e878f5..3bb8cbe09fe9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
>  ccflags-y := -Iinclude/drm
>
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> +obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c b/drivers/gpu/drm/bridge/rgb-to-vga.c
> new file mode 100644
> index 000000000000..5ff4d4f3598f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
> @@ -0,0 +1,229 @@
> +/*
> + * Copyright (C) 2015-2016 Free Electrons
> + * Copyright (C) 2015-2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct dumb_vga {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +};
> +
> +static inline struct dumb_vga *
> +drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct dumb_vga, bridge);
> +}
> +
> +static inline struct dumb_vga *
> +drm_connector_to_dumb_vga(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct dumb_vga, connector);
> +}
> +
> +static int dumb_vga_get_modes(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (IS_ERR(vga->ddc))
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, vga->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID readout failed, falling back to standard modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	return drm_add_edid_modes(connector, edid);
> +
> +fallback:
> +	/*
> +	 * In case we cannot retrieve the EDIDs (broken or missing i2c
> +	 * bus), fallback on the XGA standards
> +	 */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anyone can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs = {
> +	.get_modes	= dumb_vga_get_modes,
> +};
> +
> +static enum drm_connector_status
> +dumb_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	/*
> +	 * Even if we have an I2C bus, we can't assume that the cable
> +	 * is disconnected if drm_probe_ddc fails. Some cables don't
> +	 * wire the DDC pins, or the I2C bus might not be working at
> +	 * all.
> +	 */
> +	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
> +		return connector_status_connected;
> +
> +	return connector_status_unknown;
> +}
> +
> +static void
> +dumb_vga_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs dumb_vga_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= dumb_vga_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= dumb_vga_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int dumb_vga_attach(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&vga->connector,
> +				 &dumb_vga_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &vga->connector,
> +				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&vga->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> +	.attach		= dumb_vga_attach,
> +};
> +
> +static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> +{
> +	struct device_node *end_node, *phandle, *remote;
> +	struct i2c_adapter *ddc;
> +
> +	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!end_node) {
> +		dev_err(dev, "Missing connector endpoint\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(end_node);
> +	of_node_put(end_node);
> +	if (!remote) {
> +		dev_err(dev, "Enable to parse remote node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +	of_node_put(remote);
> +	if (!phandle)
> +		return ERR_PTR(-ENODEV);
> +
> +	ddc = of_get_i2c_adapter_by_node(phandle);
> +	of_node_put(phandle);
> +	if (!ddc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return ddc;
> +}
> +
> +static int dumb_vga_probe(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga;
> +	int ret;
> +
> +	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, vga);
> +
> +	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> +	if (IS_ERR(vga->ddc)) {
> +		if (PTR_ERR(vga->ddc) == -ENODEV) {
> +			dev_info(&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);
> +		}
> +	}
> +
> +	vga->bridge.funcs = &dumb_vga_bridge_funcs;
> +	vga->bridge.of_node = pdev->dev.of_node;
> +
> +	ret = drm_bridge_add(&vga->bridge);
> +	if (ret && !IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return ret;
> +}
> +
> +static int dumb_vga_remove(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&vga->bridge);
> +
> +	if (!IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dumb_vga_match[] = {
> +	{ .compatible = "rgb-to-vga-bridge" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dumb_vga_match);
> +
> +struct platform_driver dumb_vga_driver = {
> +	.probe	= dumb_vga_probe,
> +	.remove	= dumb_vga_remove,
> +	.driver		= {
> +		.name		= "rgb-to-vga-bridge",
> +		.of_match_table	= dumb_vga_match,
> +	},
> +};
> +module_platform_driver(dumb_vga_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
> +MODULE_LICENSE("GPL");
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

Hi Archit,

On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> Hi Maxime,
> 
> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >Some boards have an entirely passive RGB to VGA bridge, based on either
> >DACs or resistor ladders.
> >
> >Those might or might not have an i2c bus routed to the VGA connector in
> >order to access the screen EDIDs.
> >
> >Add a bridge that doesn't do anything but expose the modes available on the
> >screen, either based on the EDIDs if available, or based on the XGA
> >standards.
> >
> >Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >---
> > .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> > drivers/gpu/drm/bridge/Kconfig                     |   7 +
> > drivers/gpu/drm/bridge/Makefile                    |   1 +
> > drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
> > 4 files changed, 285 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >
> >diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >new file mode 100644
> >index 000000000000..a8375bc1f9cb
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >@@ -0,0 +1,48 @@
> >+Dumb RGB to VGA bridge
> >+----------------------
> >+
> >+This binding is aimed for dumb RGB to VGA bridges that do not require
> >+any configuration.
> >+
> >+Required properties:
> >+
> >+- compatible: Must be "rgb-to-vga-bridge"
> 
> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
> had discussed it during XDC too. He's suggested that it'd be better to
> have the compatible string as "simple-vga-dac".

I just wished this bikeshedding had taken place publicly and be
actually part of that discussion, but yeah, ok.

> Some of the reasons behind having this:
> 
> - We don't need to specify "rgb" in the compatible string since most
> simple VGA DACs can only work with an RGB input.

Ok.

> - Also, with "dac" specified in the string, we don't need to
> specifically mention "bridge" in the string. Also, bridge is a drm
> specific term.
>
> - "simple" is considered because it's an unconfigurable bridge, and it
> might be misleading for other VGA DACs to not use "vga-dac".

All those "simple" bindings are just the biggest lie we ever
told. It's simple when you introduce it, and then grows into something
much more complicated than a non-simple implementation.

> What do you think about this? If you think it's good, would it be
> possible for you to change this? I guess it's okay for the rest of
> the patch to stay the same.

I'll update and respin the serie.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

Hi Archit,

On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> Hi Maxime,
> 
> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >Some boards have an entirely passive RGB to VGA bridge, based on either
> >DACs or resistor ladders.
> >
> >Those might or might not have an i2c bus routed to the VGA connector in
> >order to access the screen EDIDs.
> >
> >Add a bridge that doesn't do anything but expose the modes available on the
> >screen, either based on the EDIDs if available, or based on the XGA
> >standards.
> >
> >Acked-by: Rob Herring <robh@kernel.org>
> >Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >---
> > .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> > drivers/gpu/drm/bridge/Kconfig                     |   7 +
> > drivers/gpu/drm/bridge/Makefile                    |   1 +
> > drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
> > 4 files changed, 285 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >
> >diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >new file mode 100644
> >index 000000000000..a8375bc1f9cb
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >@@ -0,0 +1,48 @@
> >+Dumb RGB to VGA bridge
> >+----------------------
> >+
> >+This binding is aimed for dumb RGB to VGA bridges that do not require
> >+any configuration.
> >+
> >+Required properties:
> >+
> >+- compatible: Must be "rgb-to-vga-bridge"
> 
> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
> had discussed it during XDC too. He's suggested that it'd be better to
> have the compatible string as "simple-vga-dac".

I just wished this bikeshedding had taken place publicly and be
actually part of that discussion, but yeah, ok.

> Some of the reasons behind having this:
> 
> - We don't need to specify "rgb" in the compatible string since most
> simple VGA DACs can only work with an RGB input.

Ok.

> - Also, with "dac" specified in the string, we don't need to
> specifically mention "bridge" in the string. Also, bridge is a drm
> specific term.
>
> - "simple" is considered because it's an unconfigurable bridge, and it
> might be misleading for other VGA DACs to not use "vga-dac".

All those "simple" bindings are just the biggest lie we ever
told. It's simple when you introduce it, and then grows into something
much more complicated than a non-simple implementation.

> What do you think about this? If you think it's good, would it be
> possible for you to change this? I guess it's okay for the rest of
> the patch to stay the same.

I'll update and respin the serie.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161006/701c158a/attachment.sig>

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> Hi Archit,
>
> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>> Hi Maxime,
>>
>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>> Some boards have an entirely passive RGB to VGA bridge, based on either
>>> DACs or resistor ladders.
>>>
>>> Those might or might not have an i2c bus routed to the VGA connector in
>>> order to access the screen EDIDs.
>>>
>>> Add a bridge that doesn't do anything but expose the modes available on the
>>> screen, either based on the EDIDs if available, or based on the XGA
>>> standards.
>>>
>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
>>> 4 files changed, 285 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> new file mode 100644
>>> index 000000000000..a8375bc1f9cb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> @@ -0,0 +1,48 @@
>>> +Dumb RGB to VGA bridge
>>> +----------------------
>>> +
>>> +This binding is aimed for dumb RGB to VGA bridges that do not require
>>> +any configuration.
>>> +
>>> +Required properties:
>>> +
>>> +- compatible: Must be "rgb-to-vga-bridge"
>>
>> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
>> had discussed it during XDC too. He's suggested that it'd be better to
>> have the compatible string as "simple-vga-dac".
>
> I just wished this bikeshedding had taken place publicly and be
> actually part of that discussion, but yeah, ok.

Sorry about that. I'd pinged him for an Ack, the discussion went
more than that :)

>
>> Some of the reasons behind having this:
>>
>> - We don't need to specify "rgb" in the compatible string since most
>> simple VGA DACs can only work with an RGB input.
>
> Ok.
>
>> - Also, with "dac" specified in the string, we don't need to
>> specifically mention "bridge" in the string. Also, bridge is a drm
>> specific term.
>>
>> - "simple" is considered because it's an unconfigurable bridge, and it
>> might be misleading for other VGA DACs to not use "vga-dac".
>
> All those "simple" bindings are just the biggest lie we ever
> told. It's simple when you introduce it, and then grows into something
> much more complicated than a non-simple implementation.

"simple" here is supposed to mean that it's an unconfigurable RGB to
VGA DAC. This isn't supposed to follow the simple-panel model, where
you add the "simple-panel" string in the compatible node, along with
you chip specific compatible string.

In other words, this driver shouldn't be touched again in the future :)
If someone wants to write a RGB to VGA driver which is even
slightly configurable, they'll need to write a new bridge driver.

Thanks,
Archit

>
>> What do you think about this? If you think it's good, would it be
>> possible for you to change this? I guess it's okay for the rest of
>> the patch to stay the same.
>
> I'll update and respin the serie.
>
> Thanks,
> Maxime
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> Hi Archit,
>
> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>> Hi Maxime,
>>
>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>> Some boards have an entirely passive RGB to VGA bridge, based on either
>>> DACs or resistor ladders.
>>>
>>> Those might or might not have an i2c bus routed to the VGA connector in
>>> order to access the screen EDIDs.
>>>
>>> Add a bridge that doesn't do anything but expose the modes available on the
>>> screen, either based on the EDIDs if available, or based on the XGA
>>> standards.
>>>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++++++++
>>> 4 files changed, 285 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> new file mode 100644
>>> index 000000000000..a8375bc1f9cb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>>> @@ -0,0 +1,48 @@
>>> +Dumb RGB to VGA bridge
>>> +----------------------
>>> +
>>> +This binding is aimed for dumb RGB to VGA bridges that do not require
>>> +any configuration.
>>> +
>>> +Required properties:
>>> +
>>> +- compatible: Must be "rgb-to-vga-bridge"
>>
>> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
>> had discussed it during XDC too. He's suggested that it'd be better to
>> have the compatible string as "simple-vga-dac".
>
> I just wished this bikeshedding had taken place publicly and be
> actually part of that discussion, but yeah, ok.

Sorry about that. I'd pinged him for an Ack, the discussion went
more than that :)

>
>> Some of the reasons behind having this:
>>
>> - We don't need to specify "rgb" in the compatible string since most
>> simple VGA DACs can only work with an RGB input.
>
> Ok.
>
>> - Also, with "dac" specified in the string, we don't need to
>> specifically mention "bridge" in the string. Also, bridge is a drm
>> specific term.
>>
>> - "simple" is considered because it's an unconfigurable bridge, and it
>> might be misleading for other VGA DACs to not use "vga-dac".
>
> All those "simple" bindings are just the biggest lie we ever
> told. It's simple when you introduce it, and then grows into something
> much more complicated than a non-simple implementation.

"simple" here is supposed to mean that it's an unconfigurable RGB to
VGA DAC. This isn't supposed to follow the simple-panel model, where
you add the "simple-panel" string in the compatible node, along with
you chip specific compatible string.

In other words, this driver shouldn't be touched again in the future :)
If someone wants to write a RGB to VGA driver which is even
slightly configurable, they'll need to write a new bridge driver.

Thanks,
Archit

>
>> What do you think about this? If you think it's good, would it be
>> possible for you to change this? I guess it's okay for the rest of
>> the patch to stay the same.
>
> I'll update and respin the serie.
>
> Thanks,
> Maxime
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hello,

On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>> Some boards have an entirely passive RGB to VGA bridge, based on either
> >>> DACs or resistor ladders.
> >>> 
> >>> Those might or might not have an i2c bus routed to the VGA connector in
> >>> order to access the screen EDIDs.
> >>> 
> >>> Add a bridge that doesn't do anything but expose the modes available on
> >>> the screen, either based on the EDIDs if available, or based on the XGA
> >>> standards.
> >>> 
> >>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >>> ---
> >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++
> >>> 4 files changed, 285 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t
> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t new file mode 100644
> >>> index 000000000000..a8375bc1f9cb
> >>> --- /dev/null
> >>> +++
> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t @@ -0,0 +1,48 @@
> >>> +Dumb RGB to VGA bridge
> >>> +----------------------
> >>> +
> >>> +This binding is aimed for dumb RGB to VGA bridges that do not require
> >>> +any configuration.
> >>> +
> >>> +Required properties:
> >>> +
> >>> +- compatible: Must be "rgb-to-vga-bridge"
> >> 
> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
> >> had discussed it during XDC too. He's suggested that it'd be better to
> >> have the compatible string as "simple-vga-dac".
> > 
> > I just wished this bikeshedding had taken place publicly and be
> > actually part of that discussion, but yeah, ok.
> 
> Sorry about that. I'd pinged him for an Ack, the discussion went
> more than that :)
> 
> >> Some of the reasons behind having this:
> >> 
> >> - We don't need to specify "rgb" in the compatible string since most
> >> simple VGA DACs can only work with an RGB input.
> > 
> > Ok.
> > 
> >> - Also, with "dac" specified in the string, we don't need to
> >> specifically mention "bridge" in the string. Also, bridge is a drm
> >> specific term.
> >> 
> >> - "simple" is considered because it's an unconfigurable bridge, and it
> >> might be misleading for other VGA DACs to not use "vga-dac".
> > 
> > All those "simple" bindings are just the biggest lie we ever
> > told. It's simple when you introduce it, and then grows into something
> > much more complicated than a non-simple implementation.
> 
> "simple" here is supposed to mean that it's an unconfigurable RGB to
> VGA DAC. This isn't supposed to follow the simple-panel model, where
> you add the "simple-panel" string in the compatible node, along with
> you chip specific compatible string.

I agree with Maxime, I don't like the word "simple". My preference would be 
"vga-dac" for a lack of a better qualifier than "simple" to describe the fact 
that the device requires no configuration. My only concern with "vga-dac" is 
that we would restrict usage of that compatible string for a subset of VGA 
DACs, with more complex devices not being compatible with "vga-dac" even 
though they are VGA DACs. That's a problem I can live with though.

> In other words, this driver shouldn't be touched again in the future :)
> If someone wants to write a RGB to VGA driver which is even
> slightly configurable, they'll need to write a new bridge driver.

I'm sure that won't be true. I can certainly foresee the addition of 
regulators support for instance. It's unfortunately never black and white.

> >> What do you think about this? If you think it's good, would it be
> >> possible for you to change this? I guess it's okay for the rest of
> >> the patch to stay the same.
> > 
> > I'll update and respin the serie.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hello,

On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>> Some boards have an entirely passive RGB to VGA bridge, based on either
> >>> DACs or resistor ladders.
> >>> 
> >>> Those might or might not have an i2c bus routed to the VGA connector in
> >>> order to access the screen EDIDs.
> >>> 
> >>> Add a bridge that doesn't do anything but expose the modes available on
> >>> the screen, either based on the EDIDs if available, or based on the XGA
> >>> standards.
> >>> 
> >>> Acked-by: Rob Herring <robh@kernel.org>
> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>> ---
> >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++
> >>> 4 files changed, 285 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t
> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t new file mode 100644
> >>> index 000000000000..a8375bc1f9cb
> >>> --- /dev/null
> >>> +++
> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>> t @@ -0,0 +1,48 @@
> >>> +Dumb RGB to VGA bridge
> >>> +----------------------
> >>> +
> >>> +This binding is aimed for dumb RGB to VGA bridges that do not require
> >>> +any configuration.
> >>> +
> >>> +Required properties:
> >>> +
> >>> +- compatible: Must be "rgb-to-vga-bridge"
> >> 
> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
> >> had discussed it during XDC too. He's suggested that it'd be better to
> >> have the compatible string as "simple-vga-dac".
> > 
> > I just wished this bikeshedding had taken place publicly and be
> > actually part of that discussion, but yeah, ok.
> 
> Sorry about that. I'd pinged him for an Ack, the discussion went
> more than that :)
> 
> >> Some of the reasons behind having this:
> >> 
> >> - We don't need to specify "rgb" in the compatible string since most
> >> simple VGA DACs can only work with an RGB input.
> > 
> > Ok.
> > 
> >> - Also, with "dac" specified in the string, we don't need to
> >> specifically mention "bridge" in the string. Also, bridge is a drm
> >> specific term.
> >> 
> >> - "simple" is considered because it's an unconfigurable bridge, and it
> >> might be misleading for other VGA DACs to not use "vga-dac".
> > 
> > All those "simple" bindings are just the biggest lie we ever
> > told. It's simple when you introduce it, and then grows into something
> > much more complicated than a non-simple implementation.
> 
> "simple" here is supposed to mean that it's an unconfigurable RGB to
> VGA DAC. This isn't supposed to follow the simple-panel model, where
> you add the "simple-panel" string in the compatible node, along with
> you chip specific compatible string.

I agree with Maxime, I don't like the word "simple". My preference would be 
"vga-dac" for a lack of a better qualifier than "simple" to describe the fact 
that the device requires no configuration. My only concern with "vga-dac" is 
that we would restrict usage of that compatible string for a subset of VGA 
DACs, with more complex devices not being compatible with "vga-dac" even 
though they are VGA DACs. That's a problem I can live with though.

> In other words, this driver shouldn't be touched again in the future :)
> If someone wants to write a RGB to VGA driver which is even
> slightly configurable, they'll need to write a new bridge driver.

I'm sure that won't be true. I can certainly foresee the addition of 
regulators support for instance. It's unfortunately never black and white.

> >> What do you think about this? If you think it's good, would it be
> >> possible for you to change this? I guess it's okay for the rest of
> >> the patch to stay the same.
> > 
> > I'll update and respin the serie.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Sean Paul]

On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hello,
>
> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>> > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>> >>> Some boards have an entirely passive RGB to VGA bridge, based on either
>> >>> DACs or resistor ladders.
>> >>>
>> >>> Those might or might not have an i2c bus routed to the VGA connector in
>> >>> order to access the screen EDIDs.
>> >>>
>> >>> Add a bridge that doesn't do anything but expose the modes available on
>> >>> the screen, either based on the EDIDs if available, or based on the XGA
>> >>> standards.
>> >>>
>> >>> Acked-by: Rob Herring <robh@kernel.org>
>> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >>> ---
>> >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>> >>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>> >>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>> >>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++
>> >>> 4 files changed, 285 insertions(+)
>> >>> create mode 100644
>> >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>> >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t
>> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t new file mode 100644
>> >>> index 000000000000..a8375bc1f9cb
>> >>> --- /dev/null
>> >>> +++
>> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t @@ -0,0 +1,48 @@
>> >>> +Dumb RGB to VGA bridge
>> >>> +----------------------
>> >>> +
>> >>> +This binding is aimed for dumb RGB to VGA bridges that do not require
>> >>> +any configuration.
>> >>> +
>> >>> +Required properties:
>> >>> +
>> >>> +- compatible: Must be "rgb-to-vga-bridge"
>> >>
>> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
>> >> had discussed it during XDC too. He's suggested that it'd be better to
>> >> have the compatible string as "simple-vga-dac".
>> >
>> > I just wished this bikeshedding had taken place publicly and be
>> > actually part of that discussion, but yeah, ok.
>>
>> Sorry about that. I'd pinged him for an Ack, the discussion went
>> more than that :)
>>
>> >> Some of the reasons behind having this:
>> >>
>> >> - We don't need to specify "rgb" in the compatible string since most
>> >> simple VGA DACs can only work with an RGB input.
>> >
>> > Ok.
>> >
>> >> - Also, with "dac" specified in the string, we don't need to
>> >> specifically mention "bridge" in the string. Also, bridge is a drm
>> >> specific term.
>> >>
>> >> - "simple" is considered because it's an unconfigurable bridge, and it
>> >> might be misleading for other VGA DACs to not use "vga-dac".
>> >
>> > All those "simple" bindings are just the biggest lie we ever
>> > told. It's simple when you introduce it, and then grows into something
>> > much more complicated than a non-simple implementation.
>>
>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>> you add the "simple-panel" string in the compatible node, along with
>> you chip specific compatible string.
>
> I agree with Maxime, I don't like the word "simple". My preference would be
> "vga-dac" for a lack of a better qualifier than "simple" to describe the fact
> that the device requires no configuration. My only concern with "vga-dac" is
> that we would restrict usage of that compatible string for a subset of VGA
> DACs, with more complex devices not being compatible with "vga-dac" even
> though they are VGA DACs. That's a problem I can live with though.

While we're bikeshedding (feel free to ignore my input on this), I
think Maxime's initial "dumb" qualifier was better than "simple". I
think "passive" also gets the point across better than "simple", which
we've already established as something else in drm.

Now that I've gotten that out of the way, this patch looks good to me
regardless of the name.

Reviewed-by: Sean Paul <seanpaul@chromium.org>

Sean

>
>> In other words, this driver shouldn't be touched again in the future :)
>> If someone wants to write a RGB to VGA driver which is even
>> slightly configurable, they'll need to write a new bridge driver.
>
> I'm sure that won't be true. I can certainly foresee the addition of
> regulators support for instance. It's unfortunately never black and white.
>
>> >> What do you think about this? If you think it's good, would it be
>> >> possible for you to change this? I guess it's okay for the rest of
>> >> the patch to stay the same.
>> >
>> > I'll update and respin the serie.
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Sean Paul]

On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hello,
>
> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>> > On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>> >> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>> >>> Some boards have an entirely passive RGB to VGA bridge, based on either
>> >>> DACs or resistor ladders.
>> >>>
>> >>> Those might or might not have an i2c bus routed to the VGA connector in
>> >>> order to access the screen EDIDs.
>> >>>
>> >>> Add a bridge that doesn't do anything but expose the modes available on
>> >>> the screen, either based on the EDIDs if available, or based on the XGA
>> >>> standards.
>> >>>
>> >>> Acked-by: Rob Herring <robh@kernel.org>
>> >>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >>> ---
>> >>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>> >>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>> >>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>> >>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++++
>> >>> 4 files changed, 285 insertions(+)
>> >>> create mode 100644
>> >>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
>> >>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t
>> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t new file mode 100644
>> >>> index 000000000000..a8375bc1f9cb
>> >>> --- /dev/null
>> >>> +++
>> >>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>> >>> t @@ -0,0 +1,48 @@
>> >>> +Dumb RGB to VGA bridge
>> >>> +----------------------
>> >>> +
>> >>> +This binding is aimed for dumb RGB to VGA bridges that do not require
>> >>> +any configuration.
>> >>> +
>> >>> +Required properties:
>> >>> +
>> >>> +- compatible: Must be "rgb-to-vga-bridge"
>> >>
>> >> I'd talked to Laurent on IRC if he's okay with this. And I guess you to
>> >> had discussed it during XDC too. He's suggested that it'd be better to
>> >> have the compatible string as "simple-vga-dac".
>> >
>> > I just wished this bikeshedding had taken place publicly and be
>> > actually part of that discussion, but yeah, ok.
>>
>> Sorry about that. I'd pinged him for an Ack, the discussion went
>> more than that :)
>>
>> >> Some of the reasons behind having this:
>> >>
>> >> - We don't need to specify "rgb" in the compatible string since most
>> >> simple VGA DACs can only work with an RGB input.
>> >
>> > Ok.
>> >
>> >> - Also, with "dac" specified in the string, we don't need to
>> >> specifically mention "bridge" in the string. Also, bridge is a drm
>> >> specific term.
>> >>
>> >> - "simple" is considered because it's an unconfigurable bridge, and it
>> >> might be misleading for other VGA DACs to not use "vga-dac".
>> >
>> > All those "simple" bindings are just the biggest lie we ever
>> > told. It's simple when you introduce it, and then grows into something
>> > much more complicated than a non-simple implementation.
>>
>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>> you add the "simple-panel" string in the compatible node, along with
>> you chip specific compatible string.
>
> I agree with Maxime, I don't like the word "simple". My preference would be
> "vga-dac" for a lack of a better qualifier than "simple" to describe the fact
> that the device requires no configuration. My only concern with "vga-dac" is
> that we would restrict usage of that compatible string for a subset of VGA
> DACs, with more complex devices not being compatible with "vga-dac" even
> though they are VGA DACs. That's a problem I can live with though.

While we're bikeshedding (feel free to ignore my input on this), I
think Maxime's initial "dumb" qualifier was better than "simple". I
think "passive" also gets the point across better than "simple", which
we've already established as something else in drm.

Now that I've gotten that out of the way, this patch looks good to me
regardless of the name.

Reviewed-by: Sean Paul <seanpaul@chromium.org>

Sean

>
>> In other words, this driver shouldn't be touched again in the future :)
>> If someone wants to write a RGB to VGA driver which is even
>> slightly configurable, they'll need to write a new bridge driver.
>
> I'm sure that won't be true. I can certainly foresee the addition of
> regulators support for instance. It's unfortunately never black and white.
>
>> >> What do you think about this? If you think it's good, would it be
>> >> possible for you to change this? I guess it's okay for the rest of
>> >> the patch to stay the same.
>> >
>> > I'll update and respin the serie.
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Sean,

On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> > On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>> Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>> either DACs or resistor ladders.
> >>>>> 
> >>>>> Those might or might not have an i2c bus routed to the VGA connector
> >>>>> in order to access the screen EDIDs.
> >>>>> 
> >>>>> Add a bridge that doesn't do anything but expose the modes available
> >>>>> on the screen, either based on the EDIDs if available, or based on
> >>>>> the XGA standards.
> >>>>> 
> >>>>> Acked-by: Rob Herring <robh@kernel.org>
> >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>> ---
> >>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
> >>>>> 4 files changed, 285 insertions(+)
> >>>>> create mode 100644
> >>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>>>> t
> >>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> txt
> >>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..a8375bc1f9cb
> >>>>> --- /dev/null
> >>>>> +++
> >>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> tx
> >>>>> t @@ -0,0 +1,48 @@
> >>>>> +Dumb RGB to VGA bridge
> >>>>> +----------------------
> >>>>> +
> >>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>> require
> >>>>> +any configuration.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +
> >>>>> +- compatible: Must be "rgb-to-vga-bridge"
> >>>> 
> >>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>> to had discussed it during XDC too. He's suggested that it'd be better
> >>>> to have the compatible string as "simple-vga-dac".
> >>> 
> >>> I just wished this bikeshedding had taken place publicly and be
> >>> actually part of that discussion, but yeah, ok.
> >> 
> >> Sorry about that. I'd pinged him for an Ack, the discussion went
> >> more than that :)
> >> 
> >>>> Some of the reasons behind having this:
> >>>> 
> >>>> - We don't need to specify "rgb" in the compatible string since most
> >>>> simple VGA DACs can only work with an RGB input.
> >>> 
> >>> Ok.
> >>> 
> >>>> - Also, with "dac" specified in the string, we don't need to
> >>>> specifically mention "bridge" in the string. Also, bridge is a drm
> >>>> specific term.
> >>>> 
> >>>> - "simple" is considered because it's an unconfigurable bridge, and it
> >>>> might be misleading for other VGA DACs to not use "vga-dac".
> >>> 
> >>> All those "simple" bindings are just the biggest lie we ever
> >>> told. It's simple when you introduce it, and then grows into something
> >>> much more complicated than a non-simple implementation.
> >> 
> >> "simple" here is supposed to mean that it's an unconfigurable RGB to
> >> VGA DAC. This isn't supposed to follow the simple-panel model, where
> >> you add the "simple-panel" string in the compatible node, along with
> >> you chip specific compatible string.
> > 
> > I agree with Maxime, I don't like the word "simple". My preference would
> > be "vga-dac" for a lack of a better qualifier than "simple" to describe
> > the fact that the device requires no configuration. My only concern with
> > "vga-dac" is that we would restrict usage of that compatible string for a
> > subset of VGA DACs, with more complex devices not being compatible with
> > "vga-dac" even though they are VGA DACs. That's a problem I can live with
> > though.
>
> While we're bikeshedding (feel free to ignore my input on this), I
> think Maxime's initial "dumb" qualifier was better than "simple".

I think I agree.

> I think "passive" also gets the point across better than "simple", which
> we've already established as something else in drm.

To my electrical engineer's ear, passive refers to a component or combination 
of components that is not capable of power gain. The resistors ladder VGA DAC 
that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC 
that equally requires no configuration is an active component.

> Now that I've gotten that out of the way, this patch looks good to me
> regardless of the name.
> 
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> 
> Sean
> 
> >> In other words, this driver shouldn't be touched again in the future :)
> >> If someone wants to write a RGB to VGA driver which is even
> >> slightly configurable, they'll need to write a new bridge driver.
> > 
> > I'm sure that won't be true. I can certainly foresee the addition of
> > regulators support for instance. It's unfortunately never black and white.
> > 
> >>>> What do you think about this? If you think it's good, would it be
> >>>> possible for you to change this? I guess it's okay for the rest of
> >>>> the patch to stay the same.
> >>> 
> >>> I'll update and respin the serie.

-- 
Regards,

Laurent Pinchart

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

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Sean,

On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> > On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>> Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>> either DACs or resistor ladders.
> >>>>> 
> >>>>> Those might or might not have an i2c bus routed to the VGA connector
> >>>>> in order to access the screen EDIDs.
> >>>>> 
> >>>>> Add a bridge that doesn't do anything but expose the modes available
> >>>>> on the screen, either based on the EDIDs if available, or based on
> >>>>> the XGA standards.
> >>>>> 
> >>>>> Acked-by: Rob Herring <robh@kernel.org>
> >>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>> ---
> >>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
> >>>>> 4 files changed, 285 insertions(+)
> >>>>> create mode 100644
> >>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>>>> t
> >>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> txt
> >>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..a8375bc1f9cb
> >>>>> --- /dev/null
> >>>>> +++
> >>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>> tx
> >>>>> t @@ -0,0 +1,48 @@
> >>>>> +Dumb RGB to VGA bridge
> >>>>> +----------------------
> >>>>> +
> >>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>> require
> >>>>> +any configuration.
> >>>>> +
> >>>>> +Required properties:
> >>>>> +
> >>>>> +- compatible: Must be "rgb-to-vga-bridge"
> >>>> 
> >>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>> to had discussed it during XDC too. He's suggested that it'd be better
> >>>> to have the compatible string as "simple-vga-dac".
> >>> 
> >>> I just wished this bikeshedding had taken place publicly and be
> >>> actually part of that discussion, but yeah, ok.
> >> 
> >> Sorry about that. I'd pinged him for an Ack, the discussion went
> >> more than that :)
> >> 
> >>>> Some of the reasons behind having this:
> >>>> 
> >>>> - We don't need to specify "rgb" in the compatible string since most
> >>>> simple VGA DACs can only work with an RGB input.
> >>> 
> >>> Ok.
> >>> 
> >>>> - Also, with "dac" specified in the string, we don't need to
> >>>> specifically mention "bridge" in the string. Also, bridge is a drm
> >>>> specific term.
> >>>> 
> >>>> - "simple" is considered because it's an unconfigurable bridge, and it
> >>>> might be misleading for other VGA DACs to not use "vga-dac".
> >>> 
> >>> All those "simple" bindings are just the biggest lie we ever
> >>> told. It's simple when you introduce it, and then grows into something
> >>> much more complicated than a non-simple implementation.
> >> 
> >> "simple" here is supposed to mean that it's an unconfigurable RGB to
> >> VGA DAC. This isn't supposed to follow the simple-panel model, where
> >> you add the "simple-panel" string in the compatible node, along with
> >> you chip specific compatible string.
> > 
> > I agree with Maxime, I don't like the word "simple". My preference would
> > be "vga-dac" for a lack of a better qualifier than "simple" to describe
> > the fact that the device requires no configuration. My only concern with
> > "vga-dac" is that we would restrict usage of that compatible string for a
> > subset of VGA DACs, with more complex devices not being compatible with
> > "vga-dac" even though they are VGA DACs. That's a problem I can live with
> > though.
>
> While we're bikeshedding (feel free to ignore my input on this), I
> think Maxime's initial "dumb" qualifier was better than "simple".

I think I agree.

> I think "passive" also gets the point across better than "simple", which
> we've already established as something else in drm.

To my electrical engineer's ear, passive refers to a component or combination 
of components that is not capable of power gain. The resistors ladder VGA DAC 
that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC 
that equally requires no configuration is an active component.

> Now that I've gotten that out of the way, this patch looks good to me
> regardless of the name.
> 
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> 
> Sean
> 
> >> In other words, this driver shouldn't be touched again in the future :)
> >> If someone wants to write a RGB to VGA driver which is even
> >> slightly configurable, they'll need to write a new bridge driver.
> > 
> > I'm sure that won't be true. I can certainly foresee the addition of
> > regulators support for instance. It's unfortunately never black and white.
> > 
> >>>> What do you think about this? If you think it's good, would it be
> >>>> possible for you to change this? I guess it's okay for the rest of
> >>>> the patch to stay the same.
> >>> 
> >>> I'll update and respin the serie.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> Hi Sean,
>
> On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
>> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
>>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
>>>>>>> either DACs or resistor ladders.
>>>>>>>
>>>>>>> Those might or might not have an i2c bus routed to the VGA connector
>>>>>>> in order to access the screen EDIDs.
>>>>>>>
>>>>>>> Add a bridge that doesn't do anything but expose the modes available
>>>>>>> on the screen, either based on the EDIDs if available, or based on
>>>>>>> the XGA standards.
>>>>>>>
>>>>>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>> ---
>>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
>>>>>>> 4 files changed, 285 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>>>>>>> t
>>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..a8375bc1f9cb
>>>>>>> --- /dev/null
>>>>>>> +++
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> tx
>>>>>>> t @@ -0,0 +1,48 @@
>>>>>>> +Dumb RGB to VGA bridge
>>>>>>> +----------------------
>>>>>>> +
>>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
>>>>>>> require
>>>>>>> +any configuration.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
>>>>>>
>>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
>>>>>> to had discussed it during XDC too. He's suggested that it'd be better
>>>>>> to have the compatible string as "simple-vga-dac".
>>>>>
>>>>> I just wished this bikeshedding had taken place publicly and be
>>>>> actually part of that discussion, but yeah, ok.
>>>>
>>>> Sorry about that. I'd pinged him for an Ack, the discussion went
>>>> more than that :)
>>>>
>>>>>> Some of the reasons behind having this:
>>>>>>
>>>>>> - We don't need to specify "rgb" in the compatible string since most
>>>>>> simple VGA DACs can only work with an RGB input.
>>>>>
>>>>> Ok.
>>>>>
>>>>>> - Also, with "dac" specified in the string, we don't need to
>>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
>>>>>> specific term.
>>>>>>
>>>>>> - "simple" is considered because it's an unconfigurable bridge, and it
>>>>>> might be misleading for other VGA DACs to not use "vga-dac".
>>>>>
>>>>> All those "simple" bindings are just the biggest lie we ever
>>>>> told. It's simple when you introduce it, and then grows into something
>>>>> much more complicated than a non-simple implementation.
>>>>
>>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>>>> you add the "simple-panel" string in the compatible node, along with
>>>> you chip specific compatible string.
>>>
>>> I agree with Maxime, I don't like the word "simple". My preference would
>>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
>>> the fact that the device requires no configuration. My only concern with
>>> "vga-dac" is that we would restrict usage of that compatible string for a
>>> subset of VGA DACs, with more complex devices not being compatible with
>>> "vga-dac" even though they are VGA DACs. That's a problem I can live with
>>> though.
>>
>> While we're bikeshedding (feel free to ignore my input on this), I
>> think Maxime's initial "dumb" qualifier was better than "simple".
>
> I think I agree.
>
>> I think "passive" also gets the point across better than "simple", which
>> we've already established as something else in drm.
>
> To my electrical engineer's ear, passive refers to a component or combination
> of components that is not capable of power gain. The resistors ladder VGA DAC
> that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> that equally requires no configuration is an active component.

If no one has any more objections within the next day, I'll pull in
Maxime's v5 RGB to VGA bridge driver, and change the compatible to
"dumb-vga-dac".

Thanks,
Archit


>
>> Now that I've gotten that out of the way, this patch looks good to me
>> regardless of the name.
>>
>> Reviewed-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> Sean
>>
>>>> In other words, this driver shouldn't be touched again in the future :)
>>>> If someone wants to write a RGB to VGA driver which is even
>>>> slightly configurable, they'll need to write a new bridge driver.
>>>
>>> I'm sure that won't be true. I can certainly foresee the addition of
>>> regulators support for instance. It's unfortunately never black and white.
>>>
>>>>>> What do you think about this? If you think it's good, would it be
>>>>>> possible for you to change this? I guess it's okay for the rest of
>>>>>> the patch to stay the same.
>>>>>
>>>>> I'll update and respin the serie.
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> Hi Sean,
>
> On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
>> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
>>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
>>>>>>> either DACs or resistor ladders.
>>>>>>>
>>>>>>> Those might or might not have an i2c bus routed to the VGA connector
>>>>>>> in order to access the screen EDIDs.
>>>>>>>
>>>>>>> Add a bridge that doesn't do anything but expose the modes available
>>>>>>> on the screen, either based on the EDIDs if available, or based on
>>>>>>> the XGA standards.
>>>>>>>
>>>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>>>>> ---
>>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
>>>>>>> 4 files changed, 285 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>>>>>>> t
>>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..a8375bc1f9cb
>>>>>>> --- /dev/null
>>>>>>> +++
>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>> tx
>>>>>>> t @@ -0,0 +1,48 @@
>>>>>>> +Dumb RGB to VGA bridge
>>>>>>> +----------------------
>>>>>>> +
>>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
>>>>>>> require
>>>>>>> +any configuration.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
>>>>>>
>>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
>>>>>> to had discussed it during XDC too. He's suggested that it'd be better
>>>>>> to have the compatible string as "simple-vga-dac".
>>>>>
>>>>> I just wished this bikeshedding had taken place publicly and be
>>>>> actually part of that discussion, but yeah, ok.
>>>>
>>>> Sorry about that. I'd pinged him for an Ack, the discussion went
>>>> more than that :)
>>>>
>>>>>> Some of the reasons behind having this:
>>>>>>
>>>>>> - We don't need to specify "rgb" in the compatible string since most
>>>>>> simple VGA DACs can only work with an RGB input.
>>>>>
>>>>> Ok.
>>>>>
>>>>>> - Also, with "dac" specified in the string, we don't need to
>>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
>>>>>> specific term.
>>>>>>
>>>>>> - "simple" is considered because it's an unconfigurable bridge, and it
>>>>>> might be misleading for other VGA DACs to not use "vga-dac".
>>>>>
>>>>> All those "simple" bindings are just the biggest lie we ever
>>>>> told. It's simple when you introduce it, and then grows into something
>>>>> much more complicated than a non-simple implementation.
>>>>
>>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>>>> you add the "simple-panel" string in the compatible node, along with
>>>> you chip specific compatible string.
>>>
>>> I agree with Maxime, I don't like the word "simple". My preference would
>>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
>>> the fact that the device requires no configuration. My only concern with
>>> "vga-dac" is that we would restrict usage of that compatible string for a
>>> subset of VGA DACs, with more complex devices not being compatible with
>>> "vga-dac" even though they are VGA DACs. That's a problem I can live with
>>> though.
>>
>> While we're bikeshedding (feel free to ignore my input on this), I
>> think Maxime's initial "dumb" qualifier was better than "simple".
>
> I think I agree.
>
>> I think "passive" also gets the point across better than "simple", which
>> we've already established as something else in drm.
>
> To my electrical engineer's ear, passive refers to a component or combination
> of components that is not capable of power gain. The resistors ladder VGA DAC
> that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> that equally requires no configuration is an active component.

If no one has any more objections within the next day, I'll pull in
Maxime's v5 RGB to VGA bridge driver, and change the compatible to
"dumb-vga-dac".

Thanks,
Archit


>
>> Now that I've gotten that out of the way, this patch looks good to me
>> regardless of the name.
>>
>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>>
>> Sean
>>
>>>> In other words, this driver shouldn't be touched again in the future :)
>>>> If someone wants to write a RGB to VGA driver which is even
>>>> slightly configurable, they'll need to write a new bridge driver.
>>>
>>> I'm sure that won't be true. I can certainly foresee the addition of
>>> regulators support for instance. It's unfortunately never black and white.
>>>
>>>>>> What do you think about this? If you think it's good, would it be
>>>>>> possible for you to change this? I guess it's okay for the rest of
>>>>>> the patch to stay the same.
>>>>>
>>>>> I'll update and respin the serie.
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

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

On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
> 
> 
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> >Hi Sean,
> >
> >On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> >>On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> >>>On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >>>>On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>>>>On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>>>>On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>>>>Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>>>>either DACs or resistor ladders.
> >>>>>>>
> >>>>>>>Those might or might not have an i2c bus routed to the VGA connector
> >>>>>>>in order to access the screen EDIDs.
> >>>>>>>
> >>>>>>>Add a bridge that doesn't do anything but expose the modes available
> >>>>>>>on the screen, either based on the EDIDs if available, or based on
> >>>>>>>the XGA standards.
> >>>>>>>
> >>>>>>>Acked-by: Rob Herring <robh@kernel.org>
> >>>>>>>Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>>>>---
> >>>>>>>.../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>>>>drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>>>>drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>>>>drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
> >>>>>>>4 files changed, 285 insertions(+)
> >>>>>>>create mode 100644
> >>>>>>>Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>>>>>>t
> >>>>>>>create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>>>>
> >>>>>>>diff --git
> >>>>>>>a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>new file mode 100644
> >>>>>>>index 000000000000..a8375bc1f9cb
> >>>>>>>--- /dev/null
> >>>>>>>+++
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>tx
> >>>>>>>t @@ -0,0 +1,48 @@
> >>>>>>>+Dumb RGB to VGA bridge
> >>>>>>>+----------------------
> >>>>>>>+
> >>>>>>>+This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>>>>require
> >>>>>>>+any configuration.
> >>>>>>>+
> >>>>>>>+Required properties:
> >>>>>>>+
> >>>>>>>+- compatible: Must be "rgb-to-vga-bridge"
> >>>>>>
> >>>>>>I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>>>>to had discussed it during XDC too. He's suggested that it'd be better
> >>>>>>to have the compatible string as "simple-vga-dac".
> >>>>>
> >>>>>I just wished this bikeshedding had taken place publicly and be
> >>>>>actually part of that discussion, but yeah, ok.
> >>>>
> >>>>Sorry about that. I'd pinged him for an Ack, the discussion went
> >>>>more than that :)
> >>>>
> >>>>>>Some of the reasons behind having this:
> >>>>>>
> >>>>>>- We don't need to specify "rgb" in the compatible string since most
> >>>>>>simple VGA DACs can only work with an RGB input.
> >>>>>
> >>>>>Ok.
> >>>>>
> >>>>>>- Also, with "dac" specified in the string, we don't need to
> >>>>>>specifically mention "bridge" in the string. Also, bridge is a drm
> >>>>>>specific term.
> >>>>>>
> >>>>>>- "simple" is considered because it's an unconfigurable bridge, and it
> >>>>>>might be misleading for other VGA DACs to not use "vga-dac".
> >>>>>
> >>>>>All those "simple" bindings are just the biggest lie we ever
> >>>>>told. It's simple when you introduce it, and then grows into something
> >>>>>much more complicated than a non-simple implementation.
> >>>>
> >>>>"simple" here is supposed to mean that it's an unconfigurable RGB to
> >>>>VGA DAC. This isn't supposed to follow the simple-panel model, where
> >>>>you add the "simple-panel" string in the compatible node, along with
> >>>>you chip specific compatible string.
> >>>
> >>>I agree with Maxime, I don't like the word "simple". My preference would
> >>>be "vga-dac" for a lack of a better qualifier than "simple" to describe
> >>>the fact that the device requires no configuration. My only concern with
> >>>"vga-dac" is that we would restrict usage of that compatible string for a
> >>>subset of VGA DACs, with more complex devices not being compatible with
> >>>"vga-dac" even though they are VGA DACs. That's a problem I can live with
> >>>though.
> >>
> >>While we're bikeshedding (feel free to ignore my input on this), I
> >>think Maxime's initial "dumb" qualifier was better than "simple".
> >
> >I think I agree.
> >
> >>I think "passive" also gets the point across better than "simple", which
> >>we've already established as something else in drm.
> >
> >To my electrical engineer's ear, passive refers to a component or combination
> >of components that is not capable of power gain. The resistors ladder VGA DAC
> >that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> >that equally requires no configuration is an active component.
> 
> If no one has any more objections within the next day, I'll pull in
> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
> "dumb-vga-dac".

That works for me. You'll probably want to update the Kconfig and file
name to match though.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

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

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Maxime Ripard]

On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
> 
> 
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> >Hi Sean,
> >
> >On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> >>On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> >>>On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >>>>On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>>>>On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>>>>On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>>>>Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>>>>either DACs or resistor ladders.
> >>>>>>>
> >>>>>>>Those might or might not have an i2c bus routed to the VGA connector
> >>>>>>>in order to access the screen EDIDs.
> >>>>>>>
> >>>>>>>Add a bridge that doesn't do anything but expose the modes available
> >>>>>>>on the screen, either based on the EDIDs if available, or based on
> >>>>>>>the XGA standards.
> >>>>>>>
> >>>>>>>Acked-by: Rob Herring <robh@kernel.org>
> >>>>>>>Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>>>>---
> >>>>>>>.../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>>>>drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>>>>drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>>>>drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
> >>>>>>>4 files changed, 285 insertions(+)
> >>>>>>>create mode 100644
> >>>>>>>Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
> >>>>>>>t
> >>>>>>>create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>>>>
> >>>>>>>diff --git
> >>>>>>>a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>txt
> >>>>>>>new file mode 100644
> >>>>>>>index 000000000000..a8375bc1f9cb
> >>>>>>>--- /dev/null
> >>>>>>>+++
> >>>>>>>b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
> >>>>>>>tx
> >>>>>>>t @@ -0,0 +1,48 @@
> >>>>>>>+Dumb RGB to VGA bridge
> >>>>>>>+----------------------
> >>>>>>>+
> >>>>>>>+This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>>>>require
> >>>>>>>+any configuration.
> >>>>>>>+
> >>>>>>>+Required properties:
> >>>>>>>+
> >>>>>>>+- compatible: Must be "rgb-to-vga-bridge"
> >>>>>>
> >>>>>>I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>>>>to had discussed it during XDC too. He's suggested that it'd be better
> >>>>>>to have the compatible string as "simple-vga-dac".
> >>>>>
> >>>>>I just wished this bikeshedding had taken place publicly and be
> >>>>>actually part of that discussion, but yeah, ok.
> >>>>
> >>>>Sorry about that. I'd pinged him for an Ack, the discussion went
> >>>>more than that :)
> >>>>
> >>>>>>Some of the reasons behind having this:
> >>>>>>
> >>>>>>- We don't need to specify "rgb" in the compatible string since most
> >>>>>>simple VGA DACs can only work with an RGB input.
> >>>>>
> >>>>>Ok.
> >>>>>
> >>>>>>- Also, with "dac" specified in the string, we don't need to
> >>>>>>specifically mention "bridge" in the string. Also, bridge is a drm
> >>>>>>specific term.
> >>>>>>
> >>>>>>- "simple" is considered because it's an unconfigurable bridge, and it
> >>>>>>might be misleading for other VGA DACs to not use "vga-dac".
> >>>>>
> >>>>>All those "simple" bindings are just the biggest lie we ever
> >>>>>told. It's simple when you introduce it, and then grows into something
> >>>>>much more complicated than a non-simple implementation.
> >>>>
> >>>>"simple" here is supposed to mean that it's an unconfigurable RGB to
> >>>>VGA DAC. This isn't supposed to follow the simple-panel model, where
> >>>>you add the "simple-panel" string in the compatible node, along with
> >>>>you chip specific compatible string.
> >>>
> >>>I agree with Maxime, I don't like the word "simple". My preference would
> >>>be "vga-dac" for a lack of a better qualifier than "simple" to describe
> >>>the fact that the device requires no configuration. My only concern with
> >>>"vga-dac" is that we would restrict usage of that compatible string for a
> >>>subset of VGA DACs, with more complex devices not being compatible with
> >>>"vga-dac" even though they are VGA DACs. That's a problem I can live with
> >>>though.
> >>
> >>While we're bikeshedding (feel free to ignore my input on this), I
> >>think Maxime's initial "dumb" qualifier was better than "simple".
> >
> >I think I agree.
> >
> >>I think "passive" also gets the point across better than "simple", which
> >>we've already established as something else in drm.
> >
> >To my electrical engineer's ear, passive refers to a component or combination
> >of components that is not capable of power gain. The resistors ladder VGA DAC
> >that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
> >that equally requires no configuration is an active component.
> 
> If no one has any more objections within the next day, I'll pull in
> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
> "dumb-vga-dac".

That works for me. You'll probably want to update the Kconfig and file
name to match though.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161007/cac9efa7/attachment-0001.sig>

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Archit,

On Friday 07 Oct 2016 10:27:31 Archit Taneja wrote:
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> > On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> >> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> >>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>>>> either DACs or resistor ladders.
> >>>>>>> 
> >>>>>>> Those might or might not have an i2c bus routed to the VGA connector
> >>>>>>> in order to access the screen EDIDs.
> >>>>>>> 
> >>>>>>> Add a bridge that doesn't do anything but expose the modes available
> >>>>>>> on the screen, either based on the EDIDs if available, or based on
> >>>>>>> the XGA standards.
> >>>>>>> 
> >>>>>>> Acked-by: Rob Herring <robh@kernel.org>
> >>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>>>> ---
> >>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++
> >>>>>>> 4 files changed, 285 insertions(+)
> >>>>>>> create mode 100644
> >>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.t
> >>>>>>> xt
> >>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>>>> 
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> txt
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> txt
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000000..a8375bc1f9cb
> >>>>>>> --- /dev/null
> >>>>>>> +++
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> tx
> >>>>>>> t @@ -0,0 +1,48 @@
> >>>>>>> +Dumb RGB to VGA bridge
> >>>>>>> +----------------------
> >>>>>>> +
> >>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>>>> require
> >>>>>>> +any configuration.
> >>>>>>> +
> >>>>>>> +Required properties:
> >>>>>>> +
> >>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
> >>>>>> 
> >>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>>>> to had discussed it during XDC too. He's suggested that it'd be
> >>>>>> better
> >>>>>> to have the compatible string as "simple-vga-dac".
> >>>>> 
> >>>>> I just wished this bikeshedding had taken place publicly and be
> >>>>> actually part of that discussion, but yeah, ok.
> >>>> 
> >>>> Sorry about that. I'd pinged him for an Ack, the discussion went
> >>>> more than that :)
> >>>> 
> >>>>>> Some of the reasons behind having this:
> >>>>>> 
> >>>>>> - We don't need to specify "rgb" in the compatible string since most
> >>>>>> simple VGA DACs can only work with an RGB input.
> >>>>> 
> >>>>> Ok.
> >>>>> 
> >>>>>> - Also, with "dac" specified in the string, we don't need to
> >>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
> >>>>>> specific term.
> >>>>>> 
> >>>>>> - "simple" is considered because it's an unconfigurable bridge, and
> >>>>>> it might be misleading for other VGA DACs to not use "vga-dac".
> >>>>> 
> >>>>> All those "simple" bindings are just the biggest lie we ever
> >>>>> told. It's simple when you introduce it, and then grows into something
> >>>>> much more complicated than a non-simple implementation.
> >>>> 
> >>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
> >>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
> >>>> you add the "simple-panel" string in the compatible node, along with
> >>>> you chip specific compatible string.
> >>> 
> >>> I agree with Maxime, I don't like the word "simple". My preference would
> >>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
> >>> the fact that the device requires no configuration. My only concern with
> >>> "vga-dac" is that we would restrict usage of that compatible string for
> >>> a subset of VGA DACs, with more complex devices not being compatible
> >>> with "vga-dac" even though they are VGA DACs. That's a problem I can
> >>> live with though.
> >> 
> >> While we're bikeshedding (feel free to ignore my input on this), I
> >> think Maxime's initial "dumb" qualifier was better than "simple".
> > 
> > I think I agree.
> > 
> >> I think "passive" also gets the point across better than "simple", which
> >> we've already established as something else in drm.
> > 
> > To my electrical engineer's ear, passive refers to a component or
> > combination of components that is not capable of power gain. The
> > resistors ladder VGA DAC that Maxime is trying to support is a passive
> > system, but the ADV7123 VGA DAC that equally requires no configuration is
> > an active component.
> 
> If no one has any more objections within the next day, I'll pull in
> Maxime's v5 RGB to VGA bridge driver,

I'm testing the patch with rcar-du-drm and will provide results today.

> and change the compatible to "dumb-vga-dac".

Feel free to ignore the bikeshedding, but "transparent" could be a candidate 
to replace "dumb" (either as "vga-dac-transparent" or "transparent-vga-dac").

-- 
Regards,

Laurent Pinchart

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

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Archit,

On Friday 07 Oct 2016 10:27:31 Archit Taneja wrote:
> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
> > On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
> >> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
> >>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
> >>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
> >>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
> >>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
> >>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
> >>>>>>> either DACs or resistor ladders.
> >>>>>>> 
> >>>>>>> Those might or might not have an i2c bus routed to the VGA connector
> >>>>>>> in order to access the screen EDIDs.
> >>>>>>> 
> >>>>>>> Add a bridge that doesn't do anything but expose the modes available
> >>>>>>> on the screen, either based on the EDIDs if available, or based on
> >>>>>>> the XGA standards.
> >>>>>>> 
> >>>>>>> Acked-by: Rob Herring <robh@kernel.org>
> >>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>>>>> ---
> >>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
> >>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
> >>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
> >>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++
> >>>>>>> 4 files changed, 285 insertions(+)
> >>>>>>> create mode 100644
> >>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.t
> >>>>>>> xt
> >>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> >>>>>>> 
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> txt
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> txt
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000000..a8375bc1f9cb
> >>>>>>> --- /dev/null
> >>>>>>> +++
> >>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge
> >>>>>>> .
> >>>>>>> tx
> >>>>>>> t @@ -0,0 +1,48 @@
> >>>>>>> +Dumb RGB to VGA bridge
> >>>>>>> +----------------------
> >>>>>>> +
> >>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
> >>>>>>> require
> >>>>>>> +any configuration.
> >>>>>>> +
> >>>>>>> +Required properties:
> >>>>>>> +
> >>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
> >>>>>> 
> >>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
> >>>>>> to had discussed it during XDC too. He's suggested that it'd be
> >>>>>> better
> >>>>>> to have the compatible string as "simple-vga-dac".
> >>>>> 
> >>>>> I just wished this bikeshedding had taken place publicly and be
> >>>>> actually part of that discussion, but yeah, ok.
> >>>> 
> >>>> Sorry about that. I'd pinged him for an Ack, the discussion went
> >>>> more than that :)
> >>>> 
> >>>>>> Some of the reasons behind having this:
> >>>>>> 
> >>>>>> - We don't need to specify "rgb" in the compatible string since most
> >>>>>> simple VGA DACs can only work with an RGB input.
> >>>>> 
> >>>>> Ok.
> >>>>> 
> >>>>>> - Also, with "dac" specified in the string, we don't need to
> >>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
> >>>>>> specific term.
> >>>>>> 
> >>>>>> - "simple" is considered because it's an unconfigurable bridge, and
> >>>>>> it might be misleading for other VGA DACs to not use "vga-dac".
> >>>>> 
> >>>>> All those "simple" bindings are just the biggest lie we ever
> >>>>> told. It's simple when you introduce it, and then grows into something
> >>>>> much more complicated than a non-simple implementation.
> >>>> 
> >>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
> >>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
> >>>> you add the "simple-panel" string in the compatible node, along with
> >>>> you chip specific compatible string.
> >>> 
> >>> I agree with Maxime, I don't like the word "simple". My preference would
> >>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
> >>> the fact that the device requires no configuration. My only concern with
> >>> "vga-dac" is that we would restrict usage of that compatible string for
> >>> a subset of VGA DACs, with more complex devices not being compatible
> >>> with "vga-dac" even though they are VGA DACs. That's a problem I can
> >>> live with though.
> >> 
> >> While we're bikeshedding (feel free to ignore my input on this), I
> >> think Maxime's initial "dumb" qualifier was better than "simple".
> > 
> > I think I agree.
> > 
> >> I think "passive" also gets the point across better than "simple", which
> >> we've already established as something else in drm.
> > 
> > To my electrical engineer's ear, passive refers to a component or
> > combination of components that is not capable of power gain. The
> > resistors ladder VGA DAC that Maxime is trying to support is a passive
> > system, but the ADV7123 VGA DAC that equally requires no configuration is
> > an active component.
> 
> If no one has any more objections within the next day, I'll pull in
> Maxime's v5 RGB to VGA bridge driver,

I'm testing the patch with rcar-du-drm and will provide results today.

> and change the compatible to "dumb-vga-dac".

Feel free to ignore the bikeshedding, but "transparent" could be a candidate 
to replace "dumb" (either as "vga-dac-transparent" or "transparent-vga-dac").

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Friday 30 Sep 2016 16:37:06 Maxime Ripard wrote:
> Some boards have an entirely passive RGB to VGA bridge, based on either
> DACs or resistor ladders.

A resistor ladder is a DAC :-)

> Those might or might not have an i2c bus routed to the VGA connector in
> order to access the screen EDIDs.
> 
> Add a bridge that doesn't do anything but expose the modes available on the
> screen, either based on the EDIDs if available, or based on the XGA
> standards.
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Tested-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Please see below for a few comments.

> ---
>  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 ++++++++++++++++++
>  4 files changed, 285 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> new file mode 100644
> index 000000000000..a8375bc1f9cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> @@ -0,0 +1,48 @@
> +Dumb RGB to VGA bridge
> +----------------------
> +
> +This binding is aimed for dumb RGB to VGA bridges that do not require
> +any configuration.
> +
> +Required properties:
> +
> +- compatible: Must be "rgb-to-vga-bridge"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt. +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +
> +Example
> +-------
> +
> +bridge {
> +	compatible = "rgb-to-vga-bridge";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			vga_bridge_in: endpoint {
> +				remote-endpoint = <&tcon0_out_vga>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			vga_bridge_out: endpoint {
> +				remote-endpoint = <&vga_con_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e678052d..d690398c541c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
>  	  the HDMI output of an application processor to MyDP
>  	  or DisplayPort.
> 
> +config DRM_RGB_TO_VGA
> +	tristate "Dumb RGB to VGA Bridge support"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for passive RGB to VGA bridges
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile
> b/drivers/gpu/drm/bridge/Makefile index efdb07e878f5..3bb8cbe09fe9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
>  ccflags-y := -Iinclude/drm
> 
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> +obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c
> b/drivers/gpu/drm/bridge/rgb-to-vga.c new file mode 100644
> index 000000000000..5ff4d4f3598f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
> @@ -0,0 +1,229 @@
> +/*
> + * Copyright (C) 2015-2016 Free Electrons
> + * Copyright (C) 2015-2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct dumb_vga {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +};
> +
> +static inline struct dumb_vga *
> +drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct dumb_vga, bridge);
> +}
> +
> +static inline struct dumb_vga *
> +drm_connector_to_dumb_vga(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct dumb_vga, connector);
> +}
> +
> +static int dumb_vga_get_modes(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (IS_ERR(vga->ddc))
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, vga->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID readout failed, falling back to standard 
modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	return drm_add_edid_modes(connector, edid);
> +
> +fallback:
> +	/*
> +	 * In case we cannot retrieve the EDIDs (broken or missing i2c
> +	 * bus), fallback on the XGA standards
> +	 */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anyone can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs =
> { +	.get_modes	= dumb_vga_get_modes,
> +};
> +
> +static enum drm_connector_status
> +dumb_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	/*
> +	 * Even if we have an I2C bus, we can't assume that the cable
> +	 * is disconnected if drm_probe_ddc fails. Some cables don't
> +	 * wire the DDC pins, or the I2C bus might not be working at
> +	 * all.
> +	 */
> +	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
> +		return connector_status_connected;
> +
> +	return connector_status_unknown;
> +}
> +
> +static void
> +dumb_vga_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs dumb_vga_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= dumb_vga_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= dumb_vga_connector_destroy,

You can use drm_connector_cleanup directly here.

> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int dumb_vga_attach(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&vga->connector,
> +				 &dumb_vga_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &vga->connector,
> +				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&vga->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> +	.attach		= dumb_vga_attach,
> +};
> +
> +static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> +{
> +	struct device_node *end_node, *phandle, *remote;
> +	struct i2c_adapter *ddc;
> +
> +	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!end_node) {
> +		dev_err(dev, "Missing connector endpoint\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(end_node);
> +	of_node_put(end_node);
> +	if (!remote) {
> +		dev_err(dev, "Enable to parse remote node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +	of_node_put(remote);
> +	if (!phandle)
> +		return ERR_PTR(-ENODEV);
> +
> +	ddc = of_get_i2c_adapter_by_node(phandle);
> +	of_node_put(phandle);
> +	if (!ddc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return ddc;
> +}
> +
> +static int dumb_vga_probe(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga;
> +	int ret;
> +
> +	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, vga);
> +
> +	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> +	if (IS_ERR(vga->ddc)) {
> +		if (PTR_ERR(vga->ddc) == -ENODEV) {
> +			dev_info(&pdev->dev,
> +				 "No i2c bus specified... Disabling EDID 
readout\n");

Nitpicking, there's no need for an ellipsis ("..."). I'd write the message as 
"DDC not available, disabling EDID readout".

You could also turn it into a dev_dbg() message as I'm not sure it's really 
crucial information, that's up to you.

> +		} else {
> +			dev_err(&pdev->dev, "Couldn't retrieve i2c bus\n");
> +			return PTR_ERR(vga->ddc);
> +		}
> +	}
> +
> +	vga->bridge.funcs = &dumb_vga_bridge_funcs;
> +	vga->bridge.of_node = pdev->dev.of_node;
> +
> +	ret = drm_bridge_add(&vga->bridge);
> +	if (ret && !IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return ret;
> +}
> +
> +static int dumb_vga_remove(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&vga->bridge);
> +
> +	if (!IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dumb_vga_match[] = {
> +	{ .compatible = "rgb-to-vga-bridge" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dumb_vga_match);
> +
> +struct platform_driver dumb_vga_driver = {
> +	.probe	= dumb_vga_probe,
> +	.remove	= dumb_vga_remove,
> +	.driver		= {
> +		.name		= "rgb-to-vga-bridge",

If we changing the compatible string to "dumb-vga-dac" as proposed by Archit, 
let's not forget to rename the driver (dumb-vga-dac.ko seems a good match) as 
well as the description string below ("Dumb VGA DAC bridge driver" ?). Both 
The DT binding and Kconfig texts need to be updated as well.

I would also rename struct dumb_vga to dumb_vga_dac, that's up to you.

> +		.of_match_table	= dumb_vga_match,
> +	},
> +};
> +module_platform_driver(dumb_vga_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> +MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Friday 30 Sep 2016 16:37:06 Maxime Ripard wrote:
> Some boards have an entirely passive RGB to VGA bridge, based on either
> DACs or resistor ladders.

A resistor ladder is a DAC :-)

> Those might or might not have an i2c bus routed to the VGA connector in
> order to access the screen EDIDs.
> 
> Add a bridge that doesn't do anything but expose the modes available on the
> screen, either based on the EDIDs if available, or based on the XGA
> standards.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Please see below for a few comments.

> ---
>  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 ++++++++++++++++++
>  4 files changed, 285 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> new file mode 100644
> index 000000000000..a8375bc1f9cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> @@ -0,0 +1,48 @@
> +Dumb RGB to VGA bridge
> +----------------------
> +
> +This binding is aimed for dumb RGB to VGA bridges that do not require
> +any configuration.
> +
> +Required properties:
> +
> +- compatible: Must be "rgb-to-vga-bridge"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt. +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +
> +Example
> +-------
> +
> +bridge {
> +	compatible = "rgb-to-vga-bridge";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port at 0 {
> +			reg = <0>;
> +
> +			vga_bridge_in: endpoint {
> +				remote-endpoint = <&tcon0_out_vga>;
> +			};
> +		};
> +
> +		port at 1 {
> +			reg = <1>;
> +
> +			vga_bridge_out: endpoint {
> +				remote-endpoint = <&vga_con_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e678052d..d690398c541c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -17,6 +17,13 @@ config DRM_ANALOGIX_ANX78XX
>  	  the HDMI output of an application processor to MyDP
>  	  or DisplayPort.
> 
> +config DRM_RGB_TO_VGA
> +	tristate "Dumb RGB to VGA Bridge support"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for passive RGB to VGA bridges
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile
> b/drivers/gpu/drm/bridge/Makefile index efdb07e878f5..3bb8cbe09fe9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
>  ccflags-y := -Iinclude/drm
> 
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> +obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c
> b/drivers/gpu/drm/bridge/rgb-to-vga.c new file mode 100644
> index 000000000000..5ff4d4f3598f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
> @@ -0,0 +1,229 @@
> +/*
> + * Copyright (C) 2015-2016 Free Electrons
> + * Copyright (C) 2015-2016 NextThing Co
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct dumb_vga {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +};
> +
> +static inline struct dumb_vga *
> +drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct dumb_vga, bridge);
> +}
> +
> +static inline struct dumb_vga *
> +drm_connector_to_dumb_vga(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct dumb_vga, connector);
> +}
> +
> +static int dumb_vga_get_modes(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (IS_ERR(vga->ddc))
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, vga->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID readout failed, falling back to standard 
modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	return drm_add_edid_modes(connector, edid);
> +
> +fallback:
> +	/*
> +	 * In case we cannot retrieve the EDIDs (broken or missing i2c
> +	 * bus), fallback on the XGA standards
> +	 */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anyone can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs dumb_vga_con_helper_funcs =
> { +	.get_modes	= dumb_vga_get_modes,
> +};
> +
> +static enum drm_connector_status
> +dumb_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	/*
> +	 * Even if we have an I2C bus, we can't assume that the cable
> +	 * is disconnected if drm_probe_ddc fails. Some cables don't
> +	 * wire the DDC pins, or the I2C bus might not be working at
> +	 * all.
> +	 */
> +	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
> +		return connector_status_connected;
> +
> +	return connector_status_unknown;
> +}
> +
> +static void
> +dumb_vga_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs dumb_vga_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= dumb_vga_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= dumb_vga_connector_destroy,

You can use drm_connector_cleanup directly here.

> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int dumb_vga_attach(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&vga->connector,
> +				 &dumb_vga_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &vga->connector,
> +				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&vga->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> +	.attach		= dumb_vga_attach,
> +};
> +
> +static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> +{
> +	struct device_node *end_node, *phandle, *remote;
> +	struct i2c_adapter *ddc;
> +
> +	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!end_node) {
> +		dev_err(dev, "Missing connector endpoint\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(end_node);
> +	of_node_put(end_node);
> +	if (!remote) {
> +		dev_err(dev, "Enable to parse remote node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +	of_node_put(remote);
> +	if (!phandle)
> +		return ERR_PTR(-ENODEV);
> +
> +	ddc = of_get_i2c_adapter_by_node(phandle);
> +	of_node_put(phandle);
> +	if (!ddc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return ddc;
> +}
> +
> +static int dumb_vga_probe(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga;
> +	int ret;
> +
> +	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, vga);
> +
> +	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> +	if (IS_ERR(vga->ddc)) {
> +		if (PTR_ERR(vga->ddc) == -ENODEV) {
> +			dev_info(&pdev->dev,
> +				 "No i2c bus specified... Disabling EDID 
readout\n");

Nitpicking, there's no need for an ellipsis ("..."). I'd write the message as 
"DDC not available, disabling EDID readout".

You could also turn it into a dev_dbg() message as I'm not sure it's really 
crucial information, that's up to you.

> +		} else {
> +			dev_err(&pdev->dev, "Couldn't retrieve i2c bus\n");
> +			return PTR_ERR(vga->ddc);
> +		}
> +	}
> +
> +	vga->bridge.funcs = &dumb_vga_bridge_funcs;
> +	vga->bridge.of_node = pdev->dev.of_node;
> +
> +	ret = drm_bridge_add(&vga->bridge);
> +	if (ret && !IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return ret;
> +}
> +
> +static int dumb_vga_remove(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&vga->bridge);
> +
> +	if (!IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dumb_vga_match[] = {
> +	{ .compatible = "rgb-to-vga-bridge" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dumb_vga_match);
> +
> +struct platform_driver dumb_vga_driver = {
> +	.probe	= dumb_vga_probe,
> +	.remove	= dumb_vga_remove,
> +	.driver		= {
> +		.name		= "rgb-to-vga-bridge",

If we changing the compatible string to "dumb-vga-dac" as proposed by Archit, 
let's not forget to rename the driver (dumb-vga-dac.ko seems a good match) as 
well as the description string below ("Dumb VGA DAC bridge driver" ?). Both 
The DT binding and Kconfig texts need to be updated as well.

I would also rename struct dumb_vga to dumb_vga_dac, that's up to you.

> +		.of_match_table	= dumb_vga_match,
> +	},
> +};
> +module_platform_driver(dumb_vga_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/07/2016 02:44 PM, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
>>
>>
>> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
>>> Hi Sean,
>>>
>>> On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
>>>> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
>>>>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>>>>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>>>>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>>>>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>>>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
>>>>>>>>> either DACs or resistor ladders.
>>>>>>>>>
>>>>>>>>> Those might or might not have an i2c bus routed to the VGA connector
>>>>>>>>> in order to access the screen EDIDs.
>>>>>>>>>
>>>>>>>>> Add a bridge that doesn't do anything but expose the modes available
>>>>>>>>> on the screen, either based on the EDIDs if available, or based on
>>>>>>>>> the XGA standards.
>>>>>>>>>
>>>>>>>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>>>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>>>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
>>>>>>>>> 4 files changed, 285 insertions(+)
>>>>>>>>> create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>>>>>>>>> t
>>>>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> txt
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000000..a8375bc1f9cb
>>>>>>>>> --- /dev/null
>>>>>>>>> +++
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> tx
>>>>>>>>> t @@ -0,0 +1,48 @@
>>>>>>>>> +Dumb RGB to VGA bridge
>>>>>>>>> +----------------------
>>>>>>>>> +
>>>>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
>>>>>>>>> require
>>>>>>>>> +any configuration.
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +
>>>>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
>>>>>>>>
>>>>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
>>>>>>>> to had discussed it during XDC too. He's suggested that it'd be better
>>>>>>>> to have the compatible string as "simple-vga-dac".
>>>>>>>
>>>>>>> I just wished this bikeshedding had taken place publicly and be
>>>>>>> actually part of that discussion, but yeah, ok.
>>>>>>
>>>>>> Sorry about that. I'd pinged him for an Ack, the discussion went
>>>>>> more than that :)
>>>>>>
>>>>>>>> Some of the reasons behind having this:
>>>>>>>>
>>>>>>>> - We don't need to specify "rgb" in the compatible string since most
>>>>>>>> simple VGA DACs can only work with an RGB input.
>>>>>>>
>>>>>>> Ok.
>>>>>>>
>>>>>>>> - Also, with "dac" specified in the string, we don't need to
>>>>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
>>>>>>>> specific term.
>>>>>>>>
>>>>>>>> - "simple" is considered because it's an unconfigurable bridge, and it
>>>>>>>> might be misleading for other VGA DACs to not use "vga-dac".
>>>>>>>
>>>>>>> All those "simple" bindings are just the biggest lie we ever
>>>>>>> told. It's simple when you introduce it, and then grows into something
>>>>>>> much more complicated than a non-simple implementation.
>>>>>>
>>>>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>>>>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>>>>>> you add the "simple-panel" string in the compatible node, along with
>>>>>> you chip specific compatible string.
>>>>>
>>>>> I agree with Maxime, I don't like the word "simple". My preference would
>>>>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
>>>>> the fact that the device requires no configuration. My only concern with
>>>>> "vga-dac" is that we would restrict usage of that compatible string for a
>>>>> subset of VGA DACs, with more complex devices not being compatible with
>>>>> "vga-dac" even though they are VGA DACs. That's a problem I can live with
>>>>> though.
>>>>
>>>> While we're bikeshedding (feel free to ignore my input on this), I
>>>> think Maxime's initial "dumb" qualifier was better than "simple".
>>>
>>> I think I agree.
>>>
>>>> I think "passive" also gets the point across better than "simple", which
>>>> we've already established as something else in drm.
>>>
>>> To my electrical engineer's ear, passive refers to a component or combination
>>> of components that is not capable of power gain. The resistors ladder VGA DAC
>>> that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
>>> that equally requires no configuration is an active component.
>>
>> If no one has any more objections within the next day, I'll pull in
>> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
>> "dumb-vga-dac".
>
> That works for me. You'll probably want to update the Kconfig and file
> name to match though.

Queued to drm-misc, with the changes suggested by you and Laurent.

Thanks,
Archit

>
> Maxime
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/07/2016 02:44 PM, Maxime Ripard wrote:
> On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
>>
>>
>> On 10/07/2016 02:34 AM, Laurent Pinchart wrote:
>>> Hi Sean,
>>>
>>> On Thursday 06 Oct 2016 15:53:28 Sean Paul wrote:
>>>> On Thu, Oct 6, 2016 at 1:27 PM, Laurent Pinchart wrote:
>>>>> On Thursday 06 Oct 2016 17:09:57 Archit Taneja wrote:
>>>>>> On 10/06/2016 12:51 PM, Maxime Ripard wrote:
>>>>>>> On Mon, Oct 03, 2016 at 04:40:57PM +0530, Archit Taneja wrote:
>>>>>>>> On 09/30/2016 08:07 PM, Maxime Ripard wrote:
>>>>>>>>> Some boards have an entirely passive RGB to VGA bridge, based on
>>>>>>>>> either DACs or resistor ladders.
>>>>>>>>>
>>>>>>>>> Those might or might not have an i2c bus routed to the VGA connector
>>>>>>>>> in order to access the screen EDIDs.
>>>>>>>>>
>>>>>>>>> Add a bridge that doesn't do anything but expose the modes available
>>>>>>>>> on the screen, either based on the EDIDs if available, or based on
>>>>>>>>> the XGA standards.
>>>>>>>>>
>>>>>>>>> Acked-by: Rob Herring <robh@kernel.org>
>>>>>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>>>>>>> ---
>>>>>>>>> .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  48 +++++
>>>>>>>>> drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>>>>>>>> drivers/gpu/drm/bridge/Makefile                    |   1 +
>>>>>>>>> drivers/gpu/drm/bridge/rgb-to-vga.c                | 229 +++++++++++++
>>>>>>>>> 4 files changed, 285 insertions(+)
>>>>>>>>> create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.tx
>>>>>>>>> t
>>>>>>>>> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> txt
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000000..a8375bc1f9cb
>>>>>>>>> --- /dev/null
>>>>>>>>> +++
>>>>>>>>> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.
>>>>>>>>> tx
>>>>>>>>> t @@ -0,0 +1,48 @@
>>>>>>>>> +Dumb RGB to VGA bridge
>>>>>>>>> +----------------------
>>>>>>>>> +
>>>>>>>>> +This binding is aimed for dumb RGB to VGA bridges that do not
>>>>>>>>> require
>>>>>>>>> +any configuration.
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +
>>>>>>>>> +- compatible: Must be "rgb-to-vga-bridge"
>>>>>>>>
>>>>>>>> I'd talked to Laurent on IRC if he's okay with this. And I guess you
>>>>>>>> to had discussed it during XDC too. He's suggested that it'd be better
>>>>>>>> to have the compatible string as "simple-vga-dac".
>>>>>>>
>>>>>>> I just wished this bikeshedding had taken place publicly and be
>>>>>>> actually part of that discussion, but yeah, ok.
>>>>>>
>>>>>> Sorry about that. I'd pinged him for an Ack, the discussion went
>>>>>> more than that :)
>>>>>>
>>>>>>>> Some of the reasons behind having this:
>>>>>>>>
>>>>>>>> - We don't need to specify "rgb" in the compatible string since most
>>>>>>>> simple VGA DACs can only work with an RGB input.
>>>>>>>
>>>>>>> Ok.
>>>>>>>
>>>>>>>> - Also, with "dac" specified in the string, we don't need to
>>>>>>>> specifically mention "bridge" in the string. Also, bridge is a drm
>>>>>>>> specific term.
>>>>>>>>
>>>>>>>> - "simple" is considered because it's an unconfigurable bridge, and it
>>>>>>>> might be misleading for other VGA DACs to not use "vga-dac".
>>>>>>>
>>>>>>> All those "simple" bindings are just the biggest lie we ever
>>>>>>> told. It's simple when you introduce it, and then grows into something
>>>>>>> much more complicated than a non-simple implementation.
>>>>>>
>>>>>> "simple" here is supposed to mean that it's an unconfigurable RGB to
>>>>>> VGA DAC. This isn't supposed to follow the simple-panel model, where
>>>>>> you add the "simple-panel" string in the compatible node, along with
>>>>>> you chip specific compatible string.
>>>>>
>>>>> I agree with Maxime, I don't like the word "simple". My preference would
>>>>> be "vga-dac" for a lack of a better qualifier than "simple" to describe
>>>>> the fact that the device requires no configuration. My only concern with
>>>>> "vga-dac" is that we would restrict usage of that compatible string for a
>>>>> subset of VGA DACs, with more complex devices not being compatible with
>>>>> "vga-dac" even though they are VGA DACs. That's a problem I can live with
>>>>> though.
>>>>
>>>> While we're bikeshedding (feel free to ignore my input on this), I
>>>> think Maxime's initial "dumb" qualifier was better than "simple".
>>>
>>> I think I agree.
>>>
>>>> I think "passive" also gets the point across better than "simple", which
>>>> we've already established as something else in drm.
>>>
>>> To my electrical engineer's ear, passive refers to a component or combination
>>> of components that is not capable of power gain. The resistors ladder VGA DAC
>>> that Maxime is trying to support is a passive system, but the ADV7123 VGA DAC
>>> that equally requires no configuration is an active component.
>>
>> If no one has any more objections within the next day, I'll pull in
>> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
>> "dumb-vga-dac".
>
> That works for me. You'll probably want to update the Kconfig and file
> name to match though.

Queued to drm-misc, with the changes suggested by you and Laurent.

Thanks,
Archit

>
> Maxime
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Archit,

On Monday 10 Oct 2016 11:05:10 Archit Taneja wrote:
> On 10/07/2016 02:44 PM, Maxime Ripard wrote:
> > On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:

[snip]

> >> If no one has any more objections within the next day, I'll pull in
> >> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
> >> "dumb-vga-dac".
> > 
> > That works for me. You'll probably want to update the Kconfig and file
> > name to match though.
> 
> Queued to drm-misc, with the changes suggested by you and Laurent.

Those changes would have been worth a repost. I've had a look at the patch 
you've committed and it looks OK to me, so no harm done (the commit message is 
a bit inaccurate, but it's not the end of the world). Could you please make 
sure you repost patches in the future when you change them in non-trivial ways 
?

-- 
Regards,

Laurent Pinchart

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

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Laurent Pinchart]

Hi Archit,

On Monday 10 Oct 2016 11:05:10 Archit Taneja wrote:
> On 10/07/2016 02:44 PM, Maxime Ripard wrote:
> > On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:

[snip]

> >> If no one has any more objections within the next day, I'll pull in
> >> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
> >> "dumb-vga-dac".
> > 
> > That works for me. You'll probably want to update the Kconfig and file
> > name to match though.
> 
> Queued to drm-misc, with the changes suggested by you and Laurent.

Those changes would have been worth a repost. I've had a look at the patch 
you've committed and it looks OK to me, so no harm done (the commit message is 
a bit inaccurate, but it's not the end of the world). Could you please make 
sure you repost patches in the future when you change them in non-trivial ways 
?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/10/2016 12:45 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Monday 10 Oct 2016 11:05:10 Archit Taneja wrote:
>> On 10/07/2016 02:44 PM, Maxime Ripard wrote:
>>> On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
>
> [snip]
>
>>>> If no one has any more objections within the next day, I'll pull in
>>>> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
>>>> "dumb-vga-dac".
>>>
>>> That works for me. You'll probably want to update the Kconfig and file
>>> name to match though.
>>
>> Queued to drm-misc, with the changes suggested by you and Laurent.
>
> Those changes would have been worth a repost. I've had a look at the patch
> you've committed and it looks OK to me, so no harm done (the commit message is
> a bit inaccurate, but it's not the end of the world). Could you please make
> sure you repost patches in the future when you change them in non-trivial ways
> ?

Sorry about that. Will repost from now onwards if the changes are too
significant.

Archit

>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/5] drm/bridge: Add RGB to VGA bridge support

[Archit Taneja]

On 10/10/2016 12:45 PM, Laurent Pinchart wrote:
> Hi Archit,
>
> On Monday 10 Oct 2016 11:05:10 Archit Taneja wrote:
>> On 10/07/2016 02:44 PM, Maxime Ripard wrote:
>>> On Fri, Oct 07, 2016 at 10:27:31AM +0530, Archit Taneja wrote:
>
> [snip]
>
>>>> If no one has any more objections within the next day, I'll pull in
>>>> Maxime's v5 RGB to VGA bridge driver, and change the compatible to
>>>> "dumb-vga-dac".
>>>
>>> That works for me. You'll probably want to update the Kconfig and file
>>> name to match though.
>>
>> Queued to drm-misc, with the changes suggested by you and Laurent.
>
> Those changes would have been worth a repost. I've had a look at the patch
> you've committed and it looks OK to me, so no harm done (the commit message is
> a bit inaccurate, but it's not the end of the world). Could you please make
> sure you repost patches in the future when you change them in non-trivial ways
> ?

Sorry about that. Will repost from now onwards if the changes are too
significant.

Archit

>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project