[PATCH 0/3] drm: Add LVDS decoder bridge

[PATCH 0/3] drm: Add LVDS decoder bridge

[Jacopo Mondi]

Hello,
   this small series add a driver for transparent LVDS decoders, as
Thine THC63LVD1024 device.

The R-Car Gen3 V3M Eagle board feature such a converter, that has been so far
not described as part of the display output pipeline. Now that a driver has
been added, the Eagle DTS file has been updated to include the transparent
decoder chip.

So far so good, but currently the current DRM Bridge API do not provide support
to query a mode from another bridge as it is possible to do on panel devices.
In our case the LVDS decoder is connected to the output of R-Car DU lvds encoder
(drivers/gpu/drm/rcar-du/rcar_lvds.c). As its 'mode_set' function shows, LVDS
modes cannot be propagated from bridge to another bridge, but are instead
inferred from the bus_format field of the panel's connector.

It is my intention to propose an API extension to allow formats to be propagated
through bridges, but knowing the DRM/KMS subsystem very superficially I would
appreciate any pointer from more experienced developers.

For Renesas side:
The series is based on Laurent's drm/next/du branch with patches on top for:
- Sergei: Enable PFC, I2c, GPIOs for r8a77970
- Sergei: Add support for r8a77970 in DU and add display device nodes in
  r8a77970 DTSI
- Niklas: Connect DU LVDS output to HDMI bridge adv7511w in Eagle DTS
- Sergei: fix video output on R8A77970

A base branch with these patches applied is available at
git://jmondi.org/linux v3m/v4.16-rc3/base

Tested on Eagle board, making sure DU probes and testing all available output
modes (of which only a few are actually working, I suspect due to faulty mode
propagation through DRM bridges).

Thanks
   j

Jacopo Mondi (3):
  dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
  drm: bridge: Add LVDS decoder driver
  arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

 .../bindings/display/bridge/thine,thc63lvd1024.txt |  59 +++++
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts     |  31 ++-
 drivers/gpu/drm/bridge/Kconfig                     |   8 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/lvds-decoder.c              | 239 +++++++++++++++++++++
 5 files changed, 336 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
 create mode 100644 drivers/gpu/drm/bridge/lvds-decoder.c

--
2.7.4

[PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Jacopo Mondi]

Document Thine THC63LVD1024 LVDS decoder.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../bindings/display/bridge/thine,thc63lvd1024.txt | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
new file mode 100644
index 0000000..53b6453
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
@@ -0,0 +1,59 @@
+THine Electronics THC63LVD1024 LVDS receiver
+--------------------------------------------
+
+The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
+to digital CMOS/TTL parallel data.
+
+Required properties:
+- compatible: Shall be one of the following:
+  "thine,thc63lvd1024",
+  "lvds-decoder"
+
+Optional properties:
+- supply-vcc: Power supply for TTL output and digital circuitry
+- supply-cvcc: Power supply for TTL CLOCKOUT signal
+- supply-lvcc: Power supply for LVDS inputs
+- supply-pvcc: Power supply for PLL circuitry
+- pwnd-gpio: Power down GPIO signal. Active low.
+- oe-gpio: Output enable GPIO signal. Active high.
+
+The THC63LVD1024 has two video ports, whose connections are modeled according
+to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
+
+- Port@0: LVDS input port
+- Port@1: Digital CMOS/TTL parallel output
+
+Example:
+-------
+
+	lvds_decoder: decoder-0 {
+		compatible = "thine,thc63lvd1024";
+
+		vcc-supply = <&reg_lvds_vcc>;
+		lvcc-supply = <&reg_lvds_lvcc>;
+
+		pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				lvds_dec_in: endpoint {
+					remote-endpoint = <&lvds_out>;
+				};
+			};
+
+			port@1{
+				reg = <1>;
+
+				lvds_dec_out: endpoint {
+					remote-endpoint = <&adv7511_in>;
+				};
+
+			};
+
+		};
+	};
-- 
2.7.4

[PATCH 2/3] drm: bridge: Add LVDS decoder driver

[Jacopo Mondi]

Add transparent LVDS decoder driver.

A transparent LVDS decoder is a DRM bridge device that does not require
any configuration and converts LVDS input to digital CMOS/TTL parallel
data output. The driver is compatible with Thine THC63LVD1024 device
that can control a few power supplies and a few enable GPIOs.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/gpu/drm/bridge/Kconfig        |   8 ++
 drivers/gpu/drm/bridge/Makefile       |   1 +
 drivers/gpu/drm/bridge/lvds-decoder.c | 239 ++++++++++++++++++++++++++++++++++
 3 files changed, 248 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/lvds-decoder.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 3b99d5a..e52a5af 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -32,6 +32,14 @@ config DRM_DUMB_VGA_DAC
 	help
 	  Support for RGB to VGA DAC based bridges
 
