[PATCH v2 0/3] drm/bridge: various small lvds-encoder things

[PATCH v2 0/3] drm/bridge: various small lvds-encoder things

[Peter Rosin]

From: Peter Rosin <peda@axentia.se>

Hi!

I'm not sure if I should have added the texas chips to the lvds_encoder_match
list in the driver, right next to the thine,thc63lvdm83d entry, but ended
up not doing that. That can always be added later, it needed...

Changes since v1:
- fork out the bindings for the texas chips into their own file in order
  to avoid clutter in the generic lvds-transmitter binding.
- added a patch to remove some surplus stuff in the generic lvds-transmitter
  binding.

Cheers,
Peter

Peter Rosin (3):
  dt-bindings: display: bridge: fork out ti,ds90c185 from
    lvds-transmitter
  dt-bindings: display: bridge: lvds-transmitter: cleanup example
  drm/bridge: add pwdn-gpios support to the lvds-encoder

 .../bindings/display/bridge/lvds-transmitter.txt   | 10 +---
 .../bindings/display/bridge/ti,ds90c185.txt        | 55 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/lvds-encoder.c              | 34 +++++++++++++
 3 files changed, 90 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt

-- 
2.11.0

[PATCH v2 1/3] dt-bindings: display: bridge: fork out ti,ds90c185 from lvds-transmitter

[Peter Rosin]

From: Peter Rosin <peda@axentia.se>

DS90C185 has a shutdown pin which does not fit in the lvds-transmitter
binding, which is meant to be generic.

The sister chip DS90C187 is similar to DS90C185, describe it here as well.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/display/bridge/lvds-transmitter.txt   |  8 +---
 .../bindings/display/bridge/ti,ds90c185.txt        | 55 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index 50220190c203..fd39ad34c383 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -22,13 +22,7 @@ among others.
 
 Required properties:
 
-- compatible: Must be one or more of the following
-  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
-  - "lvds-encoder" for a generic LVDS encoder device
-
-  When compatible with the generic version, nodes must list the
-  device-specific version corresponding to the device first
-  followed by the generic version.
+- compatible: Must be "lvds-encoder"
 
 Required nodes:
 
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
new file mode 100644
index 000000000000..a13e778503e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
@@ -0,0 +1,55 @@
+Texas Instruments FPD-Link (LVDS) Serializer
+--------------------------------------------
+
+The DS90C185 and DS90C187 are low-power serializers for portable
+battery-powered applications that reduces the size of the RGB
+interface between the host GPU and the display.
+
+Required properties:
+
+- compatible: Should be
+  "ti,ds90c185", "lvds-encoder"  for the TI DS90C185 FPD-Link Serializer
+  "ti,ds90c187", "lvds-encoder"  for the TI DS90C187 FPD-Link Serializer
+
+Optional properties:
+
+- pwdn-gpios: Power down control GPIO (the PDB pin, active-low)
+
+Required nodes:
+
+The devices have two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for parallel input
+- Video port 1 for LVDS output
+
+
+Example
+-------
+
+lvds-encoder {
+	compatible = "ti,ds90c185", "lvds-encoder";
+
+	pwdn-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			lvds_enc_in: endpoint {
+				remote-endpoint = <&lcdc_out_rgb>;
+			};
+		};
+
+		port@1 {
+			reg = <1>;
+
+			lvds_enc_out: endpoint {
+				remote-endpoint = <&lvds_panel_in>;
+			};
+		};
+	};
+};
-- 
2.11.0

[PATCH v2 2/3] dt-bindings: display: bridge: lvds-transmitter: cleanup example

[Peter Rosin]

From: Peter Rosin <peda@axentia.se>

