Hello, this new version fixes comments received from Andrzej and Sergei on the preceding v3.
Mostly, I cleaned up the mess with the powerdown GPIO names, and I'm now using pdwn everywhere. The regulator enabling routine has been improved as suggested by Sergei and Andrzej and I've changed Kconfig symbol documentation for consistency with other ones. Also, fix gpio initialization to comply with the APIs that work on logical level even at gpio request time.
Branch for testing available at git://jmondi.org v3m/v4.16-rc3/lvds-bridge-v4
Thanks j
v3 -> v4: - Rename permutations of "pdwn" to just "pdwn" everywhere in the series - Improve power enable/disable routines as suggested by Andrzej and Sergei - Change "pdwn" gpio initialization to use the logical output level - Change Kconfig description
v2 -> v3: - Drop support for "lvds-decoder" and make the driver THC63LVD1024 specific -- Rework bindings to describe multiple input/output ports -- Rename driver and remove "lvds-decoder" references -- Rework Eagle DTS to use new bindings
v1 -> v2: - Drop support for THC63LVD1024
Jacopo Mondi (3): dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder drm: bridge: Add thc63lvd1024 LVDS decoder driver arm64: dts: renesas: Add LVDS decoder to R-Car V3M Eagle
.../bindings/display/bridge/thine,thc63lvd1024.txt | 63 +++++ arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 33 ++- drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 255 +++++++++++++++++++++ 5 files changed, 355 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
-- 2.7.4
Document Thine THC63LVD1024 LVDS decoder device tree bindings.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org --- .../bindings/display/bridge/thine,thc63lvd1024.txt | 63 ++++++++++++++++++++++ 1 file changed, 63 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..0936bde --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -0,0 +1,63 @@ +Thine Electronics THC63LVD1024 LVDS decoder +------------------------------------------- + +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS streams +to parallel data outputs. The chip supports single/dual input/output modes, +handling up to two two input LVDS stream and up to two digital CMOS/TTL outputs. + +Required properties: +- compatible: Shall be "thine,thc63lvd1024", + +Optional properties: +- vcc-supply: Power supply for TTL output and digital circuitry +- cvcc-supply: Power supply for TTL CLOCKOUT signal +- lvcc-supply: Power supply for LVDS inputs +- pvcc-supply: Power supply for PLL circuitry +- pdwn-gpios: Power down GPIO signal. Active low. +- oe-gpios: Output enable GPIO signal. Active high. + +The THC63LVD1024 video port connections are modeled according +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt + +Required video port nodes: +- Port@0: First LVDS input port +- Port@2: First digital CMOS/TTL parallel output + +Optional video port nodes: +- Port@1: Second LVDS input port +- Port@3: Second digital CMOS/TTL parallel output + +Example: +-------- + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + vcc-supply = <®_lvds_vcc>; + lvcc-supply = <®_lvds_lvcc>; + + pdwn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lvds_dec_in_0: endpoint { + remote-endpoint = <&lvds_out>; + }; + }; + + port@2{ + reg = <2>; + + lvds_dec_out_2: endpoint { + remote-endpoint = <&adv7511_in>; + }; + + }; + + }; + };
On 15.03.2018 11:56, Jacopo Mondi wrote:
Document Thine THC63LVD1024 LVDS decoder device tree bindings.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
.../bindings/display/bridge/thine,thc63lvd1024.txt | 63 ++++++++++++++++++++++ 1 file changed, 63 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..0936bde --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -0,0 +1,63 @@ +Thine Electronics THC63LVD1024 LVDS decoder +-------------------------------------------
+The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS streams +to parallel data outputs. The chip supports single/dual input/output modes, +handling up to two two input LVDS stream and up to two digital CMOS/TTL outputs.
+Required properties: +- compatible: Shall be "thine,thc63lvd1024",
+Optional properties: +- vcc-supply: Power supply for TTL output and digital circuitry +- cvcc-supply: Power supply for TTL CLOCKOUT signal +- lvcc-supply: Power supply for LVDS inputs +- pvcc-supply: Power supply for PLL circuitry +- pdwn-gpios: Power down GPIO signal. Active low. +- oe-gpios: Output enable GPIO signal. Active high.
Nitpick: different lines ends differently: period, dot, nothing. Another thing if there will be next iteration it would be good to emphasize converter has no control bus. Anyway:
Reviewed-by: Andrzej Hajda a.hajda@samsung.com
-- Regards Andrzej
+The THC63LVD1024 video port connections are modeled according +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt
+Required video port nodes: +- Port@0: First LVDS input port +- Port@2: First digital CMOS/TTL parallel output
+Optional video port nodes: +- Port@1: Second LVDS input port +- Port@3: Second digital CMOS/TTL parallel output
+Example: +--------
- thc63lvd1024: lvds-decoder {
compatible = "thine,thc63lvd1024";vcc-supply = <®_lvds_vcc>;lvcc-supply = <®_lvds_lvcc>;pdwn-gpio = <&gpio4 15 GPIO_ACTIVE_LOW>;ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lvds_dec_in_0: endpoint {remote-endpoint = <&lvds_out>;};};port@2{reg = <2>;lvds_dec_out_2: endpoint {remote-endpoint = <&adv7511_in>;};};};- };
Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel output converter.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org --- drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 255 ++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3b99d5a..5815a20 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,6 +92,12 @@ config DRM_SII9234 It is an I2C driver, that detects connection of MHL bridge and starts encapsulation of HDMI signal.
+config DRM_THINE_THC63LVD1024 + tristate "Thine THC63LVD1024 LVDS decoder bridge" + depends on OF + ---help--- + Thine THC63LVD1024 LVDS/parallel converter driver. + config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o obj-$(CONFIG_DRM_SII902X) += sii902x.o obj-$(CONFIG_DRM_SII9234) += sii9234.o +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 index 0000000..067f231 --- /dev/null +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -0,0 +1,255 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * THC63LVD1024 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> + +enum { + THC63_LVDS_IN0, + THC63_LVDS_IN1, + THC63_DIGITAL_OUT0, + THC63_DIGITAL_OUT1, +}; + +static const char * const thc63_reg_names[] = { + "vcc", "lvcc", "pvcc", "cvcc", +}; + +struct thc63_dev { + struct device *dev; + + struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)]; + + struct gpio_desc *pdwn; + struct gpio_desc *oe; + + struct drm_bridge bridge; + struct drm_bridge *next; +}; + +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) +{ + return container_of(bridge, struct thc63_dev, bridge); +} + +static int thc63_attach(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + + return drm_bridge_attach(bridge->encoder, thc63->next, bridge); +} + +static void thc63_enable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + struct regulator *vcc; + int i; + + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) { + vcc = thc63->vccs[i]; + if (!vcc) + continue; + + if (regulator_enable(vcc)) + goto error_vcc_enable; + } + + if (thc63->pdwn) + gpiod_set_value(thc63->pdwn, 0); + + if (thc63->oe) + gpiod_set_value(thc63->oe, 1); + + return; + +error_vcc_enable: + dev_err(thc63->dev, "Failed to enable regulator %u\n", i); + + for (i = i - 1; i >= 0; i--) { + vcc = thc63->vccs[i]; + if (!vcc) + continue; + + regulator_disable(vcc); + } +} + +static void thc63_disable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + struct regulator *vcc; + int i; + + if (thc63->oe) + gpiod_set_value(thc63->oe, 0); + + if (thc63->pdwn) + gpiod_set_value(thc63->pdwn, 1); + + for (i = ARRAY_SIZE(thc63->vccs) - 1; i >= 0; i--) { + vcc = thc63->vccs[i]; + if (!vcc) + continue; + + if (regulator_disable(vcc)) + dev_dbg(thc63->dev, + "Failed to disable regulator %u\n", i); + } +} + +struct drm_bridge_funcs thc63_bridge_func = { + .attach = thc63_attach, + .enable = thc63_enable, + .disable = thc63_disable, +}; + +static int thc63_parse_dt(struct thc63_dev *thc63) +{ + struct device_node *thc63_out; + struct device_node *remote; + int ret = 0; + + thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node, + THC63_DIGITAL_OUT0, -1); + if (!thc63_out) { + dev_err(thc63->dev, "Missing endpoint in Port@%u\n", + THC63_DIGITAL_OUT0); + return -ENODEV; + } + + remote = of_graph_get_remote_port_parent(thc63_out); + if (!remote) { + dev_err(thc63->dev, "Endpoint in Port@%u unconnected\n", + THC63_DIGITAL_OUT0); + ret = -ENODEV; + goto error_put_out_node; + } + + if (!of_device_is_available(remote)) { + dev_err(thc63->dev, "Port@%u remote endpoint is disabled\n", + THC63_DIGITAL_OUT0); + ret = -ENODEV; + goto error_put_remote_node; + } + + thc63->next = of_drm_find_bridge(remote); + if (!thc63->next) + ret = -EPROBE_DEFER; + +error_put_remote_node: + of_node_put(remote); +error_put_out_node: + of_node_put(thc63_out); + + return ret; +} + +static int thc63_gpio_init(struct thc63_dev *thc63) +{ + thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW); + if (IS_ERR(thc63->oe)) { + dev_err(thc63->dev, "Unable to get "oe-gpios"\n"); + return PTR_ERR(thc63->oe); + } + + thc63->pdwn = devm_gpiod_get_optional(thc63->dev, "pdwn", + GPIOD_OUT_HIGH); + if (IS_ERR(thc63->pdwn)) { + dev_err(thc63->dev, "Unable to get "pdwn-gpios"\n"); + return PTR_ERR(thc63->pdwn); + } + + return 0; +} + +static int thc63_regulator_init(struct thc63_dev *thc63) +{ + struct regulator **reg; + int i; + + reg = &thc63->vccs[0]; + for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++, reg++) { + *reg = devm_regulator_get_optional(thc63->dev, + thc63_reg_names[i]); + if (IS_ERR(*reg)) { + if (PTR_ERR(*reg) == -EPROBE_DEFER) + return -EPROBE_DEFER; + *reg = NULL; + } + } + + return 0; +} + +static int thc63_probe(struct platform_device *pdev) +{ + struct thc63_dev *thc63; + int ret; + + thc63 = devm_kzalloc(&pdev->dev, sizeof(*thc63), GFP_KERNEL); + if (!thc63) + return -ENOMEM; + + thc63->dev = &pdev->dev; + platform_set_drvdata(pdev, thc63); + + ret = thc63_regulator_init(thc63); + if (ret) + return ret; + + ret = thc63_gpio_init(thc63); + if (ret) + return ret; + + ret = thc63_parse_dt(thc63); + if (ret) + return ret; + + thc63->bridge.driver_private = thc63; + thc63->bridge.of_node = pdev->dev.of_node; + thc63->bridge.funcs = &thc63_bridge_func; + + drm_bridge_add(&thc63->bridge); + + return 0; +} + +static int thc63_remove(struct platform_device *pdev) +{ + struct thc63_dev *thc63 = platform_get_drvdata(pdev); + + drm_bridge_remove(&thc63->bridge); + + return 0; +} + +#ifdef CONFIG_OF +static const struct of_device_id thc63_match[] = { + { .compatible = "thine,thc63lvd1024", }, + { }, +}; +MODULE_DEVICE_TABLE(of, thc63_match); +#endif + +static struct platform_driver thc63_driver = { + .probe = thc63_probe, + .remove = thc63_remove, + .driver = { + .name = "thc63lvd1024", + .of_match_table = thc63_match, + }, +}; +module_platform_driver(thc63_driver); + +MODULE_AUTHOR("Jacopo Mondi jacopo@jmondi.org"); +MODULE_DESCRIPTION("Thine THC63LVD1024 LVDS decoder DRM bridge driver"); +MODULE_LICENSE("GPL v2");
On 15.03.2018 11:56, Jacopo Mondi wrote:
Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel output converter.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 255 ++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3b99d5a..5815a20 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,6 +92,12 @@ config DRM_SII9234 It is an I2C driver, that detects connection of MHL bridge and starts encapsulation of HDMI signal.
+config DRM_THINE_THC63LVD1024
- tristate "Thine THC63LVD1024 LVDS decoder bridge"
- depends on OF
- ---help---
Thine THC63LVD1024 LVDS/parallel converter driver.config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o obj-$(CONFIG_DRM_SII902X) += sii902x.o obj-$(CONFIG_DRM_SII9234) += sii9234.o +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 index 0000000..067f231 --- /dev/null +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -0,0 +1,255 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- THC63LVD1024 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>
+enum {
- THC63_LVDS_IN0,
- THC63_LVDS_IN1,
- THC63_DIGITAL_OUT0,
- THC63_DIGITAL_OUT1,
+};
+static const char * const thc63_reg_names[] = {
- "vcc", "lvcc", "pvcc", "cvcc",
+};
+struct thc63_dev {
- struct device *dev;
- struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)];
- struct gpio_desc *pdwn;
- struct gpio_desc *oe;
- struct drm_bridge bridge;
- struct drm_bridge *next;
+};
+static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) +{
- return container_of(bridge, struct thc63_dev, bridge);
+}
+static int thc63_attach(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- return drm_bridge_attach(bridge->encoder, thc63->next, bridge);
+}
+static void thc63_enable(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- struct regulator *vcc;
- int i;
- for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
vcc = thc63->vccs[i];if (!vcc)continue;if (regulator_enable(vcc))goto error_vcc_enable;- }
- if (thc63->pdwn)
gpiod_set_value(thc63->pdwn, 0);- if (thc63->oe)
gpiod_set_value(thc63->oe, 1);- return;
+error_vcc_enable:
- dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
It would be better to use supply or regulator name instead of index.
- for (i = i - 1; i >= 0; i--) {
vcc = thc63->vccs[i];if (!vcc)continue;regulator_disable(vcc);- }
+}
+static void thc63_disable(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- struct regulator *vcc;
- int i;
- if (thc63->oe)
gpiod_set_value(thc63->oe, 0);- if (thc63->pdwn)
gpiod_set_value(thc63->pdwn, 1);- for (i = ARRAY_SIZE(thc63->vccs) - 1; i >= 0; i--) {
vcc = thc63->vccs[i];if (!vcc)continue;if (regulator_disable(vcc))dev_dbg(thc63->dev,"Failed to disable regulator %u\n", i);
ditto
- }
+}
+struct drm_bridge_funcs thc63_bridge_func = {
- .attach = thc63_attach,
- .enable = thc63_enable,
- .disable = thc63_disable,
+};
+static int thc63_parse_dt(struct thc63_dev *thc63) +{
- struct device_node *thc63_out;
- struct device_node *remote;
- int ret = 0;
- thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
THC63_DIGITAL_OUT0, -1);- if (!thc63_out) {
dev_err(thc63->dev, "Missing endpoint in Port@%u\n",THC63_DIGITAL_OUT0);return -ENODEV;- }
- remote = of_graph_get_remote_port_parent(thc63_out);
- if (!remote) {
dev_err(thc63->dev, "Endpoint in Port@%u unconnected\n",THC63_DIGITAL_OUT0);ret = -ENODEV;goto error_put_out_node;- }
- if (!of_device_is_available(remote)) {
dev_err(thc63->dev, "Port@%u remote endpoint is disabled\n",THC63_DIGITAL_OUT0);ret = -ENODEV;goto error_put_remote_node;- }
- thc63->next = of_drm_find_bridge(remote);
- if (!thc63->next)
ret = -EPROBE_DEFER;+error_put_remote_node:
- of_node_put(remote);
+error_put_out_node:
- of_node_put(thc63_out);
- return ret;
+}
+static int thc63_gpio_init(struct thc63_dev *thc63) +{
- thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW);
- if (IS_ERR(thc63->oe)) {
dev_err(thc63->dev, "Unable to get \"oe-gpios\"\n");return PTR_ERR(thc63->oe);- }
- thc63->pdwn = devm_gpiod_get_optional(thc63->dev, "pdwn",
GPIOD_OUT_HIGH);- if (IS_ERR(thc63->pdwn)) {
dev_err(thc63->dev, "Unable to get \"pdwn-gpios\"\n");return PTR_ERR(thc63->pdwn);- }
- return 0;
+}
+static int thc63_regulator_init(struct thc63_dev *thc63) +{
- struct regulator **reg;
- int i;
- reg = &thc63->vccs[0];
- for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++, reg++) {
*reg = devm_regulator_get_optional(thc63->dev,thc63_reg_names[i]);if (IS_ERR(*reg)) {if (PTR_ERR(*reg) == -EPROBE_DEFER)return -EPROBE_DEFER;*reg = NULL;}
You could use "if (!...) continue", to avoid nesting.
With or without suggested changes: Reviewed-by: Andrzej Hajda a.hajda@samsung.com
-- Regards Andrzej
- }
- return 0;
+}
+static int thc63_probe(struct platform_device *pdev) +{
- struct thc63_dev *thc63;
- int ret;
- thc63 = devm_kzalloc(&pdev->dev, sizeof(*thc63), GFP_KERNEL);
- if (!thc63)
return -ENOMEM;- thc63->dev = &pdev->dev;
- platform_set_drvdata(pdev, thc63);
- ret = thc63_regulator_init(thc63);
- if (ret)
return ret;- ret = thc63_gpio_init(thc63);
- if (ret)
return ret;- ret = thc63_parse_dt(thc63);
- if (ret)
return ret;- thc63->bridge.driver_private = thc63;
- thc63->bridge.of_node = pdev->dev.of_node;
- thc63->bridge.funcs = &thc63_bridge_func;
- drm_bridge_add(&thc63->bridge);
- return 0;
+}
+static int thc63_remove(struct platform_device *pdev) +{
- struct thc63_dev *thc63 = platform_get_drvdata(pdev);
- drm_bridge_remove(&thc63->bridge);
- return 0;
+}
+#ifdef CONFIG_OF +static const struct of_device_id thc63_match[] = {
- { .compatible = "thine,thc63lvd1024", },
- { },
+}; +MODULE_DEVICE_TABLE(of, thc63_match); +#endif
+static struct platform_driver thc63_driver = {
- .probe = thc63_probe,
- .remove = thc63_remove,
- .driver = {
.name = "thc63lvd1024",.of_match_table = thc63_match,- },
+}; +module_platform_driver(thc63_driver);
+MODULE_AUTHOR("Jacopo Mondi jacopo@jmondi.org"); +MODULE_DESCRIPTION("Thine THC63LVD1024 LVDS decoder DRM bridge driver"); +MODULE_LICENSE("GPL v2");
Hi Andrzej, thanks for your patience in reviewing this series
On Thu, Mar 15, 2018 at 02:37:00PM +0100, Andrzej Hajda wrote:
On 15.03.2018 11:56, Jacopo Mondi wrote:
Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel output converter.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 255 ++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3b99d5a..5815a20 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -92,6 +92,12 @@ config DRM_SII9234 It is an I2C driver, that detects connection of MHL bridge and starts encapsulation of HDMI signal.
+config DRM_THINE_THC63LVD1024
- tristate "Thine THC63LVD1024 LVDS decoder bridge"
- depends on OF
- ---help---
Thine THC63LVD1024 LVDS/parallel converter driver.config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o obj-$(CONFIG_DRM_SII902X) += sii902x.o obj-$(CONFIG_DRM_SII9234) += sii9234.o +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 index 0000000..067f231 --- /dev/null +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -0,0 +1,255 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- THC63LVD1024 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>
+enum {
- THC63_LVDS_IN0,
- THC63_LVDS_IN1,
- THC63_DIGITAL_OUT0,
- THC63_DIGITAL_OUT1,
+};
+static const char * const thc63_reg_names[] = {
- "vcc", "lvcc", "pvcc", "cvcc",
+};
+struct thc63_dev {
- struct device *dev;
- struct regulator *vccs[ARRAY_SIZE(thc63_reg_names)];
- struct gpio_desc *pdwn;
- struct gpio_desc *oe;
- struct drm_bridge bridge;
- struct drm_bridge *next;
+};
+static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) +{
- return container_of(bridge, struct thc63_dev, bridge);
+}
+static int thc63_attach(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- return drm_bridge_attach(bridge->encoder, thc63->next, bridge);
+}
+static void thc63_enable(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- struct regulator *vcc;
- int i;
- for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++) {
vcc = thc63->vccs[i];if (!vcc)continue;if (regulator_enable(vcc))goto error_vcc_enable;- }
- if (thc63->pdwn)
gpiod_set_value(thc63->pdwn, 0);- if (thc63->oe)
gpiod_set_value(thc63->oe, 1);- return;
+error_vcc_enable:
- dev_err(thc63->dev, "Failed to enable regulator %u\n", i);
It would be better to use supply or regulator name instead of index.
Yeah, now it's easy as I have the array with names in the driver global scope...
- for (i = i - 1; i >= 0; i--) {
vcc = thc63->vccs[i];if (!vcc)continue;regulator_disable(vcc);- }
+}
+static void thc63_disable(struct drm_bridge *bridge) +{
- struct thc63_dev *thc63 = to_thc63(bridge);
- struct regulator *vcc;
- int i;
- if (thc63->oe)
gpiod_set_value(thc63->oe, 0);- if (thc63->pdwn)
gpiod_set_value(thc63->pdwn, 1);- for (i = ARRAY_SIZE(thc63->vccs) - 1; i >= 0; i--) {
vcc = thc63->vccs[i];if (!vcc)continue;if (regulator_disable(vcc))dev_dbg(thc63->dev,"Failed to disable regulator %u\n", i);ditto
- }
+}
+struct drm_bridge_funcs thc63_bridge_func = {
- .attach = thc63_attach,
- .enable = thc63_enable,
- .disable = thc63_disable,
+};
+static int thc63_parse_dt(struct thc63_dev *thc63) +{
- struct device_node *thc63_out;
- struct device_node *remote;
- int ret = 0;
- thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
THC63_DIGITAL_OUT0, -1);- if (!thc63_out) {
dev_err(thc63->dev, "Missing endpoint in Port@%u\n",THC63_DIGITAL_OUT0);return -ENODEV;- }
- remote = of_graph_get_remote_port_parent(thc63_out);
- if (!remote) {
dev_err(thc63->dev, "Endpoint in Port@%u unconnected\n",THC63_DIGITAL_OUT0);ret = -ENODEV;goto error_put_out_node;- }
- if (!of_device_is_available(remote)) {
dev_err(thc63->dev, "Port@%u remote endpoint is disabled\n",THC63_DIGITAL_OUT0);ret = -ENODEV;goto error_put_remote_node;- }
- thc63->next = of_drm_find_bridge(remote);
- if (!thc63->next)
ret = -EPROBE_DEFER;+error_put_remote_node:
- of_node_put(remote);
+error_put_out_node:
- of_node_put(thc63_out);
- return ret;
+}
+static int thc63_gpio_init(struct thc63_dev *thc63) +{
- thc63->oe = devm_gpiod_get_optional(thc63->dev, "oe", GPIOD_OUT_LOW);
- if (IS_ERR(thc63->oe)) {
dev_err(thc63->dev, "Unable to get \"oe-gpios\"\n");return PTR_ERR(thc63->oe);- }
- thc63->pdwn = devm_gpiod_get_optional(thc63->dev, "pdwn",
GPIOD_OUT_HIGH);- if (IS_ERR(thc63->pdwn)) {
dev_err(thc63->dev, "Unable to get \"pdwn-gpios\"\n");return PTR_ERR(thc63->pdwn);- }
- return 0;
+}
+static int thc63_regulator_init(struct thc63_dev *thc63) +{
- struct regulator **reg;
- int i;
- reg = &thc63->vccs[0];
- for (i = 0; i < ARRAY_SIZE(thc63->vccs); i++, reg++) {
*reg = devm_regulator_get_optional(thc63->dev,thc63_reg_names[i]);if (IS_ERR(*reg)) {if (PTR_ERR(*reg) == -EPROBE_DEFER)return -EPROBE_DEFER;*reg = NULL;}You could use "if (!...) continue", to avoid nesting.
(unfortunately) regulator_get_optional() does not return NULL as gpiod_get_optional() does (I'm sure there are good reasons) but returns ERR_PTR(-ENODEV), instead of a dummy regulator returned by the non _optional() version. If I got your comment right, I cannot simply check for a NULL value here, but instead make sure the 'reg' pointer is set to NULL so that I can skip it during enable/disable.
With or without suggested changes: Reviewed-by: Andrzej Hajda a.hajda@samsung.com
Thanks, v5 will have your Reviewed-by tag on all patches!
Thanks j
-- Regards Andrzej
- }
- return 0;
+}
+static int thc63_probe(struct platform_device *pdev) +{
- struct thc63_dev *thc63;
- int ret;
- thc63 = devm_kzalloc(&pdev->dev, sizeof(*thc63), GFP_KERNEL);
- if (!thc63)
return -ENOMEM;- thc63->dev = &pdev->dev;
- platform_set_drvdata(pdev, thc63);
- ret = thc63_regulator_init(thc63);
- if (ret)
return ret;- ret = thc63_gpio_init(thc63);
- if (ret)
return ret;- ret = thc63_parse_dt(thc63);
- if (ret)
return ret;- thc63->bridge.driver_private = thc63;
- thc63->bridge.of_node = pdev->dev.of_node;
- thc63->bridge.funcs = &thc63_bridge_func;
- drm_bridge_add(&thc63->bridge);
- return 0;
+}
+static int thc63_remove(struct platform_device *pdev) +{
- struct thc63_dev *thc63 = platform_get_drvdata(pdev);
- drm_bridge_remove(&thc63->bridge);
- return 0;
+}
+#ifdef CONFIG_OF +static const struct of_device_id thc63_match[] = {
- { .compatible = "thine,thc63lvd1024", },
- { },
+}; +MODULE_DEVICE_TABLE(of, thc63_match); +#endif
+static struct platform_driver thc63_driver = {
- .probe = thc63_probe,
- .remove = thc63_remove,
- .driver = {
.name = "thc63lvd1024",.of_match_table = thc63_match,- },
+}; +module_platform_driver(thc63_driver);
+MODULE_AUTHOR("Jacopo Mondi jacopo@jmondi.org"); +MODULE_DESCRIPTION("Thine THC63LVD1024 LVDS decoder DRM bridge driver"); +MODULE_LICENSE("GPL v2");
The R-Car V3M Eagle board includes a transparent THC63LVD1024 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 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 | 33 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index c0fd144..69f43b8 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -42,6 +42,33 @@ }; }; }; + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + thc63lvd1024_in_0: endpoint { + remote-endpoint = <&lvds0_out>; + }; + }; + + port@2{ + reg = <2>; + + thc63lvd1024_out_2: endpoint { + remote-endpoint = <&adv7511_in>; + }; + + }; + + }; + }; };
&avb { @@ -98,7 +125,7 @@ port@0 { reg = <0>; adv7511_in: endpoint { - remote-endpoint = <&lvds0_out>; + remote-endpoint = <&thc63lvd1024_out_2>; }; };
@@ -152,8 +179,8 @@
ports { port@1 { - endpoint { - remote-endpoint = <&adv7511_in>; + lvds0_out: endpoint { + remote-endpoint = <&thc63lvd1024_in_0>; }; }; };
On 15.03.2018 11:56, Jacopo Mondi wrote:
The R-Car V3M Eagle board includes a transparent THC63LVD1024 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 is available, describe it in DT as well.
Signed-off-by: Jacopo Mondi jacopo+renesas@jmondi.org
Reviewed-by: Andrzej Hajda a.hajda@samsung.com
-- Regards Andrzej
arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 33 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index c0fd144..69f43b8 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -42,6 +42,33 @@ }; }; };
- thc63lvd1024: lvds-decoder {
compatible = "thine,thc63lvd1024";ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;thc63lvd1024_in_0: endpoint {remote-endpoint = <&lvds0_out>;};};port@2{reg = <2>;thc63lvd1024_out_2: endpoint {remote-endpoint = <&adv7511_in>;};};};- };
};
&avb { @@ -98,7 +125,7 @@ port@0 { reg = <0>; adv7511_in: endpoint {
remote-endpoint = <&lvds0_out>;
remote-endpoint = <&thc63lvd1024_out_2>; }; };@@ -152,8 +179,8 @@
ports { port@1 {
endpoint {remote-endpoint = <&adv7511_in>;
lvds0_out: endpoint {remote-endpoint = <&thc63lvd1024_in_0>; };
dri-devel@lists.freedesktop.org
-
Andrzej Hajda -
jacopo mondi -
Jacopo Mondi