+config DRM_LVDS_DECODER
+	tristate "Transparent LVDS to parallel decoder support"
+	depends on OF
+	select DRM_PANEL_BRIDGE
+	help
+	  Support for transparent LVDS to parallel decoders that don't require
+	  any configuration.
+
 config DRM_LVDS_ENCODER
 	tristate "Transparent parallel to LVDS encoder support"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 373eb28..edc2332 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
 obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o
+obj-$(CONFIG_DRM_LVDS_DECODER) += lvds-decoder.o
 obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o
 obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/lvds-decoder.c b/drivers/gpu/drm/bridge/lvds-decoder.c
new file mode 100644
index 0000000..3aada7e
--- /dev/null
+++ b/drivers/gpu/drm/bridge/lvds-decoder.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LVDS to parallel data DRM bridge driver.
+ *
+ * Copyright (C) 2018 Jacopo Mondi <jacopo+renesas@jmondi.org>
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_panel.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+
+struct lvds_decoder {
+	struct device *dev;
+
+	struct regulator *vccs[4];
+
+	struct gpio_desc *pwdn;
+	struct gpio_desc *oe;
+
+	struct drm_bridge bridge;
+	struct drm_bridge *next;
+	struct drm_encoder *bridge_encoder;
+};
+
+static inline struct lvds_decoder *to_lvds_decoder(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct lvds_decoder, bridge);
+}
+
+static int lvds_decoder_attach(struct drm_bridge *bridge)
+{
+	struct lvds_decoder *lvds = to_lvds_decoder(bridge);
+
+	return drm_bridge_attach(bridge->encoder, lvds->next, bridge);
+}
+
+static void lvds_decoder_enable(struct drm_bridge *bridge)
+{
+	struct lvds_decoder *lvds = to_lvds_decoder(bridge);
+	struct regulator *vcc;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(lvds->vccs); i++) {
+		vcc = lvds->vccs[i];
+		if (vcc) {
+			ret = regulator_enable(vcc);
+			if (ret)
+				goto error_vcc_enable;
+		}
+	}
+
+	if (lvds->pwdn)
+		gpiod_set_value(lvds->pwdn, 0);
+
+	if (lvds->oe)
+		gpiod_set_value(lvds->oe, 1);
+
+	return;
+
+error_vcc_enable:
+	dev_err(lvds->dev, "Failed to enable regulator %u\n", i);
+}
+
+static void lvds_decoder_disable(struct drm_bridge *bridge)
+{
+	struct lvds_decoder *lvds = to_lvds_decoder(bridge);
+	struct regulator *vcc;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(lvds->vccs); i++) {
+		vcc = lvds->vccs[i];
+		if (vcc) {
+			ret = regulator_disable(vcc);
+			if (ret)
+				goto error_vcc_disable;
+		}
+	}
+
+	if (lvds->pwdn)
+		gpiod_set_value(lvds->pwdn, 1);
+
+	if (lvds->oe)
+		gpiod_set_value(lvds->oe, 0);
+
+	return;
+
+error_vcc_disable:
+	dev_err(lvds->dev, "Failed to enable regulator %u\n", i);
+}
+
+struct drm_bridge_funcs lvds_dec_bridge_func = {
+	.attach	= lvds_decoder_attach,
+	.enable = lvds_decoder_enable,
+	.disable = lvds_decoder_disable,
+};
+
+static int lvds_decoder_parse_dt(struct lvds_decoder *lvds)
+{
+	struct device_node *lvds_output;
+	struct device_node *remote;
+	int ret = 0;
+
+	lvds_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, -1);
+	if (!lvds_output) {
+		dev_err(lvds->dev, "Missing endpoint in Port@1\n");
+		return -ENODEV;
+	}
+
+	remote = of_graph_get_remote_port_parent(lvds_output);
+	if (!remote) {
+		dev_err(lvds->dev, "Endpoint in Port@1 unconnected\n");
+		ret = -ENODEV;
+		goto error_put_lvds_node;
+	}
+
+	if (!of_device_is_available(remote)) {
+		dev_err(lvds->dev, "Port@1 remote endpoint is disabled\n");
+		ret = -ENODEV;
+		goto error_put_remote_node;
+	}
+
+	lvds->next = of_drm_find_bridge(remote);
+	if (!lvds->next)
+		ret = -EPROBE_DEFER;
+
+error_put_remote_node:
+	of_node_put(remote);
+error_put_lvds_node:
+	of_node_put(lvds_output);
+
+	return ret;
+}
+
+static int lvds_decoder_gpio_init(struct lvds_decoder *lvds)
+{
+	lvds->pwdn = devm_gpiod_get_optional(lvds->dev, "pwdn", GPIOD_OUT_LOW);
+	if (IS_ERR(lvds->pwdn)) {
+		dev_err(lvds->dev, "Unable to get GPIO \"pwdn\"\n");
+		return PTR_ERR(lvds->pwdn);
+	}
+
+	lvds->oe = devm_gpiod_get_optional(lvds->dev, "oe", GPIOD_OUT_LOW);
+	if (IS_ERR(lvds->oe)) {
+		dev_err(lvds->dev, "Unable to get GPIO \"oe\"\n");
+		return PTR_ERR(lvds->oe);
+	}
+
+	return 0;
+}
+
+static int lvds_decoder_regulator_init(struct lvds_decoder *lvds)
+{
+	const char *reg_names[4] = { "vcc", "lvcc", "pvcc", "cvcc", };
+	struct regulator **reg;
+	int i;
+
+	for (i = 0, reg = &lvds->vccs[0];
+	     i < ARRAY_SIZE(lvds->vccs); i++, reg++) {
+		*reg = devm_regulator_get_optional(lvds->dev, reg_names[i]);
+		if (IS_ERR(*reg)) {
+			if (PTR_ERR(*reg) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+			*reg = NULL;
+		}
+	}
+
+	return 0;
+}
+
+static int lvds_decoder_probe(struct platform_device *pdev)
+{
+	struct lvds_decoder *lvds;
+	int ret;
+
+	lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
+	if (!lvds)
+		return -ENOMEM;
+
+	lvds->dev = &pdev->dev;
+	platform_set_drvdata(pdev, lvds);
+
+	ret = lvds_decoder_regulator_init(lvds);
+	if (ret)
+		return ret;
+
+	ret = lvds_decoder_gpio_init(lvds);
+	if (ret)
+		return ret;
+
+	ret = lvds_decoder_parse_dt(lvds);
+	if (ret)
+		return ret;
+
+	lvds->bridge.driver_private = lvds;
+	lvds->bridge.of_node = pdev->dev.of_node;
+	lvds->bridge.funcs = &lvds_dec_bridge_func;
+
+	drm_bridge_add(&lvds->bridge);
+
+	return 0;
+}
+
+static int lvds_decoder_remove(struct platform_device *pdev)
+{
+	struct lvds_decoder *lvds = platform_get_drvdata(pdev);
+
+	drm_bridge_remove(&lvds->bridge);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id lvds_decoder_match[] = {
+	{ .compatible = "thine,thc63lvd1024", },
+	{ .compatible = "lvds-decoder", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lvds_decoder_match);
+#endif
+
+static struct platform_driver lvds_decoder_driver = {
+	.probe	= lvds_decoder_probe,
+	.remove	= lvds_decoder_remove,
+	.driver	= {
+		.name		= "lvds_decoder",
+		.of_match_table	= lvds_decoder_match,
+	},
+};
+module_platform_driver(lvds_decoder_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo@jmondi.org>");
+MODULE_DESCRIPTION("Transparent LVDS to parallel data decoder");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

[Jacopo Mondi]

The R-Car V3M Eagle board includes a transparent LVDS decoder, connected
to the on-chip LVDS encoder output on one side and to HDMI encoder
ADV7511w on the other one.

As the decoder does not need any configuration it has been so-far
omitted from DTS. Now that a driver for transparent LVDS decoder is
available, describe it in DT as well.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 31 ++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
index c0fd144..0d62b40 100644
--- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
@@ -42,6 +42,33 @@
 			};
 		};
 	};
+
+	lvds_decoder {
+		compatible = "thine,thc63lvd1024";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				lvds_dec_in: endpoint {
+					remote-endpoint = <&lvds0_out>;
+				};
+			};
+
+			port@1{
+				reg = <1>;
+
+				lvds_dec_out: endpoint {
+					remote-endpoint = <&adv7511_in>;
+				};
+
+			};
+
+		};
+	};
 };
 
 &avb {
@@ -98,7 +125,7 @@
 			port@0 {
 				reg = <0>;
 				adv7511_in: endpoint {
-					remote-endpoint = <&lvds0_out>;
+					remote-endpoint = <&lvds_dec_out>;
 				};
 			};
 
@@ -153,7 +180,7 @@
 	ports {
 		port@1 {
 			endpoint {
-				remote-endpoint = <&adv7511_in>;
+				remote-endpoint = <&lvds_dec_in>;
 			};
 		};
 	};
-- 
2.7.4

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Andrzej Hajda]