Drop #address-cells and #size-cells from the root node in the
example, they are unused.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index fd39ad34c383..36ce07c8d8e6 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -38,8 +38,6 @@ Example
 
 lvds-encoder {
 	compatible = "lvds-encoder";
-	#address-cells = <1>;
-	#size-cells = <0>;
 
 	ports {
 		#address-cells = <1>;
-- 
2.11.0

[PATCH v2 3/3] drm/bridge: add pwdn-gpios support to the lvds-encoder

[Peter Rosin]

From: Peter Rosin <peda@axentia.se>

Optionally power down the LVDS-encoder when it is not in use.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/gpu/drm/bridge/lvds-encoder.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
index 75b0d3f6e4de..43d584a6a6b9 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -11,11 +11,13 @@
 #include <drm/drm_bridge.h>
 #include <drm/drm_panel.h>
 
+#include <linux/gpio/consumer.h>
 #include <linux/of_graph.h>
 
 struct lvds_encoder {
 	struct drm_bridge bridge;
 	struct drm_bridge *panel_bridge;
+	struct gpio_desc *pwdn_gpio;
 };
 
 static int lvds_encoder_attach(struct drm_bridge *bridge)
@@ -28,8 +30,30 @@ static int lvds_encoder_attach(struct drm_bridge *bridge)
 				 bridge);
 }
 
+static void lvds_encoder_enable(struct drm_bridge *bridge)
+{
+	struct lvds_encoder *lvds_encoder = container_of(bridge,
+							 struct lvds_encoder,
+							 bridge);
+
+	if (lvds_encoder->pwdn_gpio)
+		gpiod_set_value_cansleep(lvds_encoder->pwdn_gpio, 0);
+}
+
+static void lvds_encoder_disable(struct drm_bridge *bridge)
+{
+	struct lvds_encoder *lvds_encoder = container_of(bridge,
+							 struct lvds_encoder,
+							 bridge);
+
+	if (lvds_encoder->pwdn_gpio)
+		gpiod_set_value_cansleep(lvds_encoder->pwdn_gpio, 1);
+}
+
 static struct drm_bridge_funcs funcs = {
 	.attach = lvds_encoder_attach,
+	.enable = lvds_encoder_enable,
+	.disable = lvds_encoder_disable,
 };
 
 static int lvds_encoder_probe(struct platform_device *pdev)
@@ -45,6 +69,16 @@ static int lvds_encoder_probe(struct platform_device *pdev)
 	if (!lvds_encoder)
 		return -ENOMEM;
 
+	lvds_encoder->pwdn_gpio = devm_gpiod_get_optional(&pdev->dev, "pwdn",
+							  GPIOD_OUT_HIGH);
+	if (IS_ERR(lvds_encoder->pwdn_gpio)) {
+		int err = PTR_ERR(lvds_encoder->pwdn_gpio);
+
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "pwdn GPIO failure: %d\n", err);
+		return err;
+	}
+
 	/* Locate the panel DT node. */
 	port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
 	if (!port) {
-- 
2.11.0

Re: [PATCH v2 1/3] dt-bindings: display: bridge: fork out ti,ds90c185 from lvds-transmitter

[Rob Herring]

On Wed, Dec 19, 2018 at 02:04:47PM +0100, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
> 
> DS90C185 has a shutdown pin which does not fit in the lvds-transmitter
> binding, which is meant to be generic.
> 
> The sister chip DS90C187 is similar to DS90C185, describe it here as well.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../bindings/display/bridge/lvds-transmitter.txt   |  8 +---
>  .../bindings/display/bridge/ti,ds90c185.txt        | 55 ++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> index 50220190c203..fd39ad34c383 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -22,13 +22,7 @@ among others.
>  
>  Required properties:
>  
> -- compatible: Must be one or more of the following
> -  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
> -  - "lvds-encoder" for a generic LVDS encoder device
> -
> -  When compatible with the generic version, nodes must list the
> -  device-specific version corresponding to the device first
> -  followed by the generic version.
> +- compatible: Must be "lvds-encoder"
>  
>  Required nodes:
>  
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> new file mode 100644
> index 000000000000..a13e778503e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> @@ -0,0 +1,55 @@
> +Texas Instruments FPD-Link (LVDS) Serializer
> +--------------------------------------------
> +
> +The DS90C185 and DS90C187 are low-power serializers for portable
> +battery-powered applications that reduces the size of the RGB
> +interface between the host GPU and the display.
> +
> +Required properties:
> +
> +- compatible: Should be
> +  "ti,ds90c185", "lvds-encoder"  for the TI DS90C185 FPD-Link Serializer
> +  "ti,ds90c187", "lvds-encoder"  for the TI DS90C187 FPD-Link Serializer
> +
> +Optional properties:
> +
> +- pwdn-gpios: Power down control GPIO (the PDB pin, active-low)

powerdown-gpios is the standard name.

> +
> +Required nodes:
> +
> +The devices have two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for parallel input
> +- Video port 1 for LVDS output
> +
> +
> +Example
> +-------
> +
> +lvds-encoder {
> +	compatible = "ti,ds90c185", "lvds-encoder";
> +
> +	pwdn-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			lvds_enc_in: endpoint {
> +				remote-endpoint = <&lcdc_out_rgb>;
> +			};
> +		};
> +
> +		port@1 {
> +			reg = <1>;
> +
> +			lvds_enc_out: endpoint {
> +				remote-endpoint = <&lvds_panel_in>;
> +			};
> +		};
> +	};
> +};
> -- 
> 2.11.0
> 

