2016-12-07 10:42:57

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v5 0/4] ARM: dts: da850: tilcdc related DT changes

This series contains the last DT changes required for LCDC support
on da850-lcdk. The first one adds the dumb-vga-dac nodes, the second
limits the maximum pixel clock rate.

v1 -> v2:
- drop patch 3/3 (already merged)
- use max-pixelclock instead of max-bandwidth for display mode limiting

v2 -> v3:
- make the commit message in patch [2/2] more detailed
- move the max-pixelclock property to da850.dtsi as the limit
affects all da850-based boards

v3 -> v4:
- remove the input port from the display node
- move the display ports node to da850-lcdk.dts
- rename the vga_bridge node to vga-bridge
- move the LCDC pins to the LCDC node (from the vga bridge node)

v4 -> v5:
- rename the display label to lcdc
- instead of using the 'dumb-vga-dac' compatible, add bindings for
ti,ths8135 and use it as the vga-bridge node compatible

Bartosz Golaszewski (4):
ARM: dts: da850: rename the display node label
drm: bridge: add support for TI ths8135
ARM: dts: da850-lcdk: add the vga-bridge node
ARM: dts: da850: specify the maximum pixel clock rate for tilcdc

.../bindings/display/bridge/ti,ths8135.txt | 52 +++++++++++++++++
arch/arm/boot/dts/da850-lcdk.dts | 67 ++++++++++++++++++++++
arch/arm/boot/dts/da850.dtsi | 3 +-
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
4 files changed, 122 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt

--
2.9.3


2016-12-07 10:43:08

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v5 3/4] ARM: dts: da850-lcdk: add the vga-bridge node

Add the vga-bridge node to the board DT together with corresponding
ports and vga connector. This allows to retrieve the edid info from
the display automatically.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm/boot/dts/da850-lcdk.dts | 67 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index afcb482..39602eb 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -51,6 +51,51 @@
system-clock-frequency = <24576000>;
};
};
+
+ vga-bridge {
+ compatible = "ti,ths8135";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_bridge_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lcdc_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_bridge_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+ };
+
+ vga {
+ compatible = "vga-connector";
+
+ ddc-i2c-bus = <&i2c0>;
+
+ port {
+ vga_con_in: endpoint {
+ remote-endpoint = <&vga_bridge_out>;
+ };
+ };
+ };
};

&pmx_core {
@@ -236,3 +281,25 @@
&memctrl {
status = "okay";
};
+
+&lcdc {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd_pins>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ lcdc_out: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ lcdc_out_vga: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_bridge_in>;
+ };
+ };
+ };
+};
--
2.9.3

2016-12-07 10:43:02

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v5 1/4] ARM: dts: da850: rename the display node label

The tilcdc node name is 'display' as per the ePAPR 1.1 recommendation.
The label is also 'display', but change it to 'lcdc' to make it clear
what the underlying hardware is.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm/boot/dts/da850.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index ffc6e1a..3f51e59 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -448,7 +448,7 @@
dma-names = "tx", "rx";
};

- display: display@213000 {
+ lcdc: display@213000 {
compatible = "ti,da850-tilcdc";
reg = <0x213000 0x1000>;
interrupts = <52>;
--
2.9.3

2016-12-07 10:43:16

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v5 4/4] ARM: dts: da850: specify the maximum pixel clock rate for tilcdc

At maximum CPU frequency of 300 MHz the maximum pixel clock frequency
is 37.5 MHz[1]. We must filter out any mode for which the calculated
pixel clock rate would exceed this value.

Specify the max-pixelclock property for the display node for
da850-lcdk.

[1] http://processors.wiki.ti.com/index.php/OMAP-L1x/C674x/AM1x_LCD_Controller_(LCDC)_Throughput_and_Optimization_Techniques

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
arch/arm/boot/dts/da850.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 3f51e59..ba5bf80 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -452,6 +452,7 @@
compatible = "ti,da850-tilcdc";
reg = <0x213000 0x1000>;
interrupts = <52>;
+ max-pixelclock = <37500>;
status = "disabled";
};
};
--
2.9.3

2016-12-07 10:43:36

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH v5 2/4] drm: bridge: add support for TI ths8135

