2018-12-19 16:33:44

by Peter Rosin

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

From: Peter Rosin <[email protected]>

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



2018-12-19 13:25:40

by Peter Rosin

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

From: Peter Rosin <[email protected]>

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

Signed-off-by: Peter Rosin <[email protected]>
---
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


2018-12-19 13:26:50

by Peter Rosin

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

From: Peter Rosin <[email protected]>

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 <[email protected]>
---
.../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


2018-12-19 15:21:52

by Peter Rosin

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

From: Peter Rosin <[email protected]>

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

Signed-off-by: Peter Rosin <[email protected]>
---
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


2018-12-28 08:29:18

by Peter Rosin

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

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 <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> .../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
>>

2018-12-28 09:57:48

by Rob Herring

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

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

Reviewed-by: Rob Herring <[email protected]>

2018-12-28 09:57:48

by Rob Herring

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

On Wed, Dec 19, 2018 at 02:04:47PM +0100, Peter Rosin wrote:
> From: Peter Rosin <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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
>

2018-12-29 05:36:24

by Rob Herring

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

On Thu, Dec 27, 2018 at 4:54 PM Peter Rosin <[email protected]> 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 <[email protected]>
> >>
> >> 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 <[email protected]>
> >> ---
> >> .../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