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, if needed...
Changes since v2:
- changed from pwdn-gpios to powerdown-gpios after discussion with Rob with
new patch 3/5 updating the thine,thc63lvdm83d binding as well as a
consequence
- added patch 4/5 which helps keep lines shorter in the lvds-encoder driver
- added tag from Rob on patch 2/5
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 (5):
dt-bindings: display: bridge: fork out ti,ds90c185 from
lvds-transmitter
dt-bindings: display: bridge: lvds-transmitter: cleanup example
dt-bindings: display: bridge: thc63lvdm83d: use standard
powerdown-gpios
drm/bridge: lvds-encoder: add dev helper variable in .probe()
drm/bridge: lvds-encoder: add powerdown-gpios support
.../bindings/display/bridge/lvds-transmitter.txt | 10 +---
.../bindings/display/bridge/thine,thc63lvdm83d.txt | 2 +-
.../bindings/display/bridge/ti,ds90c185.txt | 55 ++++++++++++++++++++++
drivers/gpu/drm/bridge/lvds-encoder.c | 53 +++++++++++++++++----
4 files changed, 100 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ds90c185.txt
--
2.11.0
Drop #address-cells and #size-cells from the root node in the
example, they are unused.
Reviewed-by: Rob Herring <[email protected]>
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
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..e575f996959a
--- /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:
+
+- powerdown-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";
+
+ powerdown-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
Make the code easier to read and modify.
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/gpu/drm/bridge/lvds-encoder.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
index 75b0d3f6e4de..f6770e83d49d 100644
--- a/drivers/gpu/drm/bridge/lvds-encoder.c
+++ b/drivers/gpu/drm/bridge/lvds-encoder.c
@@ -34,48 +34,47 @@ static struct drm_bridge_funcs funcs = {
static int lvds_encoder_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct device_node *port;
struct device_node *endpoint;
struct device_node *panel_node;
struct drm_panel *panel;
struct lvds_encoder *lvds_encoder;
- lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder),
- GFP_KERNEL);
+ lvds_encoder = devm_kzalloc(dev, sizeof(*lvds_encoder), GFP_KERNEL);
if (!lvds_encoder)
return -ENOMEM;
/* Locate the panel DT node. */
- port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
+ port = of_graph_get_port_by_id(dev->of_node, 1);
if (!port) {
- dev_dbg(&pdev->dev, "port 1 not found\n");
+ dev_dbg(dev, "port 1 not found\n");
return -ENXIO;
}
endpoint = of_get_child_by_name(port, "endpoint");
of_node_put(port);
if (!endpoint) {
- dev_dbg(&pdev->dev, "no endpoint for port 1\n");
+ dev_dbg(dev, "no endpoint for port 1\n");
return -ENXIO;
}
panel_node = of_graph_get_remote_port_parent(endpoint);
of_node_put(endpoint);
if (!panel_node) {
- dev_dbg(&pdev->dev, "no remote endpoint for port 1\n");
+ dev_dbg(dev, "no remote endpoint for port 1\n");
return -ENXIO;
}
panel = of_drm_find_panel(panel_node);
of_node_put(panel_node);
if (!panel) {
- dev_dbg(&pdev->dev, "panel not found, deferring probe\n");
+ dev_dbg(dev, "panel not found, deferring probe\n");
return -EPROBE_DEFER;
}
lvds_encoder->panel_bridge =
- devm_drm_panel_bridge_add(&pdev->dev,
- panel, DRM_MODE_CONNECTOR_LVDS);
+ devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_LVDS);
if (IS_ERR(lvds_encoder->panel_bridge))
return PTR_ERR(lvds_encoder->panel_bridge);
@@ -83,7 +82,7 @@ static int lvds_encoder_probe(struct platform_device *pdev)
* but we need a bridge attached to our of_node for our user
* to look up.
*/
- lvds_encoder->bridge.of_node = pdev->dev.of_node;
+ lvds_encoder->bridge.of_node = dev->of_node;
lvds_encoder->bridge.funcs = &funcs;
drm_bridge_add(&lvds_encoder->bridge);
--
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 f6770e83d49d..36d8557bc097 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 *powerdown_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->powerdown_gpio)
+ gpiod_set_value_cansleep(lvds_encoder->powerdown_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->powerdown_gpio)
+ gpiod_set_value_cansleep(lvds_encoder->powerdown_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->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(lvds_encoder->powerdown_gpio)) {
+ int err = PTR_ERR(lvds_encoder->powerdown_gpio);
+
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "powerdown GPIO failure: %d\n", err);
+ return err;
+ }
+
/* Locate the panel DT node. */
port = of_graph_get_port_by_id(dev->of_node, 1);
if (!port) {
--
2.11.0
The name powerdown-gpios is the standard property name for the
functionality covered by the previous pwdn-gpios name. This rename
should be safe to do since the linux driver supporting the binding
(lvds-encoder.c) never implemented the property, and no dts file
names it. At least not upstream.
Signed-off-by: Peter Rosin <[email protected]>
---
Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
index 527e236e9a2a..fee3c88e1a17 100644
--- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
+++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
@@ -10,7 +10,7 @@ Required properties:
Optional properties:
-- pwdn-gpios: Power down control GPIO
+- powerdown-gpios: Power down control GPIO (the /PWDN pin, active low).
Required nodes:
--
2.11.0
On Sat, Dec 29, 2018 at 07:07:19AM +0000, Peter Rosin wrote:
> 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.
I think you should keep something here. I'd reword it to say any encoder
with additional properties must have a device specific compatible in
addition.
> +- 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..e575f996959a
> --- /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:
> +
> +- powerdown-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";
> +
> + powerdown-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
>
On Sat, 29 Dec 2018 07:07:31 +0000, Peter Rosin wrote:
> The name powerdown-gpios is the standard property name for the
> functionality covered by the previous pwdn-gpios name. This rename
> should be safe to do since the linux driver supporting the binding
> (lvds-encoder.c) never implemented the property, and no dts file
> names it. At least not upstream.
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Rob Herring <[email protected]>
Hi Peter,
Thank you for the patch.
On Sat, Dec 29, 2018 at 07:07:25AM +0000, Peter Rosin wrote:
> Drop #address-cells and #size-cells from the root node in the
> example, they are unused.
>
> Reviewed-by: Rob Herring <[email protected]>
> Signed-off-by: Peter Rosin <[email protected]>
Reviewed-by: Laurent Pinchart <[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>;
--
Regards,
Laurent Pinchart
Hello,
On Fri, Jan 11, 2019 at 08:49:04AM -0600, Rob Herring wrote:
> On Sat, Dec 29, 2018 at 07:07:19AM +0000, Peter Rosin wrote:
> > 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.
>
> I think you should keep something here. I'd reword it to say any encoder
> with additional properties must have a device specific compatible in
> addition.
I was going to mention this too. Maybe
- compatible: Shall contain a device-specific compatible string follow
by "lvds-encoder".
That's stronger than Rob's suggestion, as I don't think we should allow
omitting the device-specific compatible string in any case to make sure
we can preserve backward compatibility in the future.
> > +- 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..e575f996959a
> > --- /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
s/ for/ for/
With these fixed,
Reviewed-by: Laurent Pinchart <[email protected]>
> > +
> > +Optional properties:
> > +
> > +- powerdown-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";
> > +
> > + powerdown-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>;
> > + };
> > + };
> > + };
> > +};
--
Regards,
Laurent Pinchart
Hi Peter,
Thank you for the patch.
On Sat, Dec 29, 2018 at 07:07:31AM +0000, Peter Rosin wrote:
> The name powerdown-gpios is the standard property name for the
> functionality covered by the previous pwdn-gpios name. This rename
> should be safe to do since the linux driver supporting the binding
> (lvds-encoder.c) never implemented the property, and no dts file
> names it. At least not upstream.
>
> Signed-off-by: Peter Rosin <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
> index 527e236e9a2a..fee3c88e1a17 100644
> --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvdm83d.txt
> @@ -10,7 +10,7 @@ Required properties:
>
> Optional properties:
>
> -- pwdn-gpios: Power down control GPIO
> +- powerdown-gpios: Power down control GPIO (the /PWDN pin, active low).
>
> Required nodes:
>
--
Regards,
Laurent Pinchart
Hi Peter,
Thank you for the patch.
On Sat, Dec 29, 2018 at 07:07:37AM +0000, Peter Rosin wrote:
> Make the code easier to read and modify.
>
> Signed-off-by: Peter Rosin <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/gpu/drm/bridge/lvds-encoder.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c
> index 75b0d3f6e4de..f6770e83d49d 100644
> --- a/drivers/gpu/drm/bridge/lvds-encoder.c
> +++ b/drivers/gpu/drm/bridge/lvds-encoder.c
> @@ -34,48 +34,47 @@ static struct drm_bridge_funcs funcs = {
>
> static int lvds_encoder_probe(struct platform_device *pdev)
> {
> + struct device *dev = &pdev->dev;
> struct device_node *port;
> struct device_node *endpoint;
> struct device_node *panel_node;
> struct drm_panel *panel;
> struct lvds_encoder *lvds_encoder;
>
> - lvds_encoder = devm_kzalloc(&pdev->dev, sizeof(*lvds_encoder),
> - GFP_KERNEL);
> + lvds_encoder = devm_kzalloc(dev, sizeof(*lvds_encoder), GFP_KERNEL);
> if (!lvds_encoder)
> return -ENOMEM;
>
> /* Locate the panel DT node. */
> - port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
> + port = of_graph_get_port_by_id(dev->of_node, 1);
> if (!port) {
> - dev_dbg(&pdev->dev, "port 1 not found\n");
> + dev_dbg(dev, "port 1 not found\n");
> return -ENXIO;
> }
>
> endpoint = of_get_child_by_name(port, "endpoint");
> of_node_put(port);
> if (!endpoint) {
> - dev_dbg(&pdev->dev, "no endpoint for port 1\n");
> + dev_dbg(dev, "no endpoint for port 1\n");
> return -ENXIO;
> }
>
> panel_node = of_graph_get_remote_port_parent(endpoint);
> of_node_put(endpoint);
> if (!panel_node) {
> - dev_dbg(&pdev->dev, "no remote endpoint for port 1\n");
> + dev_dbg(dev, "no remote endpoint for port 1\n");
> return -ENXIO;
> }
>
> panel = of_drm_find_panel(panel_node);
> of_node_put(panel_node);
> if (!panel) {
> - dev_dbg(&pdev->dev, "panel not found, deferring probe\n");
> + dev_dbg(dev, "panel not found, deferring probe\n");
> return -EPROBE_DEFER;
> }
>
> lvds_encoder->panel_bridge =
> - devm_drm_panel_bridge_add(&pdev->dev,
> - panel, DRM_MODE_CONNECTOR_LVDS);
> + devm_drm_panel_bridge_add(dev, panel, DRM_MODE_CONNECTOR_LVDS);
> if (IS_ERR(lvds_encoder->panel_bridge))
> return PTR_ERR(lvds_encoder->panel_bridge);
>
> @@ -83,7 +82,7 @@ static int lvds_encoder_probe(struct platform_device *pdev)
> * but we need a bridge attached to our of_node for our user
> * to look up.
> */
> - lvds_encoder->bridge.of_node = pdev->dev.of_node;
> + lvds_encoder->bridge.of_node = dev->of_node;
> lvds_encoder->bridge.funcs = &funcs;
> drm_bridge_add(&lvds_encoder->bridge);
>
> --
> 2.11.0
>
--
Regards,
Laurent Pinchart
Hi Peter,
Thank you for the patch.
On Sat, Dec 29, 2018 at 07:07:43AM +0000, Peter Rosin wrote:
> Optionally power down the LVDS-encoder when it is not in use.
>
> Signed-off-by: Peter Rosin <[email protected]>
As the powerdown-gpios property is only defined for two encoders, and
both of them should work well with this code, I think the change is
fine. In the future if different power-up/down sequences need to be
implemented for different devices this will need to be made
device-specific.
> ---
> 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 f6770e83d49d..36d8557bc097 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 *powerdown_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->powerdown_gpio)
> + gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 0);
gpiod_set_value_cansleep() already contains a NULL check, so you can
drop it from here.
> +}
> +
> +static void lvds_encoder_disable(struct drm_bridge *bridge)
> +{
> + struct lvds_encoder *lvds_encoder = container_of(bridge,
> + struct lvds_encoder,
> + bridge);
> +
> + if (lvds_encoder->powerdown_gpio)
> + gpiod_set_value_cansleep(lvds_encoder->powerdown_gpio, 1);
Same here.
> +}
> +
> static struct drm_bridge_funcs funcs = {
While at it, could you make this const ?
With these fixed,
Reviewed-by: Laurent Pinchart <[email protected]>
> .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->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(lvds_encoder->powerdown_gpio)) {
> + int err = PTR_ERR(lvds_encoder->powerdown_gpio);
> +
> + if (err != -EPROBE_DEFER)
> + dev_err(dev, "powerdown GPIO failure: %d\n", err);
> + return err;
> + }
> +
> /* Locate the panel DT node. */
> port = of_graph_get_port_by_id(dev->of_node, 1);
> if (!port) {
--
Regards,
Laurent Pinchart