THS8135 is a configurable video DAC. Add DT bindings for this chip and
use the dumb-vga-dac driver for now as no configuration is required to
make it work.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
.../bindings/display/bridge/ti,ths8135.txt | 52 ++++++++++++++++++++++
drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
2 files changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
new file mode 100644
index 0000000..23cd8ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
@@ -0,0 +1,52 @@
+THS8135 Video DAC
+-----------------
+
+This is the binding for Texas Instruments THS8135 Video DAC bridge.
+
+Required properties:
+
+- compatible: Must be "ti,ths8135"
+
+Required nodes:
+
+This device has two video ports. Their connections are modelled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+Example
+-------
+
+vga-bridge {
+ compatible = "ti,ths8135";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ vga_bridge_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lcdc_out_vga>;
+ };
+ };
+
+ port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ vga_bridge_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&vga_con_in>;
+ };
+ };
+ };
+};
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232..498fa75 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)

static const struct of_device_id dumb_vga_match[] = {
{ .compatible = "dumb-vga-dac" },
+ { .compatible = "ti,ths8135" },
{},
};
MODULE_DEVICE_TABLE(of, dumb_vga_match);
--
2.9.3

2016-12-07 10:45:24

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] drm: bridge: add support for TI ths8135

2016-12-07 11:42 GMT+01:00 Bartosz Golaszewski <[email protected]>:
> THS8135 is a configurable video DAC. Add DT bindings for this chip and
> use the dumb-vga-dac driver for now as no configuration is required to
> make it work.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> .../bindings/display/bridge/ti,ths8135.txt | 52 ++++++++++++++++++++++
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> 2 files changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> new file mode 100644
> index 0000000..23cd8ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> @@ -0,0 +1,52 @@
> +THS8135 Video DAC
> +-----------------
> +
> +This is the binding for Texas Instruments THS8135 Video DAC bridge.
> +
> +Required properties:
> +
> +- compatible: Must be "ti,ths8135"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modelled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +Example
> +-------
> +
> +vga-bridge {
> + compatible = "ti,ths8135";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&lcdc_out_vga>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_con_in>;
> + };
> + };
> + };
> +};
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232..498fa75 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>
> static const struct of_device_id dumb_vga_match[] = {
> { .compatible = "dumb-vga-dac" },
> + { .compatible = "ti,ths8135" },
> {},
> };
> MODULE_DEVICE_TABLE(of, dumb_vga_match);
> --
> 2.9.3
>

+ Maxime

Sorry, I forgot to include your e-mail.

Bartosz

2016-12-07 14:22:57

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] drm: bridge: add support for TI ths8135

Hi Bartosz,

Thank you for the patch.

On Wednesday 07 Dec 2016 11:45:11 Bartosz Golaszewski wrote:
> 2016-12-07 11:42 GMT+01:00 Bartosz Golaszewski <[email protected]>:
> > THS8135 is a configurable video DAC. Add DT bindings for this chip and
> > use the dumb-vga-dac driver for now as no configuration is required to
> > make it work.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > ---
> >
> > .../bindings/display/bridge/ti,ths8135.txt | 52 +++++++++++++++++
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +

You might be asked by DT maintainers to split this in two, but regardless of
whether it's one patch or two, you can add my

Reviewed-by: Laurent Pinchart <[email protected]>

> > 2 files changed, 53 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt>
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> > b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt new
> > file mode 100644
> > index 0000000..23cd8ee
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> > @@ -0,0 +1,52 @@
> > +THS8135 Video DAC
> > +-----------------
> > +
> > +This is the binding for Texas Instruments THS8135 Video DAC bridge.
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "ti,ths8135"
> > +
> > +Required nodes:
> > +
> > +This device has two video ports. Their connections are modelled using the
> > OF +graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 for RGB input
> > +- Video port 1 for VGA output
> > +
> > +Example
> > +-------
> > +
> > +vga-bridge {
> > + compatible = "ti,ths8135";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0>;
> > +
> > + vga_bridge_in: endpoint@0 {
> > + reg = <0>;
> > + remote-endpoint = <&lcdc_out_vga>;
> > + };
> > + };
> > +
> > + port@1 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <1>;
> > +
> > + vga_bridge_out: endpoint@0 {
> > + reg = <0>;
> > + remote-endpoint = <&vga_con_in>;
> > + };
> > + };
> > + };
> > +};
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232..498fa75 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
> > *pdev)>
> > static const struct of_device_id dumb_vga_match[] = {
> >
> > { .compatible = "dumb-vga-dac" },
> >
> > + { .compatible = "ti,ths8135" },
> >
> > {},
> >
> > };
> > MODULE_DEVICE_TABLE(of, dumb_vga_match);
> >
> > --
> > 2.9.3
>
> + Maxime
>
> Sorry, I forgot to include your e-mail.
>
> Bartosz