On 08.03.2018 16:24, Jacopo Mondi wrote:
> Document Thine THC63LVD1024 LVDS decoder.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../bindings/display/bridge/thine,thc63lvd1024.txt | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> new file mode 100644
> index 0000000..53b6453
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,59 @@
> +THine Electronics THC63LVD1024 LVDS receiver
> +--------------------------------------------
> +
> +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> +to digital CMOS/TTL parallel data.

You say multiple streams, but bindings describe only one stream.
> +
> +Required properties:
> +- compatible: Shall be one of the following:
> +  "thine,thc63lvd1024",
> +  "lvds-decoder"
> +
> +Optional properties:
> +- supply-vcc: Power supply for TTL output and digital circuitry
> +- supply-cvcc: Power supply for TTL CLOCKOUT signal
> +- supply-lvcc: Power supply for LVDS inputs
> +- supply-pvcc: Power supply for PLL circuitry
> +- pwnd-gpio: Power down GPIO signal. Active low.

Specs [1] uses "/PDWN" name for the pin, moreover gpios suffix is preferred.

Another issue I see is two possibly contradicting conventions:
1. Properties should be named according to specs - so here it should be
named pdwn-gpios.
2. The bindings tries to be generic for lvds decoders, in such case
probably preferred name should be more generic, maybe power-gpios.

