Hi!
These patches are useful for me. Please consider merging.
Cheers,
Peter
Peter Rosin (3):
dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187
dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
drm/bridge: add pwdn-gpios support to the lvds-encoder
.../bindings/display/bridge/lvds-transmitter.txt | 4 +++
drivers/gpu/drm/bridge/lvds-encoder.c | 34 ++++++++++++++++++++++
2 files changed, 38 insertions(+)
--
2.11.0
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
This chip is compatible with "lvds-encoder".
Signed-off-by: Peter Rosin <[email protected]>
---
Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index 50220190c203..f9e7dd666f58 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -24,6 +24,7 @@ Required properties:
- compatible: Must be one or more of the following
- "ti,ds90c185" for the TI DS90C185 FPD-Link Serializer
+ - "ti,ds90c187" for the TI DS90C187 Dual Pixel FPD-Link Serializer
- "lvds-encoder" for a generic LVDS encoder device
When compatible with the generic version, nodes must list the
--
2.11.0
Add optional property to specify a power-down GPIO.
The pwdn-gpios name is already in use by the thine,thc63lvdm83d
binding, so go with that.
Signed-off-by: Peter Rosin <[email protected]>
---
Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
index f9e7dd666f58..47941d39f92f 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
@@ -31,6 +31,9 @@ Required properties:
device-specific version corresponding to the device first
followed by the generic version.
+Optional properties:
+- pwdn-gpios: Power-down control GPIO
+
Required nodes:
This device has two video ports. Their connections are modeled using the OF
--
2.11.0
On 19.12.2018 00:19, Peter Rosin wrote:
> Add optional property to specify a power-down GPIO.
> The pwdn-gpios name is already in use by the thine,thc63lvdm83d
> binding, so go with that.
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> index f9e7dd666f58..47941d39f92f 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> @@ -31,6 +31,9 @@ Required properties:
> device-specific version corresponding to the device first
> followed by the generic version.
>
> +Optional properties:
> +- pwdn-gpios: Power-down control GPIO
> +
Since naming is not enforced by any datasheet I would propose something
more popular with less twisted logic. Maybe:
- enable-gpios: ... (active high).
Regards
Andrzej
> Required nodes:
>
> This device has two video ports. Their connections are modeled using the OF
On 2018-12-19 10:12, Andrzej Hajda wrote:
> On 19.12.2018 00:19, Peter Rosin wrote:
>> Add optional property to specify a power-down GPIO.
>> The pwdn-gpios name is already in use by the thine,thc63lvdm83d
>> binding, so go with that.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
>> ---
>> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> index f9e7dd666f58..47941d39f92f 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>> @@ -31,6 +31,9 @@ Required properties:
>> device-specific version corresponding to the device first
>> followed by the generic version.
>>
>> +Optional properties:
>> +- pwdn-gpios: Power-down control GPIO
>> +
>
>
> Since naming is not enforced by any datasheet I would propose something
> more popular with less twisted logic. Maybe:
>
> - enable-gpios: ... (active high).
That was my original thought too, but the driver implementing the
lvds-encoder bindings also handles the mentioned thine,thc63lvdm83d
lvds encoder, and that binding has the "pwdn" naming. So, for driver
implementation simplicity I went with what was already there, thus
allowing adding support for both bindings with one implementation
(in patch 3/3).
Adding code just to handle multiple names for the same thing does
not sounds too appealing.
Cheers,
Peter
Hello,
On Wednesday, 19 December 2018 11:57:32 EET Peter Rosin wrote:
> On 2018-12-19 10:12, Andrzej Hajda wrote:
> > On 19.12.2018 00:19, Peter Rosin wrote:
> >> Add optional property to specify a power-down GPIO.
> >> The pwdn-gpios name is already in use by the thine,thc63lvdm83d
> >> binding, so go with that.
> >>
> >> Signed-off-by: Peter Rosin <[email protected]>
> >> ---
> >>
> >> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt |
> >> 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> index f9e7dd666f58..47941d39f92f 100644
> >> ---
> >> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> +++
> >> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
> >> @@ -31,6 +31,9 @@ Required properties:
> >> device-specific version corresponding to the device first
> >> followed by the generic version.
> >>
> >> +Optional properties:
> >> +- pwdn-gpios: Power-down control GPIO
> >> +
> >
> > Since naming is not enforced by any datasheet I would propose something
> > more popular with less twisted logic. Maybe:
> >
> > - enable-gpios: ... (active high).
>
> That was my original thought too, but the driver implementing the
> lvds-encoder bindings also handles the mentioned thine,thc63lvdm83d
> lvds encoder, and that binding has the "pwdn" naming. So, for driver
> implementation simplicity I went with what was already there, thus
> allowing adding support for both bindings with one implementation
> (in patch 3/3).
>
> Adding code just to handle multiple names for the same thing does
> not sounds too appealing.
I'm afraid I think we shouldn't add pwdn-gpios support to the lvds encoder DT
bindings. The reason is that control GPIOs (and regulators) come with device-
specific semantics. If we add pwdn-gpios now, we'll then add reset-gpios, and
power supplies, and all of a sudden we'll end up having to encode sequencing
of GPIOs and power supplies in DT. That path has been tried in the past, with
no good results.
I would instead create device-specific bindings, like done for
thine,thc63lvdm83d. It's fine to then add support for the pwdn-gpios property
in the lvds-encoder driver, as long as the meaning of the property comes from
specific DT bindings, not from the generic ones.
--
Regards,
Laurent Pinchart
On 2018-12-19 12:38, Laurent Pinchart wrote:
> Hello,
>
> On Wednesday, 19 December 2018 11:57:32 EET Peter Rosin wrote:
>> On 2018-12-19 10:12, Andrzej Hajda wrote:
>>> On 19.12.2018 00:19, Peter Rosin wrote:
>>>> Add optional property to specify a power-down GPIO.
>>>> The pwdn-gpios name is already in use by the thine,thc63lvdm83d
>>>> binding, so go with that.
>>>>
>>>> Signed-off-by: Peter Rosin <[email protected]>
>>>> ---
>>>>
>>>> Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt |
>>>> 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>>>> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>>>> index f9e7dd666f58..47941d39f92f 100644
>>>> ---
>>>> a/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>>>> +++
>>>> b/Documentation/devicetree/bindings/display/bridge/lvds-transmitter.txt
>>>> @@ -31,6 +31,9 @@ Required properties:
>>>> device-specific version corresponding to the device first
>>>> followed by the generic version.
>>>>
>>>> +Optional properties:
>>>> +- pwdn-gpios: Power-down control GPIO
>>>> +
>>>
>>> Since naming is not enforced by any datasheet I would propose something
>>> more popular with less twisted logic. Maybe:
>>>
>>> - enable-gpios: ... (active high).
>>
>> That was my original thought too, but the driver implementing the
>> lvds-encoder bindings also handles the mentioned thine,thc63lvdm83d
>> lvds encoder, and that binding has the "pwdn" naming. So, for driver
>> implementation simplicity I went with what was already there, thus
>> allowing adding support for both bindings with one implementation
>> (in patch 3/3).
>>
>> Adding code just to handle multiple names for the same thing does
>> not sounds too appealing.
>
> I'm afraid I think we shouldn't add pwdn-gpios support to the lvds encoder DT
> bindings. The reason is that control GPIOs (and regulators) come with device-
> specific semantics. If we add pwdn-gpios now, we'll then add reset-gpios, and
> power supplies, and all of a sudden we'll end up having to encode sequencing
> of GPIOs and power supplies in DT. That path has been tried in the past, with
> no good results.
>
> I would instead create device-specific bindings, like done for
> thine,thc63lvdm83d. It's fine to then add support for the pwdn-gpios property
> in the lvds-encoder driver, as long as the meaning of the property comes from
> specific DT bindings, not from the generic ones.
Right, I'll fork out the bindings for the texas chips. v2 coming up.
Cheers,
Peter