--
Regards,

Laurent Pinchart

2016-12-07 14:25:04

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] ARM: dts: da850-lcdk: add the vga-bridge node

Hi Bartosz,

Thank you for the patch.

On Wednesday 07 Dec 2016 11:42:44 Bartosz Golaszewski wrote:
> Add the vga-bridge node to the board DT together with corresponding
> ports and vga connector. This allows to retrieve the edid info from
> the display automatically.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> arch/arm/boot/dts/da850-lcdk.dts | 67 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
> b/arch/arm/boot/dts/da850-lcdk.dts index afcb482..39602eb 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -51,6 +51,51 @@
> system-clock-frequency = <24576000>;
> };
> };
> +
> + vga-bridge {
> + compatible = "ti,ths8135";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&lcdc_out_vga>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_con_in>;
> + };
> + };
> + };
> + };
> +
> + vga {
> + compatible = "vga-connector";
> +
> + ddc-i2c-bus = <&i2c0>;
> +
> + port {
> + vga_con_in: endpoint {
> + remote-endpoint = <&vga_bridge_out>;
> + };
> + };
> + };
> };
>
> &pmx_core {
> @@ -236,3 +281,25 @@
> &memctrl {
> status = "okay";
> };
> +
> +&lcdc {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcd_pins>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + lcdc_out: port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + lcdc_out_vga: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_bridge_in>;
> + };
> + };

Just to make sure you're aware, when there's a single endpoint you can
simplify the DT by omitting the endpoint number. This would become

lcdc_out: port@1 {
reg = <1>;

lcdc_out_vga: endpoint {
remote-endpoint = <&vga_bridge_in>;
};
};

It's entirely up to you, both get my

Reviewed-by: Laurent Pinchart <[email protected]>

> + };
> +};

--
Regards,

Laurent Pinchart

2016-12-07 14:30:50

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] ARM: dts: da850-lcdk: add the vga-bridge node

2016-12-07 15:25 GMT+01:00 Laurent Pinchart <[email protected]>:
> Hi Bartosz,
>
> Thank you for the patch.
>
> On Wednesday 07 Dec 2016 11:42:44 Bartosz Golaszewski wrote:
>> Add the vga-bridge node to the board DT together with corresponding
>> ports and vga connector. This allows to retrieve the edid info from
>> the display automatically.
>>
>> Signed-off-by: Bartosz Golaszewski <[email protected]>
>> ---
>> arch/arm/boot/dts/da850-lcdk.dts | 67 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>> b/arch/arm/boot/dts/da850-lcdk.dts index afcb482..39602eb 100644
>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>> @@ -51,6 +51,51 @@
>> system-clock-frequency = <24576000>;
>> };
>> };
>> +
>> + vga-bridge {
>> + compatible = "ti,ths8135";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + vga_bridge_in: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&lcdc_out_vga>;
>> + };
>> + };
>> +
>> + port@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + vga_bridge_out: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_con_in>;
>> + };
>> + };
>> + };
>> + };
>> +
>> + vga {
>> + compatible = "vga-connector";
>> +
>> + ddc-i2c-bus = <&i2c0>;
>> +
>> + port {
>> + vga_con_in: endpoint {
>> + remote-endpoint = <&vga_bridge_out>;
>> + };
>> + };
>> + };
>> };
>>
>> &pmx_core {
>> @@ -236,3 +281,25 @@
>> &memctrl {
>> status = "okay";
>> };
>> +
>> +&lcdc {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&lcd_pins>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + lcdc_out: port@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + lcdc_out_vga: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&vga_bridge_in>;
>> + };
>> + };
>
> Just to make sure you're aware, when there's a single endpoint you can
> simplify the DT by omitting the endpoint number. This would become
>
> lcdc_out: port@1 {
> reg = <1>;
>
> lcdc_out_vga: endpoint {
> remote-endpoint = <&vga_bridge_in>;
> };
> };
>
> It's entirely up to you, both get my
>
> Reviewed-by: Laurent Pinchart <[email protected]>
>
>> + };
>> +};
>
> --
> Regards,
>
> Laurent Pinchart
>