Personally I would prefer 1, in such case generic lvds-decoder driver
should look for gpio names according to compatible string.

[1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf

> +- oe-gpio: Output enable GPIO signal. Active high.

oe-gpios

> +
> +The THC63LVD1024 has two video ports, whose connections are modeled according
> +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
> +
> +- Port@0: LVDS input port
> +- Port@1: Digital CMOS/TTL parallel output

According to specs it has two lvds input and two parallel output ports,
maybe it would be good to describe all here.

Regards
Andrzej

> +
> +Example:
> +-------
> +
> +	lvds_decoder: decoder-0 {
> +		compatible = "thine,thc63lvd1024";
> +
> +		vcc-supply = <&reg_lvds_vcc>;
> +		lvcc-supply = <&reg_lvds_lvcc>;
> +
> +		pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				lvds_dec_in: endpoint {
> +					remote-endpoint = <&lvds_out>;
> +				};
> +			};
> +
> +			port@1{
> +				reg = <1>;
> +
> +				lvds_dec_out: endpoint {
> +					remote-endpoint = <&adv7511_in>;
> +				};
> +
> +			};
> +
> +		};
> +	};

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Andrzej Hajda]

On 08.03.2018 16:24, Jacopo Mondi wrote:
> Document Thine THC63LVD1024 LVDS decoder.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../bindings/display/bridge/thine,thc63lvd1024.txt | 59 ++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> new file mode 100644
> index 0000000..53b6453
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,59 @@
> +THine Electronics THC63LVD1024 LVDS receiver
> +--------------------------------------------
> +
> +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> +to digital CMOS/TTL parallel data.

You say multiple streams, but bindings describe only one stream.
> +
> +Required properties:
> +- compatible: Shall be one of the following:
> +  "thine,thc63lvd1024",
> +  "lvds-decoder"
> +
> +Optional properties:
> +- supply-vcc: Power supply for TTL output and digital circuitry
> +- supply-cvcc: Power supply for TTL CLOCKOUT signal
> +- supply-lvcc: Power supply for LVDS inputs
> +- supply-pvcc: Power supply for PLL circuitry
> +- pwnd-gpio: Power down GPIO signal. Active low.

Specs [1] uses "/PDWN" name for the pin, moreover gpios suffix is preferred.

Another issue I see is two possibly contradicting conventions:
1. Properties should be named according to specs - so here it should be
named pdwn-gpios.
2. The bindings tries to be generic for lvds decoders, in such case
probably preferred name should be more generic, maybe power-gpios.

Personally I would prefer 1, in such case generic lvds-decoder driver
should look for gpio names according to compatible string.

[1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf

> +- oe-gpio: Output enable GPIO signal. Active high.

oe-gpios

> +
> +The THC63LVD1024 has two video ports, whose connections are modeled according
> +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
> +
> +- Port@0: LVDS input port
> +- Port@1: Digital CMOS/TTL parallel output

According to specs it has two lvds input and two parallel output ports,
maybe it would be good to describe all here.

Regards
Andrzej

> +
> +Example:
> +-------
> +
> +	lvds_decoder: decoder-0 {
> +		compatible = "thine,thc63lvd1024";
> +
> +		vcc-supply = <&reg_lvds_vcc>;
> +		lvcc-supply = <&reg_lvds_lvcc>;
> +
> +		pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				lvds_dec_in: endpoint {
> +					remote-endpoint = <&lvds_out>;
> +				};
> +			};
> +
> +			port@1{
> +				reg = <1>;
> +
> +				lvds_dec_out: endpoint {
> +					remote-endpoint = <&adv7511_in>;
> +				};
> +
> +			};
> +
> +		};
> +	};


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

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Document Thine THC63LVD1024 LVDS decoder.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,59 @@
> +THine Electronics THC63LVD1024 LVDS receiver

Thine

> +--------------------------------------------
> +
> +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> +to digital CMOS/TTL parallel data.
> +
> +Required properties:
> +- compatible: Shall be one of the following:
> +  "thine,thc63lvd1024",
> +  "lvds-decoder"

What's the purpose of the second compatible value?
When should it be used?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Document Thine THC63LVD1024 LVDS decoder.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -0,0 +1,59 @@
> +THine Electronics THC63LVD1024 LVDS receiver

Thine

> +--------------------------------------------
> +
> +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> +to digital CMOS/TTL parallel data.
> +
> +Required properties:
> +- compatible: Shall be one of the following:
> +  "thine,thc63lvd1024",
> +  "lvds-decoder"

What's the purpose of the second compatible value?
When should it be used?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

[Sergei Shtylyov]

Hello!

On 3/8/2018 6:24 PM, Jacopo Mondi wrote:

