* [PATCH 0/3] drm/bridge: various small lvds-encoder things
@ 2018-12-18 23:19 Peter Rosin
2018-12-18 23:19 ` [PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187 Peter Rosin
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Peter Rosin @ 2018-12-18 23:19 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, David Airlie, Rob Herring, Mark Rutland,
Archit Taneja, Andrzej Hajda, Laurent Pinchart, dri-devel,
devicetree
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187
2018-12-18 23:19 [PATCH 0/3] drm/bridge: various small lvds-encoder things Peter Rosin
@ 2018-12-18 23:19 ` Peter Rosin
2018-12-18 23:19 ` [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios Peter Rosin
2018-12-18 23:19 ` [PATCH 3/3] drm/bridge: add pwdn-gpios support to the lvds-encoder Peter Rosin
2 siblings, 0 replies; 8+ messages in thread
From: Peter Rosin @ 2018-12-18 23:19 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, David Airlie, Rob Herring, Mark Rutland,
Archit Taneja, Andrzej Hajda, Laurent Pinchart, dri-devel,
devicetree
This chip is compatible with "lvds-encoder".
Signed-off-by: Peter Rosin <peda@axentia.se>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
2018-12-18 23:19 [PATCH 0/3] drm/bridge: various small lvds-encoder things Peter Rosin
2018-12-18 23:19 ` [PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187 Peter Rosin
@ 2018-12-18 23:19 ` Peter Rosin
2018-12-19 9:12 ` Andrzej Hajda
2018-12-18 23:19 ` [PATCH 3/3] drm/bridge: add pwdn-gpios support to the lvds-encoder Peter Rosin
2 siblings, 1 reply; 8+ messages in thread
From: Peter Rosin @ 2018-12-18 23:19 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, David Airlie, Rob Herring, Mark Rutland,
Archit Taneja, Andrzej Hajda, Laurent Pinchart, dri-devel,
devicetree
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 <peda@axentia.se>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] drm/bridge: add pwdn-gpios support to the lvds-encoder
2018-12-18 23:19 [PATCH 0/3] drm/bridge: various small lvds-encoder things Peter Rosin
2018-12-18 23:19 ` [PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187 Peter Rosin
2018-12-18 23:19 ` [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios Peter Rosin
@ 2018-12-18 23:19 ` Peter Rosin
2 siblings, 0 replies; 8+ messages in thread
From: Peter Rosin @ 2018-12-18 23:19 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, David Airlie, Rob Herring, Mark Rutland,
Archit Taneja, Andrzej Hajda, Laurent Pinchart, dri-devel,
devicetree
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
2018-12-18 23:19 ` [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios Peter Rosin
@ 2018-12-19 9:12 ` Andrzej Hajda
2018-12-19 9:57 ` Peter Rosin
0 siblings, 1 reply; 8+ messages in thread
From: Andrzej Hajda @ 2018-12-19 9:12 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: David Airlie, Rob Herring, Mark Rutland, Archit Taneja,
Laurent Pinchart, dri-devel, devicetree
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 <peda@axentia.se>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
2018-12-19 9:12 ` Andrzej Hajda
@ 2018-12-19 9:57 ` Peter Rosin
2018-12-19 11:38 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Peter Rosin @ 2018-12-19 9:57 UTC (permalink / raw)
To: Andrzej Hajda, linux-kernel
Cc: David Airlie, Rob Herring, Mark Rutland, Archit Taneja,
Laurent Pinchart, dri-devel, devicetree
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 <peda@axentia.se>
>> ---
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
2018-12-19 9:57 ` Peter Rosin
@ 2018-12-19 11:38 ` Laurent Pinchart
2018-12-19 12:54 ` Peter Rosin
0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2018-12-19 11:38 UTC (permalink / raw)
To: Peter Rosin
Cc: Andrzej Hajda, linux-kernel, David Airlie, Rob Herring,
Mark Rutland, Archit Taneja, dri-devel, devicetree
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 <peda@axentia.se>
> >> ---
> >>
> >> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios
2018-12-19 11:38 ` Laurent Pinchart
@ 2018-12-19 12:54 ` Peter Rosin
0 siblings, 0 replies; 8+ messages in thread
From: Peter Rosin @ 2018-12-19 12:54 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Andrzej Hajda, linux-kernel, David Airlie, Rob Herring,
Mark Rutland, Archit Taneja, dri-devel, devicetree
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 <peda@axentia.se>
>>>> ---
>>>>
>>>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-19 12:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-18 23:19 [PATCH 0/3] drm/bridge: various small lvds-encoder things Peter Rosin
2018-12-18 23:19 ` [PATCH 1/3] dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c187 Peter Rosin
2018-12-18 23:19 ` [PATCH 2/3] dt-bindings: display: bridge: lvds-transmitter: add pwdn-gpios Peter Rosin
2018-12-19 9:12 ` Andrzej Hajda
2018-12-19 9:57 ` Peter Rosin
2018-12-19 11:38 ` Laurent Pinchart
2018-12-19 12:54 ` Peter Rosin
2018-12-18 23:19 ` [PATCH 3/3] drm/bridge: add pwdn-gpios support to the lvds-encoder Peter Rosin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).