Hi Laurent,

thanks for the hint, I'll change it if there's going to be a new
version of the series.

Best regards,
Bartosz Golaszewski

2016-12-12 16:45:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] drm: bridge: add support for TI ths8135

On Wed, Dec 07, 2016 at 11:42:43AM +0100, Bartosz Golaszewski wrote:
> THS8135 is a configurable video DAC. Add DT bindings for this chip and
> use the dumb-vga-dac driver for now as no configuration is required to
> make it work.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> .../bindings/display/bridge/ti,ths8135.txt | 52 ++++++++++++++++++++++
> drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> 2 files changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt

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

But one nit below:

>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> new file mode 100644
> index 0000000..23cd8ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> @@ -0,0 +1,52 @@
> +THS8135 Video DAC
> +-----------------
> +
> +This is the binding for Texas Instruments THS8135 Video DAC bridge.
> +
> +Required properties:
> +
> +- compatible: Must be "ti,ths8135"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modelled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +Example
> +-------
> +
> +vga-bridge {
> + compatible = "ti,ths8135";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + vga_bridge_in: endpoint@0 {
> + reg = <0>;

You don't need reg here.

> + remote-endpoint = <&lcdc_out_vga>;
> + };
> + };
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + vga_bridge_out: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&vga_con_in>;
> + };
> + };
> + };
> +};
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232..498fa75 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device *pdev)
>
> static const struct of_device_id dumb_vga_match[] = {
> { .compatible = "dumb-vga-dac" },
> + { .compatible = "ti,ths8135" },
> {},
> };
> MODULE_DEVICE_TABLE(of, dumb_vga_match);
> --
> 2.9.3
>

2016-12-12 16:48:55

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] drm: bridge: add support for TI ths8135

Hello,

On Monday 12 Dec 2016 10:45:47 Rob Herring wrote:
> On Wed, Dec 07, 2016 at 11:42:43AM +0100, Bartosz Golaszewski wrote:
> > THS8135 is a configurable video DAC. Add DT bindings for this chip and
> > use the dumb-vga-dac driver for now as no configuration is required to
> > make it work.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > ---
> >
> > .../bindings/display/bridge/ti,ths8135.txt | 52 +++++++++++++++++
> > drivers/gpu/drm/bridge/dumb-vga-dac.c | 1 +
> > 2 files changed, 53 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> Acked-by: Rob Herring <[email protected]>
>
> But one nit below:
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> > b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt new
> > file mode 100644
> > index 0000000..23cd8ee
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/ti,ths8135.txt
> > @@ -0,0 +1,52 @@
> > +THS8135 Video DAC
> > +-----------------
> > +
> > +This is the binding for Texas Instruments THS8135 Video DAC bridge.
> > +
> > +Required properties:
> > +
> > +- compatible: Must be "ti,ths8135"
> > +
> > +Required nodes:
> > +
> > +This device has two video ports. Their connections are modelled using the
> > OF +graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 for RGB input
> > +- Video port 1 for VGA output
> > +
> > +Example
> > +-------
> > +
> > +vga-bridge {
> > + compatible = "ti,ths8135";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0>;
> > +
> > + vga_bridge_in: endpoint@0 {
> > + reg = <0>;
>
> You don't need reg here.

In which case the endpoint node should be named endpoint, not endpoint@0. You
could then also remove the #address-cells and #size-cells properties from the
port@0 node. Same for port@1 below.

> > + remote-endpoint = <&lcdc_out_vga>;
> > + };
> > + };
> > +
> > + port@1 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <1>;
> > +
> > + vga_bridge_out: endpoint@0 {
> > + reg = <0>;
> > + remote-endpoint = <&vga_con_in>;
> > + };
> > + };
> > + };
> > +};
> > diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > b/drivers/gpu/drm/bridge/dumb-vga-dac.c index afec232..498fa75 100644
> > --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> > @@ -204,6 +204,7 @@ static int dumb_vga_remove(struct platform_device
> > *pdev)
> >
> > static const struct of_device_id dumb_vga_match[] = {
> > { .compatible = "dumb-vga-dac" },
> > + { .compatible = "ti,ths8135" },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, dumb_vga_match);

--
Regards,

Laurent Pinchart