> The R-Car V3M Eagle board includes a transparent LVDS decoder, connected
> to the on-chip LVDS encoder output on one side and to HDMI encoder
> ADV7511w on the other one.
> 
> As the decoder does not need any configuration it has been so-far
> omitted from DTS. Now that a driver for transparent LVDS decoder is
> available, describe it in DT as well.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>   arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 31 ++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> index c0fd144..0d62b40 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> @@ -42,6 +42,33 @@
>   			};
>   		};
>   	};
> +
> +	lvds_decoder {

    Hyphens are preferred to underscored in the node names.

> +		compatible = "thine,thc63lvd1024";
[...]

MBR, Sergei

Re: [PATCH 3/3] arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle

[Sergei Shtylyov]

Hello!

On 3/8/2018 6:24 PM, Jacopo Mondi wrote:

> The R-Car V3M Eagle board includes a transparent LVDS decoder, connected
> to the on-chip LVDS encoder output on one side and to HDMI encoder
> ADV7511w on the other one.
> 
> As the decoder does not need any configuration it has been so-far
> omitted from DTS. Now that a driver for transparent LVDS decoder is
> available, describe it in DT as well.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>   arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 31 ++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> index c0fd144..0d62b40 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts
> @@ -42,6 +42,33 @@
>   			};
>   		};
>   	};
> +
> +	lvds_decoder {

    Hyphens are preferred to underscored in the node names.

> +		compatible = "thine,thc63lvd1024";
[...]

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

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Andrzej,

On Fri, Mar 09, 2018 at 09:01:24AM +0100, Andrzej Hajda wrote:
> On 08.03.2018 16:24, Jacopo Mondi wrote:
> > Document Thine THC63LVD1024 LVDS decoder.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../bindings/display/bridge/thine,thc63lvd1024.txt | 59 ++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > new file mode 100644
> > index 0000000..53b6453
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,59 @@
> > +THine Electronics THC63LVD1024 LVDS receiver
> > +--------------------------------------------
> > +
> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> > +to digital CMOS/TTL parallel data.
>
>You say multiple streams, but bindings describe only one stream.

I'm always confused by the fact that "bindings should describe
hardware" even when the driver does not support some features the
hardware provides.

In this case, the driver and its bindigns does not expose "MODE1/2"
pins that are used to control single/double stream mode, assuming they
are hard-wired and single/double stream mode is not controllable by
the SoC.

I should have reserved two more ports for one (optional) additional input and
one (optional) additional output, as chip can be configured to work in
that mode even if MODE1/2 are not hardwired.

Will add them in v2.

> > +
> > +Required properties:
> > +- compatible: Shall be one of the following:
> > +  "thine,thc63lvd1024",
> > +  "lvds-decoder"
> > +
> > +Optional properties:
> > +- supply-vcc: Power supply for TTL output and digital circuitry
> > +- supply-cvcc: Power supply for TTL CLOCKOUT signal
> > +- supply-lvcc: Power supply for LVDS inputs
> > +- supply-pvcc: Power supply for PLL circuitry
> > +- pwnd-gpio: Power down GPIO signal. Active low.
>
> Specs [1] uses "/PDWN" name for the pin, moreover gpios suffix is preferred.
>
> Another issue I see is two possibly contradicting conventions:
> 1. Properties should be named according to specs - so here it should be
> named pdwn-gpios.
> 2. The bindings tries to be generic for lvds decoders, in such case
> probably preferred name should be more generic, maybe power-gpios.
>
> Personally I would prefer 1, in such case generic lvds-decoder driver
> should look for gpio names according to compatible string.
>

I will go for 1 and associate the power control gpio name to the
matched compatible string.

"thine,thc63lvd1024" will look for "pwnd-gpios"
"lvds,decoder" will look for "power-gpios"

> [1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf
>
> > +- oe-gpio: Output enable GPIO signal. Active high.
>
> oe-gpios
>
> > +
> > +The THC63LVD1024 has two video ports, whose connections are modeled according
> > +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
> > +
> > +- Port@0: LVDS input port
> > +- Port@1: Digital CMOS/TTL parallel output
>
> According to specs it has two lvds input and two parallel output ports,
> maybe it would be good to describe all here.

I will in v2.

Thanks
   j

>
> Regards
> Andrzej
>
> > +
> > +Example:
> > +-------
> > +
> > +	lvds_decoder: decoder-0 {
> > +		compatible = "thine,thc63lvd1024";
> > +
> > +		vcc-supply = <&reg_lvds_vcc>;
> > +		lvcc-supply = <&reg_lvds_lvcc>;
> > +
> > +		pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +
> > +				lvds_dec_in: endpoint {
> > +					remote-endpoint = <&lvds_out>;
> > +				};
> > +			};
> > +
> > +			port@1{
> > +				reg = <1>;
> > +
> > +				lvds_dec_out: endpoint {
> > +					remote-endpoint = <&adv7511_in>;
> > +				};
> > +
> > +			};
> > +
> > +		};
> > +	};
>
>

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Andrzej,