Re: [PATCH v2 2/3] dt-bindings: display: bridge: lvds-transmitter: cleanup example

[Rob Herring]

On Wed, 19 Dec 2018 14:04:48 +0100, Peter Rosin wrote:
> From: Peter Rosin <peda@axentia.se>
> 
> Drop #address-cells and #size-cells from the root node in the
> example, they are unused.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 2 --
>  1 file changed, 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 1/3] dt-bindings: display: bridge: fork out ti,ds90c185 from lvds-transmitter

[Peter Rosin]

On 2018-12-27 22:27, Rob Herring wrote:
> On Wed, Dec 19, 2018 at 02:04:47PM +0100, Peter Rosin wrote:
>> From: Peter Rosin <peda@axentia.se>
>>
>> DS90C185 has a shutdown pin which does not fit in the lvds-transmitter
>> binding, which is meant to be generic.
>>
>> The sister chip DS90C187 is similar to DS90C185, describe it here as well.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  .../bindings/display/bridge/lvds-transmitter.txt   |  8 +---
>>  .../bindings/display/bridge/ti,ds90c185.txt        | 55 ++++++++++++++++++++++
>>  2 files changed, 56 insertions(+), 7 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> index 50220190c203..fd39ad34c383 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> @@ -22,13 +22,7 @@ among others.
>>  
>>  Required properties:
>>  
>> -- compatible: Must be one or more of the following
>> -  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
>> -  - "lvds-encoder" for a generic LVDS encoder device
>> -
>> -  When compatible with the generic version, nodes must list the
>> -  device-specific version corresponding to the device first
>> -  followed by the generic version.
>> +- compatible: Must be "lvds-encoder"
>>  
>>  Required nodes:
>>  
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
>> new file mode 100644
>> index 000000000000..a13e778503e6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
>> @@ -0,0 +1,55 @@
>> +Texas Instruments FPD-Link (LVDS) Serializer
>> +--------------------------------------------
>> +
>> +The DS90C185 and DS90C187 are low-power serializers for portable
>> +battery-powered applications that reduces the size of the RGB
>> +interface between the host GPU and the display.
>> +
>> +Required properties:
>> +
>> +- compatible: Should be
>> +  "ti,ds90c185", "lvds-encoder"  for the TI DS90C185 FPD-Link Serializer
>> +  "ti,ds90c187", "lvds-encoder"  for the TI DS90C187 FPD-Link Serializer
>> +
>> +Optional properties:
>> +
>> +- pwdn-gpios: Power down control GPIO (the PDB pin, active-low)
> 
> powerdown-gpios is the standard name.

The lvds-encoder driver handles this binding, and that driver incidentally
also implements the thine,thc63lvdm83d binding which already has a
pwdn-gpios property. Should the thine,thc63lvdm83d binding be updated and
the driver be made to support both properties?

Since the lvds-encoder driver never had support for the pwdn-gpios (at least
not upstream) I suppose there is also the option to simply go with
powerdown-gpios as you suggest and not bother with support for the
pwdn-gpios property.