On Fri, Mar 09, 2018 at 09:01:24AM +0100, Andrzej Hajda wrote:
> On 08.03.2018 16:24, Jacopo Mondi wrote:
> > Document Thine THC63LVD1024 LVDS decoder.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../bindings/display/bridge/thine,thc63lvd1024.txt | 59 ++++++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > new file mode 100644
> > index 0000000..53b6453
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,59 @@
> > +THine Electronics THC63LVD1024 LVDS receiver
> > +--------------------------------------------
> > +
> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> > +to digital CMOS/TTL parallel data.
>
>You say multiple streams, but bindings describe only one stream.

I'm always confused by the fact that "bindings should describe
hardware" even when the driver does not support some features the
hardware provides.

In this case, the driver and its bindigns does not expose "MODE1/2"
pins that are used to control single/double stream mode, assuming they
are hard-wired and single/double stream mode is not controllable by
the SoC.

I should have reserved two more ports for one (optional) additional input and
one (optional) additional output, as chip can be configured to work in
that mode even if MODE1/2 are not hardwired.

Will add them in v2.

> > +
> > +Required properties:
> > +- compatible: Shall be one of the following:
> > +  "thine,thc63lvd1024",
> > +  "lvds-decoder"
> > +
> > +Optional properties:
> > +- supply-vcc: Power supply for TTL output and digital circuitry
> > +- supply-cvcc: Power supply for TTL CLOCKOUT signal
> > +- supply-lvcc: Power supply for LVDS inputs
> > +- supply-pvcc: Power supply for PLL circuitry
> > +- pwnd-gpio: Power down GPIO signal. Active low.
>
> Specs [1] uses "/PDWN" name for the pin, moreover gpios suffix is preferred.
>
> Another issue I see is two possibly contradicting conventions:
> 1. Properties should be named according to specs - so here it should be
> named pdwn-gpios.
> 2. The bindings tries to be generic for lvds decoders, in such case
> probably preferred name should be more generic, maybe power-gpios.
>
> Personally I would prefer 1, in such case generic lvds-decoder driver
> should look for gpio names according to compatible string.
>

I will go for 1 and associate the power control gpio name to the
matched compatible string.

"thine,thc63lvd1024" will look for "pwnd-gpios"
"lvds,decoder" will look for "power-gpios"

> [1]: http://www.thine.co.jp/files/topics/179_ext_12_0.pdf
>
> > +- oe-gpio: Output enable GPIO signal. Active high.
>
> oe-gpios
>
> > +
> > +The THC63LVD1024 has two video ports, whose connections are modeled according
> > +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
> > +
> > +- Port@0: LVDS input port
> > +- Port@1: Digital CMOS/TTL parallel output
>
> According to specs it has two lvds input and two parallel output ports,
> maybe it would be good to describe all here.

I will in v2.

Thanks
   j

>
> Regards
> Andrzej
>
> > +
> > +Example:
> > +-------
> > +
> > +	lvds_decoder: decoder-0 {
> > +		compatible = "thine,thc63lvd1024";
> > +
> > +		vcc-supply = <&reg_lvds_vcc>;
> > +		lvcc-supply = <&reg_lvds_lvcc>;
> > +
> > +		pwdn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +
> > +				lvds_dec_in: endpoint {
> > +					remote-endpoint = <&lvds_out>;
> > +				};
> > +			};
> > +
> > +			port@1{
> > +				reg = <1>;
> > +
> > +				lvds_dec_out: endpoint {
> > +					remote-endpoint = <&adv7511_in>;
> > +				};
> > +
> > +			};
> > +
> > +		};
> > +	};
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Geert,
   thanks for review

On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > Document Thine THC63LVD1024 LVDS decoder.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,59 @@
> > +THine Electronics THC63LVD1024 LVDS receiver
>
> Thine
>
> > +--------------------------------------------
> > +
> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> > +to digital CMOS/TTL parallel data.
> > +
> > +Required properties:
> > +- compatible: Shall be one of the following:
> > +  "thine,thc63lvd1024",
> > +  "lvds-decoder"
>
> What's the purpose of the second compatible value?
> When should it be used?

It is probably my bad having started with a generic LVDS decoder in
mind and having then added properties specific to THC63LVD1024 to the
driver and its bindings.

"lvds,decoder" can be used when the chip is completely transparent to
the SoC and none of the optional properties I have described in the
bindings are specified (a generic "power-gpios" apart, see Andrzej
comments on "pwdn-gpios" property).

Also, I should make the driver behavior depend on the matched compatible
string. When "lvds-decoder" is matched, it will just look for an
optional power down gpio, when "thc63lvd1024" is matched, all of its
Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
eventually used in enable/disable routines.

I'm just not sure how to describe that in bindings. Would something
like the following work?

Optional properties for "lvds,decoder"
- power-gpios: Power control GPIOs

Optional properties for "thine,thc63lvd1024"
- pwdn-gpios: ...
- oe-gpios: ...
- supply-vcc: ...
- supply-cvcc: ...
- supply-pvcc: ...
- supply-lvcc: ...

Thanks
   j

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Geert,
   thanks for review

On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > Document Thine THC63LVD1024 LVDS decoder.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,59 @@
> > +THine Electronics THC63LVD1024 LVDS receiver
>
> Thine
>
> > +--------------------------------------------
> > +
> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> > +to digital CMOS/TTL parallel data.
> > +
> > +Required properties:
> > +- compatible: Shall be one of the following:
> > +  "thine,thc63lvd1024",
> > +  "lvds-decoder"
>
> What's the purpose of the second compatible value?
> When should it be used?

It is probably my bad having started with a generic LVDS decoder in
mind and having then added properties specific to THC63LVD1024 to the
driver and its bindings.

"lvds,decoder" can be used when the chip is completely transparent to
the SoC and none of the optional properties I have described in the
bindings are specified (a generic "power-gpios" apart, see Andrzej
comments on "pwdn-gpios" property).

Also, I should make the driver behavior depend on the matched compatible
string. When "lvds-decoder" is matched, it will just look for an
optional power down gpio, when "thc63lvd1024" is matched, all of its
Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
eventually used in enable/disable routines.

I'm just not sure how to describe that in bindings. Would something
like the following work?

Optional properties for "lvds,decoder"
- power-gpios: Power control GPIOs

Optional properties for "thine,thc63lvd1024"
- pwdn-gpios: ...
- oe-gpios: ...
- supply-vcc: ...
- supply-cvcc: ...
- supply-pvcc: ...
- supply-lvcc: ...

Thanks
   j

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, Mar 9, 2018 at 10:04 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>> > Document Thine THC63LVD1024 LVDS decoder.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>> > @@ -0,0 +1,59 @@
>> > +THine Electronics THC63LVD1024 LVDS receiver
>>
>> Thine
>>
>> > +--------------------------------------------
>> > +
>> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
>> > +to digital CMOS/TTL parallel data.
>> > +
>> > +Required properties:
>> > +- compatible: Shall be one of the following:
>> > +  "thine,thc63lvd1024",
>> > +  "lvds-decoder"
>>
>> What's the purpose of the second compatible value?
>> When should it be used?
>
> It is probably my bad having started with a generic LVDS decoder in
> mind and having then added properties specific to THC63LVD1024 to the
> driver and its bindings.
>
> "lvds,decoder" can be used when the chip is completely transparent to
> the SoC and none of the optional properties I have described in the
> bindings are specified (a generic "power-gpios" apart, see Andrzej
> comments on "pwdn-gpios" property).
>
> Also, I should make the driver behavior depend on the matched compatible
> string. When "lvds-decoder" is matched, it will just look for an
> optional power down gpio, when "thc63lvd1024" is matched, all of its
> Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
> eventually used in enable/disable routines.
>
> I'm just not sure how to describe that in bindings. Would something
> like the following work?
>
> Optional properties for "lvds,decoder"

"lvds-decoder"?

> - power-gpios: Power control GPIOs
>
> Optional properties for "thine,thc63lvd1024"
> - pwdn-gpios: ...
> - oe-gpios: ...
> - supply-vcc: ...
> - supply-cvcc: ...
> - supply-pvcc: ...
> - supply-lvcc: ...

Sounds like you need a (separate) generic lvds-decoder DT bindings document,
which you can extend/refer to from the THC63LVD1024-specific bindings.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, Mar 9, 2018 at 10:04 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>> > Document Thine THC63LVD1024 LVDS decoder.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>> > @@ -0,0 +1,59 @@
>> > +THine Electronics THC63LVD1024 LVDS receiver
>>
>> Thine
>>
>> > +--------------------------------------------
>> > +
>> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
>> > +to digital CMOS/TTL parallel data.
>> > +
>> > +Required properties:
>> > +- compatible: Shall be one of the following:
>> > +  "thine,thc63lvd1024",
>> > +  "lvds-decoder"
>>
>> What's the purpose of the second compatible value?
>> When should it be used?
>
> It is probably my bad having started with a generic LVDS decoder in
> mind and having then added properties specific to THC63LVD1024 to the
> driver and its bindings.
>
> "lvds,decoder" can be used when the chip is completely transparent to
> the SoC and none of the optional properties I have described in the
> bindings are specified (a generic "power-gpios" apart, see Andrzej
> comments on "pwdn-gpios" property).
>
> Also, I should make the driver behavior depend on the matched compatible
> string. When "lvds-decoder" is matched, it will just look for an
> optional power down gpio, when "thc63lvd1024" is matched, all of its
> Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
> eventually used in enable/disable routines.
>
> I'm just not sure how to describe that in bindings. Would something
> like the following work?
>
> Optional properties for "lvds,decoder"

"lvds-decoder"?

> - power-gpios: Power control GPIOs
>
> Optional properties for "thine,thc63lvd1024"
> - pwdn-gpios: ...
> - oe-gpios: ...
> - supply-vcc: ...
> - supply-cvcc: ...
> - supply-pvcc: ...
> - supply-lvcc: ...