I'm quite willing to send an updated series, but I don't know what is
preferred, and am in need of guidance.

Cheers,
Peter

>> +
>> +Required nodes:
>> +
>> +The devices have two video ports. Their connections are modeled using the OF
>> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>> +
>> +- Video port 0 for parallel input
>> +- Video port 1 for LVDS output
>> +
>> +
>> +Example
>> +-------
>> +
>> +lvds-encoder {
>> +	compatible = "ti,ds90c185", "lvds-encoder";
>> +
>> +	pwdn-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
>> +
>> +	ports {
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		port@0 {
>> +			reg = <0>;
>> +
>> +			lvds_enc_in: endpoint {
>> +				remote-endpoint = <&lcdc_out_rgb>;
>> +			};
>> +		};
>> +
>> +		port@1 {
>> +			reg = <1>;
>> +
>> +			lvds_enc_out: endpoint {
>> +				remote-endpoint = <&lvds_panel_in>;
>> +			};
>> +		};
>> +	};
>> +};
>> -- 
>> 2.11.0
>>

Re: [PATCH v2 1/3] dt-bindings: display: bridge: fork out ti,ds90c185 from lvds-transmitter

[Rob Herring]

On Thu, Dec 27, 2018 at 4:54 PM Peter Rosin <peda@axentia.se> wrote:
>
> On 2018-12-27 22:27, Rob Herring wrote:
> > On Wed, Dec 19, 2018 at 02:04:47PM +0100, Peter Rosin wrote:
> >> From: Peter Rosin <peda@axentia.se>
> >>
> >> DS90C185 has a shutdown pin which does not fit in the lvds-transmitter
> >> binding, which is meant to be generic.
> >>
> >> The sister chip DS90C187 is similar to DS90C185, describe it here as well.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >>  .../bindings/display/bridge/lvds-transmitter.txt   |  8 +---
> >>  .../bindings/display/bridge/ti,ds90c185.txt        | 55 ++++++++++++++++++++++
> >>  2 files changed, 56 insertions(+), 7 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> index 50220190c203..fd39ad34c383 100644
> >> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> @@ -22,13 +22,7 @@ among others.
> >>
> >>  Required properties:
> >>
> >> -- compatible: Must be one or more of the following
> >> -  - "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
> >> -  - "lvds-encoder" for a generic LVDS encoder device
> >> -
> >> -  When compatible with the generic version, nodes must list the
> >> -  device-specific version corresponding to the device first
> >> -  followed by the generic version.
> >> +- compatible: Must be "lvds-encoder"
> >>
> >>  Required nodes:
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> >> new file mode 100644
> >> index 000000000000..a13e778503e6
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
> >> @@ -0,0 +1,55 @@
> >> +Texas Instruments FPD-Link (LVDS) Serializer
> >> +--------------------------------------------
> >> +
> >> +The DS90C185 and DS90C187 are low-power serializers for portable
> >> +battery-powered applications that reduces the size of the RGB
> >> +interface between the host GPU and the display.
> >> +
> >> +Required properties:
> >> +
> >> +- compatible: Should be
> >> +  "ti,ds90c185", "lvds-encoder"  for the TI DS90C185 FPD-Link Serializer
> >> +  "ti,ds90c187", "lvds-encoder"  for the TI DS90C187 FPD-Link Serializer
> >> +
> >> +Optional properties:
> >> +
> >> +- pwdn-gpios: Power down control GPIO (the PDB pin, active-low)
> >
> > powerdown-gpios is the standard name.
>
> The lvds-encoder driver handles this binding, and that driver incidentally
> also implements the thine,thc63lvdm83d binding which already has a
> pwdn-gpios property. Should the thine,thc63lvdm83d binding be updated and
> the driver be made to support both properties?
>
> Since the lvds-encoder driver never had support for the pwdn-gpios (at least
> not upstream) I suppose there is also the option to simply go with
> powerdown-gpios as you suggest and not bother with support for the
> pwdn-gpios property.

It is probably safe to just change it given it is not used in any dts
nor supported in the driver.

Rob