Sounds like you need a (separate) generic lvds-decoder DT bindings document,
which you can extend/refer to from the THC63LVD1024-specific bindings.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Geert,

On Fri, Mar 09, 2018 at 10:22:39AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Fri, Mar 9, 2018 at 10:04 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> > On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
> >> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> >> > Document Thine THC63LVD1024 LVDS decoder.
> >> >
> >> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>
> >> Thanks for your patch!
> >>
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >> > @@ -0,0 +1,59 @@
> >> > +THine Electronics THC63LVD1024 LVDS receiver
> >>
> >> Thine
> >>
> >> > +--------------------------------------------
> >> > +
> >> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> >> > +to digital CMOS/TTL parallel data.
> >> > +
> >> > +Required properties:
> >> > +- compatible: Shall be one of the following:
> >> > +  "thine,thc63lvd1024",
> >> > +  "lvds-decoder"
> >>
> >> What's the purpose of the second compatible value?
> >> When should it be used?
> >
> > It is probably my bad having started with a generic LVDS decoder in
> > mind and having then added properties specific to THC63LVD1024 to the
> > driver and its bindings.
> >
> > "lvds,decoder" can be used when the chip is completely transparent to
> > the SoC and none of the optional properties I have described in the
> > bindings are specified (a generic "power-gpios" apart, see Andrzej
> > comments on "pwdn-gpios" property).
> >
> > Also, I should make the driver behavior depend on the matched compatible
> > string. When "lvds-decoder" is matched, it will just look for an
> > optional power down gpio, when "thc63lvd1024" is matched, all of its
> > Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
> > eventually used in enable/disable routines.
> >
> > I'm just not sure how to describe that in bindings. Would something
> > like the following work?
> >
> > Optional properties for "lvds,decoder"
>
> "lvds-decoder"?
>

Yes, sorry

> > - power-gpios: Power control GPIOs
> >
> > Optional properties for "thine,thc63lvd1024"
> > - pwdn-gpios: ...
> > - oe-gpios: ...
> > - supply-vcc: ...
> > - supply-cvcc: ...
> > - supply-pvcc: ...
> > - supply-lvcc: ...
>
> Sounds like you need a (separate) generic lvds-decoder DT bindings document,
> which you can extend/refer to from the THC63LVD1024-specific bindings.

Ok, I'll go with two bindings document then and see how it looks.

Thanks
   j



>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[jacopo mondi]

Hi Geert,

On Fri, Mar 09, 2018 at 10:22:39AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Fri, Mar 9, 2018 at 10:04 AM, jacopo mondi <jacopo@jmondi.org> wrote:
> > On Fri, Mar 09, 2018 at 09:10:55AM +0100, Geert Uytterhoeven wrote:
> >> On Thu, Mar 8, 2018 at 4:24 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> >> > Document Thine THC63LVD1024 LVDS decoder.
> >> >
> >> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>
> >> Thanks for your patch!
> >>
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> >> > @@ -0,0 +1,59 @@
> >> > +THine Electronics THC63LVD1024 LVDS receiver
> >>
> >> Thine
> >>
> >> > +--------------------------------------------
> >> > +
> >> > +The THC63LVD1024 is an LVDS receiver designed to convert multiple LVDS streams
> >> > +to digital CMOS/TTL parallel data.
> >> > +
> >> > +Required properties:
> >> > +- compatible: Shall be one of the following:
> >> > +  "thine,thc63lvd1024",
> >> > +  "lvds-decoder"
> >>
> >> What's the purpose of the second compatible value?
> >> When should it be used?
> >
> > It is probably my bad having started with a generic LVDS decoder in
> > mind and having then added properties specific to THC63LVD1024 to the
> > driver and its bindings.
> >
> > "lvds,decoder" can be used when the chip is completely transparent to
> > the SoC and none of the optional properties I have described in the
> > bindings are specified (a generic "power-gpios" apart, see Andrzej
> > comments on "pwdn-gpios" property).
> >
> > Also, I should make the driver behavior depend on the matched compatible
> > string. When "lvds-decoder" is matched, it will just look for an
> > optional power down gpio, when "thc63lvd1024" is matched, all of its
> > Vcc supplies, pwdn gpio and oe gpios will be queried and, if present,
> > eventually used in enable/disable routines.
> >
> > I'm just not sure how to describe that in bindings. Would something
> > like the following work?
> >
> > Optional properties for "lvds,decoder"
>
> "lvds-decoder"?
>

Yes, sorry

> > - power-gpios: Power control GPIOs
> >
> > Optional properties for "thine,thc63lvd1024"
> > - pwdn-gpios: ...
> > - oe-gpios: ...
> > - supply-vcc: ...
> > - supply-cvcc: ...
> > - supply-pvcc: ...
> > - supply-lvcc: ...
>
> Sounds like you need a (separate) generic lvds-decoder DT bindings document,
> which you can extend/refer to from the THC63LVD1024-specific bindings.

Ok, I'll go with two bindings document then and see how it looks.

Thanks
   j



>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel