Dear All,
this series adds support for dual-LVDS panel IDK-2121WR from Advantech: https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-ID... Dual link support is very recent for R-Car Gen3, and I couldn't find much on dual link panels in the kernel either, therefore comments are very welcome to get this right.
The panel doesn't come with the EK874 kit, but it's advertised as supported, therefore this series adds a new dts file to support the configuration of the EK874 + IDK-2121WR.
Finally, this series depends on a fix that's still pending: https://patchwork.kernel.org/patch/11054755/
Thanks, Fab
Fabrizio Castro (12): dt-bindings: display: renesas: lvds: RZ/G2E needs renesas,companion too dt-bindings: display: renesas: lvds: Document renesas,swap-data dt-bindings: panel: lvds: Add dual-link LVDS display support dt-bindings: display: Add bindings for Advantech IDK-2121WR drm: rcar-du: lvds: Add data swap support drm: rcar-du: lvds: Do not look at ports for identifying bridges drm: rcar-du: lvds: Add support for dual link panels drm: rcar-du: lvds: Fix bridge_to_rcar_lvds drm: rcar-du: lvds: Fix companion's mode arm64: dts: renesas: r8a774c0: Point LVDS0 to its companion LVDS1 arm64: dts: renesas: cat874: Add definition for 12V regulator arm64: dts: renesas: Add EK874 board with idk-2121wr display support
.../bindings/display/bridge/renesas,lvds.txt | 11 +- .../display/panel/advantech,idk-2121wr.txt | 62 ++++++++++++ .../bindings/display/panel/panel-lvds.txt | 91 ++++++++++++----- arch/arm64/boot/dts/renesas/Makefile | 3 +- arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 9 ++ .../boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts | 112 +++++++++++++++++++++ arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 + drivers/gpu/drm/rcar-du/rcar_lvds.c | 65 ++++++------ 8 files changed, 291 insertions(+), 64 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt create mode 100644 arch/arm64/boot/dts/renesas/r8a774c0-ek874-idk-2121wr.dts
Document RZ/G2E support for property renesas,companion.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index c6a196d..dece79e 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -49,9 +49,9 @@ Each port shall have a single endpoint. Optional properties:
- renesas,companion : phandle to the companion LVDS encoder. This property is - mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to - the second encoder to be used as a companion in dual-link mode. It shall not - be set for any other LVDS encoder. + mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, + and shall point to the second encoder to be used as a companion in dual-link + mode. It shall not be set for any other LVDS encoder.
Example:
Hello Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:33:58AM +0100, Fabrizio Castro wrote:
Document RZ/G2E support for property renesas,companion.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
and taken in my tree.
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index c6a196d..dece79e 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -49,9 +49,9 @@ Each port shall have a single endpoint. Optional properties:
- renesas,companion : phandle to the companion LVDS encoder. This property is
- mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
- the second encoder to be used as a companion in dual-link mode. It shall not
- be set for any other LVDS encoder.
- mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs,
- and shall point to the second encoder to be used as a companion in dual-link
- mode. It shall not be set for any other LVDS encoder.
Example:
R-Car D3, R-Car E3, and RZ/G2E support dual-link mode. In such a mode, the first LVDS encoder emits even data, and the second LVDS encoder emits odd data. This patch documents property renesas,swap-data, used to swap even and odd data around.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index dece79e..8980179 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -52,6 +52,11 @@ Optional properties: mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, and shall point to the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally + emits even data, and the second LVDS encoder emits odd data. When property + renesas,swap-data is specified, the data emitted by the two encoders will be + swapped around. This property can only be used in conjunction with property + renesas,companion.
Example:
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
R-Car D3, R-Car E3, and RZ/G2E support dual-link mode. In such a mode, the first LVDS encoder emits even data, and the second LVDS encoder emits odd data. This patch documents property renesas,swap-data, used to swap even and odd data around.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index dece79e..8980179 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -52,6 +52,11 @@ Optional properties: mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, and shall point to the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
- emits even data, and the second LVDS encoder emits odd data. When property
- renesas,swap-data is specified, the data emitted by the two encoders will be
- swapped around. This property can only be used in conjunction with property
- renesas,companion.
From an LVDS encoder point of view this is more a configuration option
than a description of the hardware. Wouldn't it be better for the LVDS sink to report which of the odd or even pixels it expects on each of its endpoints ? The LVDS encoder driver could then query that at runtime and configure itself accordingly. Ideally this should be queried through the drm_bridge_timings structure (or through a similar mean), not through DT. An LVDS sink that has a fixed mapping of odd/even pixels to endpoints wouldn't need the information to be specified in DT at all.
Example:
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 08:44 Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
R-Car D3, R-Car E3, and RZ/G2E support dual-link mode. In such a mode, the first LVDS encoder emits even data, and the second LVDS encoder emits odd data. This patch documents property renesas,swap-data, used to swap even and odd data around.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index dece79e..8980179 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -52,6 +52,11 @@ Optional properties: mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, and shall point to the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
- emits even data, and the second LVDS encoder emits odd data. When property
- renesas,swap-data is specified, the data emitted by the two encoders will be
- swapped around. This property can only be used in conjunction with property
- renesas,companion.
From an LVDS encoder point of view this is more a configuration option than a description of the hardware. Wouldn't it be better for the LVDS sink to report which of the odd or even pixels it expects on each of its endpoints ?
Yes, that would be my preference too, and it would be better, I am just not entirely what's the best place for this information though
The LVDS encoder driver could then query that at runtime and configure itself accordingly. Ideally this should be queried through the drm_bridge_timings structure (or through a similar mean), not through DT. An LVDS sink that has a fixed mapping of odd/even pixels to endpoints wouldn't need the information to be specified in DT at all.
Isn't drm_bridge_timings specific for bridges?
Thanks! Fab
Example:
-- Regards,
Laurent Pinchart
Hi Fabrizio,
On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 08:44 Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
R-Car D3, R-Car E3, and RZ/G2E support dual-link mode. In such a mode, the first LVDS encoder emits even data, and the second LVDS encoder emits odd data. This patch documents property renesas,swap-data, used to swap even and odd data around.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index dece79e..8980179 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -52,6 +52,11 @@ Optional properties: mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, and shall point to the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
- emits even data, and the second LVDS encoder emits odd data. When property
- renesas,swap-data is specified, the data emitted by the two encoders will be
- swapped around. This property can only be used in conjunction with property
- renesas,companion.
From an LVDS encoder point of view this is more a configuration option than a description of the hardware. Wouldn't it be better for the LVDS sink to report which of the odd or even pixels it expects on each of its endpoints ?
Yes, that would be my preference too, and it would be better, I am just not entirely what's the best place for this information though
The LVDS encoder driver could then query that at runtime and configure itself accordingly. Ideally this should be queried through the drm_bridge_timings structure (or through a similar mean), not through DT. An LVDS sink that has a fixed mapping of odd/even pixels to endpoints wouldn't need the information to be specified in DT at all.
Isn't drm_bridge_timings specific for bridges?
Its name makes it specific to bridges, but the information it contains could equally apply to panels. I would thus use it for both, possibly after renaming it.
Hi Laurent,
From: linux-kernel-owner@vger.kernel.org linux-kernel-owner@vger.kernel.org On Behalf Of Laurent Pinchart Sent: 05 August 2019 10:36 Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
Hi Fabrizio,
On Mon, Aug 05, 2019 at 08:59:51AM +0000, Fabrizio Castro wrote:
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 08:44 Subject: Re: [PATCH/RFC 02/12] dt-bindings: display: renesas: lvds: Document renesas,swap-data
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:33:59AM +0100, Fabrizio Castro wrote:
R-Car D3, R-Car E3, and RZ/G2E support dual-link mode. In such a mode, the first LVDS encoder emits even data, and the second LVDS encoder emits odd data. This patch documents property renesas,swap-data, used to swap even and odd data around.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index dece79e..8980179 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -52,6 +52,11 @@ Optional properties: mandatory for the first LVDS encoder on R-Car D3, R-Car E3, and RZ/G2E SoCs, and shall point to the second encoder to be used as a companion in dual-link mode. It shall not be set for any other LVDS encoder. +- renesas,swap-data : when in dual-link mode, the first LVDS encoder normally
- emits even data, and the second LVDS encoder emits odd data. When property
- renesas,swap-data is specified, the data emitted by the two encoders will be
- swapped around. This property can only be used in conjunction with property
- renesas,companion.
From an LVDS encoder point of view this is more a configuration option than a description of the hardware. Wouldn't it be better for the LVDS sink to report which of the odd or even pixels it expects on each of its endpoints ?
Yes, that would be my preference too, and it would be better, I am just not entirely what's the best place for this information though
The LVDS encoder driver could then query that at runtime and configure itself accordingly. Ideally this should be queried through the drm_bridge_timings structure (or through a similar mean), not through DT. An LVDS sink that has a fixed mapping of odd/even pixels to endpoints wouldn't need the information to be specified in DT at all.
Isn't drm_bridge_timings specific for bridges?
Its name makes it specific to bridges, but the information it contains could equally apply to panels. I would thus use it for both, possibly after renaming it.
Will give this a try then.
Thanks, Fab
-- Regards,
Laurent Pinchart
Dual-link LVDS displays have two ports, therefore document this with the bindings.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- .../bindings/display/panel/panel-lvds.txt | 91 ++++++++++++++++------ 1 file changed, 67 insertions(+), 24 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt index 250850a..07795441 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt @@ -41,7 +41,8 @@ Required nodes:
- panel-timing: See panel-common.txt. - ports: See panel-common.txt. These bindings require a single port subnode - corresponding to the panel LVDS input. + (for a single link display) or two port subnodes (for a dual link display) + corresponding to the panel LVDS input(s).
LVDS data mappings are defined as follows. @@ -92,30 +93,72 @@ CTL3: 0 Example -------
-panel { - compatible = "mitsubishi,aa121td01", "panel-lvds"; - - width-mm = <261>; - height-mm = <163>; - - data-mapping = "jeida-24"; - - panel-timing { - /* 1280x800 @60Hz */ - clock-frequency = <71000000>; - hactive = <1280>; - vactive = <800>; - hsync-len = <70>; - hfront-porch = <20>; - hback-porch = <70>; - vsync-len = <5>; - vfront-porch = <3>; - vback-porch = <15>; +Single port: + panel { + compatible = "mitsubishi,aa121td01", "panel-lvds"; + + width-mm = <261>; + height-mm = <163>; + + data-mapping = "jeida-24"; + + panel-timing { + /* 1280x800 @60Hz */ + clock-frequency = <71000000>; + hactive = <1280>; + vactive = <800>; + hsync-len = <70>; + hfront-porch = <20>; + hback-porch = <70>; + vsync-len = <5>; + vfront-porch = <3>; + vback-porch = <15>; + }; + + port { + panel_in: endpoint { + remote-endpoint = <&lvds_encoder>; + }; + }; };
- port { - panel_in: endpoint { - remote-endpoint = <&lvds_encoder>; +Two ports: + panel { + compatible = "advantech,idk-2121wr", "panel-lvds"; + + width-mm = <476>; + height-mm = <268>; + + data-mapping = "vesa-24"; + + panel-timing { + clock-frequency = <148500000>; + hactive = <1920>; + vactive = <1080>; + hsync-len = <44>; + hfront-porch = <88>; + hback-porch = <148>; + vfront-porch = <4>; + vback-porch = <36>; + vsync-len = <5>; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds0_panel_in: endpoint { + remote-endpoint = <&lvds0_out>; + }; + }; + + port@1 { + reg = <1>; + lvds1_panel_in: endpoint { + remote-endpoint = <&lvds1_out>; + }; + }; }; }; -};
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:00AM +0100, Fabrizio Castro wrote:
Dual-link LVDS displays have two ports, therefore document this with the bindings.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
.../bindings/display/panel/panel-lvds.txt | 91 ++++++++++++++++------ 1 file changed, 67 insertions(+), 24 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt index 250850a..07795441 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt @@ -41,7 +41,8 @@ Required nodes:
- panel-timing: See panel-common.txt.
- ports: See panel-common.txt. These bindings require a single port subnode
- corresponding to the panel LVDS input.
- (for a single link display) or two port subnodes (for a dual link display)
- corresponding to the panel LVDS input(s).
I think you should expand this a bit to explain what the ports correspond to in the dual link mode.
LVDS data mappings are defined as follows. @@ -92,30 +93,72 @@ CTL3: 0 Example
-panel {
- compatible = "mitsubishi,aa121td01", "panel-lvds";
- width-mm = <261>;
- height-mm = <163>;
- data-mapping = "jeida-24";
- panel-timing {
/* 1280x800 @60Hz */clock-frequency = <71000000>;hactive = <1280>;vactive = <800>;hsync-len = <70>;hfront-porch = <20>;hback-porch = <70>;vsync-len = <5>;vfront-porch = <3>;vback-porch = <15>;+Single port:
- panel {
compatible = "mitsubishi,aa121td01", "panel-lvds";width-mm = <261>;height-mm = <163>;data-mapping = "jeida-24";panel-timing {/* 1280x800 @60Hz */clock-frequency = <71000000>;hactive = <1280>;vactive = <800>;hsync-len = <70>;hfront-porch = <20>;hback-porch = <70>;vsync-len = <5>;vfront-porch = <3>;vback-porch = <15>;};port {panel_in: endpoint {remote-endpoint = <&lvds_encoder>;};};
- port {
panel_in: endpoint {remote-endpoint = <&lvds_encoder>;+Two ports:
- panel {
compatible = "advantech,idk-2121wr", "panel-lvds";width-mm = <476>;height-mm = <268>;data-mapping = "vesa-24";panel-timing {clock-frequency = <148500000>;hactive = <1920>;vactive = <1080>;hsync-len = <44>;hfront-porch = <88>;hback-porch = <148>;vfront-porch = <4>;vback-porch = <36>;vsync-len = <5>;};ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lvds0_panel_in: endpoint {
I would name the label panel_in0 and panel_in1 below to have a common prefix showing that both refer to the same panel.
remote-endpoint = <&lvds0_out>;};};port@1 {reg = <1>;lvds1_panel_in: endpoint {remote-endpoint = <&lvds1_out>;};};-};
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:00 Subject: Re: [PATCH/RFC 03/12] dt-bindings: panel: lvds: Add dual-link LVDS display support
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:00AM +0100, Fabrizio Castro wrote:
Dual-link LVDS displays have two ports, therefore document this with the bindings.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
.../bindings/display/panel/panel-lvds.txt | 91 ++++++++++++++++------ 1 file changed, 67 insertions(+), 24 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
index 250850a..07795441 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt @@ -41,7 +41,8 @@ Required nodes:
- panel-timing: See panel-common.txt.
- ports: See panel-common.txt. These bindings require a single port subnode
- corresponding to the panel LVDS input.
- (for a single link display) or two port subnodes (for a dual link display)
- corresponding to the panel LVDS input(s).
I think you should expand this a bit to explain what the ports correspond to in the dual link mode.
Will change.
LVDS data mappings are defined as follows. @@ -92,30 +93,72 @@ CTL3: 0 Example
-panel {
- compatible = "mitsubishi,aa121td01", "panel-lvds";
- width-mm = <261>;
- height-mm = <163>;
- data-mapping = "jeida-24";
- panel-timing {
/* 1280x800 @60Hz */clock-frequency = <71000000>;hactive = <1280>;vactive = <800>;hsync-len = <70>;hfront-porch = <20>;hback-porch = <70>;vsync-len = <5>;vfront-porch = <3>;vback-porch = <15>;+Single port:
- panel {
compatible = "mitsubishi,aa121td01", "panel-lvds";width-mm = <261>;height-mm = <163>;data-mapping = "jeida-24";panel-timing {/* 1280x800 @60Hz */clock-frequency = <71000000>;hactive = <1280>;vactive = <800>;hsync-len = <70>;hfront-porch = <20>;hback-porch = <70>;vsync-len = <5>;vfront-porch = <3>;vback-porch = <15>;};port {panel_in: endpoint {remote-endpoint = <&lvds_encoder>;};};
- port {
panel_in: endpoint {remote-endpoint = <&lvds_encoder>;+Two ports:
- panel {
compatible = "advantech,idk-2121wr", "panel-lvds";width-mm = <476>;height-mm = <268>;data-mapping = "vesa-24";panel-timing {clock-frequency = <148500000>;hactive = <1920>;vactive = <1080>;hsync-len = <44>;hfront-porch = <88>;hback-porch = <148>;vfront-porch = <4>;vback-porch = <36>;vsync-len = <5>;};ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lvds0_panel_in: endpoint {I would name the label panel_in0 and panel_in1 below to have a common prefix showing that both refer to the same panel.
I agree, will change, thank you for pointing this out.
remote-endpoint = <&lvds0_out>;};};port@1 {reg = <1>;lvds1_panel_in: endpoint {remote-endpoint = <&lvds1_out>;};};-};
Thanks, Fab
-- Regards,
Laurent Pinchart
This panel is handled through the generic lvds-panel bindings, so only needs its additional compatible specified.
Some panel specific documentation can be found here: https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-ID...
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- .../display/panel/advantech,idk-2121wr.txt | 62 ++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt new file mode 100644 index 0000000..70b15b6 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt @@ -0,0 +1,62 @@ +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel +=============================================== + +Required properties: +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds" + +This binding is compatible with the lvds-panel binding, which is specified +in panel-lvds.txt in this directory. + +Example +------- + + panel { + compatible = "advantech,idk-2121wr", "panel-lvds"; + + width-mm = <476>; + height-mm = <268>; + + data-mapping = "vesa-24"; + + panel-timing { + clock-frequency = <148500000>; + hactive = <1920>; + vactive = <1080>; + hsync-len = <44>; + hfront-porch = <88>; + hback-porch = <148>; + vfront-porch = <4>; + vback-porch = <36>; + vsync-len = <5>; + }; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds0_panel_in: endpoint { + remote-endpoint = <&lvds0_out>; + }; + }; + + port@1 { + reg = <1>; + lvds1_panel_in: endpoint { + remote-endpoint = <&lvds1_out>; + }; + }; + }; + }; + + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&pwm5 0 50000>; + + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <6>; + + power-supply = <®_12p0v>; + enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>; + };
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:01AM +0100, Fabrizio Castro wrote:
This panel is handled through the generic lvds-panel bindings, so only needs its additional compatible specified.
Some panel specific documentation can be found here:
s/panel specific/panel-specific/
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-ID...
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
.../display/panel/advantech,idk-2121wr.txt | 62 ++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt new file mode 100644 index 0000000..70b15b6 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt @@ -0,0 +1,62 @@ +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel +===============================================
+Required properties: +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
+This binding is compatible with the lvds-panel binding, which is specified +in panel-lvds.txt in this directory.
How about adding "The panel operates in dual-link mode and thus requires two port nodes." ?
+Example +-------
- panel {
compatible = "advantech,idk-2121wr", "panel-lvds";width-mm = <476>;height-mm = <268>;data-mapping = "vesa-24";panel-timing {clock-frequency = <148500000>;hactive = <1920>;vactive = <1080>;hsync-len = <44>;hfront-porch = <88>;hback-porch = <148>;vfront-porch = <4>;vback-porch = <36>;vsync-len = <5>;};ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lvds0_panel_in: endpoint {remote-endpoint = <&lvds0_out>;};};port@1 {reg = <1>;lvds1_panel_in: endpoint {remote-endpoint = <&lvds1_out>;};};};- };
- backlight: backlight {
compatible = "pwm-backlight";pwms = <&pwm5 0 50000>;brightness-levels = <0 4 8 16 32 64 128 255>;default-brightness-level = <6>;power-supply = <®_12p0v>;enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;- };
I think you can drop the backlight here, it's a bit out of scope.
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:04 Subject: Re: [PATCH/RFC 04/12] dt-bindings: display: Add bindings for Advantech IDK-2121WR
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:01AM +0100, Fabrizio Castro wrote:
This panel is handled through the generic lvds-panel bindings, so only needs its additional compatible specified.
Some panel specific documentation can be found here:
s/panel specific/panel-specific/
Will change
https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-ID...
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
.../display/panel/advantech,idk-2121wr.txt | 62 ++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
diff --git a/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt
new file mode 100644 index 0000000..70b15b6 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt @@ -0,0 +1,62 @@ +Advantech Co., Ltd. IDK-2121WR 21.5" LVDS panel +===============================================
+Required properties: +- compatible: should be "advantech,idk-2121wr" followed by "panel-lvds"
+This binding is compatible with the lvds-panel binding, which is specified +in panel-lvds.txt in this directory.
How about adding "The panel operates in dual-link mode and thus requires two port nodes." ?
You are right, will add.
+Example +-------
- panel {
compatible = "advantech,idk-2121wr", "panel-lvds";width-mm = <476>;height-mm = <268>;data-mapping = "vesa-24";panel-timing {clock-frequency = <148500000>;hactive = <1920>;vactive = <1080>;hsync-len = <44>;hfront-porch = <88>;hback-porch = <148>;vfront-porch = <4>;vback-porch = <36>;vsync-len = <5>;};ports {#address-cells = <1>;#size-cells = <0>;port@0 {reg = <0>;lvds0_panel_in: endpoint {remote-endpoint = <&lvds0_out>;};};port@1 {reg = <1>;lvds1_panel_in: endpoint {remote-endpoint = <&lvds1_out>;};};};- };
- backlight: backlight {
compatible = "pwm-backlight";pwms = <&pwm5 0 50000>;brightness-levels = <0 4 8 16 32 64 128 255>;default-brightness-level = <6>;power-supply = <®_12p0v>;enable-gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;- };
I think you can drop the backlight here, it's a bit out of scope.
Agreed.
Thanks, Fab
-- Regards,
Laurent Pinchart
When in vertical stripe mode of operation, there is the option of swapping even data and odd data on the two LVDS interfaces used to drive the video output. Add data swap support by exposing a new DT property named "renesas,swap-data".
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3aeaf9e..c306fab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -69,6 +69,7 @@ struct rcar_lvds {
struct drm_bridge *companion; bool dual_link; + bool stripe_swap_data; };
#define bridge_to_rcar_lvds(bridge) \ @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { - /* - * Configure vertical stripe based on the mode of operation of - * the connected device. - */ - rcar_lvds_write(lvds, LVDSTRIPE, - lvds->dual_link ? LVDSTRIPE_ST_ON : 0); + u32 lvdstripe = 0; + + if (lvds->dual_link) + /* + * Configure vertical stripe based on the mode of + * operation of the connected device. + */ + lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ? + LVDSTRIPE_ST_SWAP : 0); + rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe); }
/* @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } }
- if (lvds->dual_link) + if (lvds->dual_link) { + lvds->stripe_swap_data = of_property_read_bool( + lvds->dev->of_node, + "renesas,swap-data"); ret = rcar_lvds_parse_dt_companion(lvds); + }
done: of_node_put(local_output);
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
When in vertical stripe mode of operation, there is the option of swapping even data and odd data on the two LVDS interfaces used to drive the video output. Add data swap support by exposing a new DT property named "renesas,swap-data".
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3aeaf9e..c306fab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -69,6 +69,7 @@ struct rcar_lvds {
struct drm_bridge *companion; bool dual_link;
- bool stripe_swap_data;
};
#define bridge_to_rcar_lvds(bridge) \ @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/** Configure vertical stripe based on the mode of operation of* the connected device.*/rcar_lvds_write(lvds, LVDSTRIPE,lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
u32 lvdstripe = 0;if (lvds->dual_link)/** Configure vertical stripe based on the mode of* operation of the connected device.*/lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?LVDSTRIPE_ST_SWAP : 0);
Would the following be simpler ?
lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0) | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);}
/*
@@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } }
- if (lvds->dual_link)
- if (lvds->dual_link) {
lvds->stripe_swap_data = of_property_read_bool(lvds->dev->of_node,"renesas,swap-data");- }
As explained in the review of the corresponding DT bindings, I think this should be queried from the remote device rather than specified in DT.
done: of_node_put(local_output);
Hi Laurent,
On Fri, Aug 2, 2019 at 10:06 AM Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
When in vertical stripe mode of operation, there is the option of swapping even data and odd data on the two LVDS interfaces used to drive the video output. Add data swap support by exposing a new DT property named "renesas,swap-data".
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/** Configure vertical stripe based on the mode of operation of* the connected device.*/rcar_lvds_write(lvds, LVDSTRIPE,lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
u32 lvdstripe = 0;if (lvds->dual_link)/** Configure vertical stripe based on the mode of* operation of the connected device.*/lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?LVDSTRIPE_ST_SWAP : 0);Would the following be simpler ?
lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0) | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
From the point of view of "wc -l": yes. From the point of view of readability, I'd go for:
if (lvds->dual_link) lvdstripe |= LVDSTRIPE_ST_ON; if (lvds->stripe_swap_data) lvdstripe |= LVDSTRIPE_ST_SWAP;
rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe); } /*@@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
Gr{oetje,eeting}s,
Geert
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:06 Subject: Re: [PATCH/RFC 05/12] drm: rcar-du: lvds: Add data swap support
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
When in vertical stripe mode of operation, there is the option of swapping even data and odd data on the two LVDS interfaces used to drive the video output. Add data swap support by exposing a new DT property named "renesas,swap-data".
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3aeaf9e..c306fab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -69,6 +69,7 @@ struct rcar_lvds {
struct drm_bridge *companion; bool dual_link;
- bool stripe_swap_data;
};
#define bridge_to_rcar_lvds(bridge) \ @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/** Configure vertical stripe based on the mode of operation of* the connected device.*/rcar_lvds_write(lvds, LVDSTRIPE,lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
u32 lvdstripe = 0;if (lvds->dual_link)/** Configure vertical stripe based on the mode of* operation of the connected device.*/lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?LVDSTRIPE_ST_SWAP : 0);Would the following be simpler ?
lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0) | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
I actually think I need to rework this patch slightly, because the user manual says that ST_SWAP is reserved for LVD1STRIPE, so I need to make sure we don't set it for LVDS1.
So perhaps, this could translate to something like: If (lvds->dual_link) lvdstripe = LVDSTRIPE_ST_ON | (<swap-whatever> && lvds->companion) ? LVDSTRIPE_ST_SWAP : 0);
I don't think we should be setting LVDSTRIPE_ST_SWAP without LVDSTRIPE_ST_ON, this solution would allow us to test lvds->dual_link only once, and will prevent us from setting LVDSTRIPE_ST_SWAP if LVDSTRIPE_ST_ON is not set or if we are touching LVDS1.
What do you think?
Thanks, Fab
}
/* @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } }
- if (lvds->dual_link)
- if (lvds->dual_link) {
lvds->stripe_swap_data = of_property_read_bool(lvds->dev->of_node,"renesas,swap-data");- }
As explained in the review of the corresponding DT bindings, I think this should be queried from the remote device rather than specified in DT.
done: of_node_put(local_output);
-- Regards,
Laurent Pinchart
Hi Fabrizio,
On Mon, Aug 05, 2019 at 09:32:42AM +0000, Fabrizio Castro wrote:
On 02 August 2019 09:06 Laurent Pinchart wrote:
On Fri, Aug 02, 2019 at 08:34:02AM +0100, Fabrizio Castro wrote:
When in vertical stripe mode of operation, there is the option of swapping even data and odd data on the two LVDS interfaces used to drive the video output. Add data swap support by exposing a new DT property named "renesas,swap-data".
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3aeaf9e..c306fab 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -69,6 +69,7 @@ struct rcar_lvds {
struct drm_bridge *companion; bool dual_link;
- bool stripe_swap_data;
};
#define bridge_to_rcar_lvds(bridge) \ @@ -439,12 +440,16 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/** Configure vertical stripe based on the mode of operation of* the connected device.*/rcar_lvds_write(lvds, LVDSTRIPE,lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
u32 lvdstripe = 0;if (lvds->dual_link)/** Configure vertical stripe based on the mode of* operation of the connected device.*/lvdstripe = LVDSTRIPE_ST_ON | (lvds->stripe_swap_data ?LVDSTRIPE_ST_SWAP : 0);Would the following be simpler ?
lvdstripe = (lvds->dual_link ? LVDSTRIPE_ST_ON : 0) | (lvds->stripe_swap_data ? LVDSTRIPE_ST_SWAP : 0);
rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);I actually think I need to rework this patch slightly, because the user manual says that ST_SWAP is reserved for LVD1STRIPE, so I need to make sure we don't set it for LVDS1.
So perhaps, this could translate to something like: If (lvds->dual_link) lvdstripe = LVDSTRIPE_ST_ON | (<swap-whatever> && lvds->companion) ? LVDSTRIPE_ST_SWAP : 0);
I don't think we should be setting LVDSTRIPE_ST_SWAP without LVDSTRIPE_ST_ON, this solution would allow us to test lvds->dual_link only once, and will prevent us from setting LVDSTRIPE_ST_SWAP if LVDSTRIPE_ST_ON is not set or if we are touching LVDS1.
What do you think?
I was thinking that lvds->stripe_swap_data should only be set when lvds->dual_link is set and lvds->companion is non-NULL, so both are roughly equivalent. It's a detail anyway, it doesn't matter too much.
}
/* @@ -770,8 +775,12 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } }
- if (lvds->dual_link)
- if (lvds->dual_link) {
lvds->stripe_swap_data = of_property_read_bool(lvds->dev->of_node,"renesas,swap-data");- }
As explained in the review of the corresponding DT bindings, I think this should be queried from the remote device rather than specified in DT.
done: of_node_put(local_output);
We may be connected to a dual LVDS display, therefore checking if node != remote_input for identifying bridges is not going to work anymore. We could try to match the ports on the remote end to the LVDS encoders, however the companion LVDS encoder instance doesn't hold a reference to the primary LVDS encoder instance. We know we could be connected to either a bridge, or a panel, therefore look through the registered bridges and panels, until we have a match.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index c306fab..2d54ae5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL; - struct device_node *remote_input = NULL; struct device_node *remote = NULL; - struct device_node *node; - bool is_bridge = false; int ret = 0;
local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0); @@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) goto done; }
- remote_input = of_graph_get_remote_endpoint(local_output); - - for_each_endpoint_of_node(remote, node) { - if (node != remote_input) { - /* - * We've found one endpoint other than the input, this - * must be a bridge. - */ - is_bridge = true; - of_node_put(node); - break; - } - } - - if (is_bridge) { - lvds->next_bridge = of_drm_find_bridge(remote); - if (!lvds->next_bridge) { - ret = -EPROBE_DEFER; - goto done; - } - + lvds->next_bridge = of_drm_find_bridge(remote); + if (lvds->next_bridge) { if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) lvds->dual_link = lvds->next_bridge->timings ? lvds->next_bridge->timings->dual_link @@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } else { lvds->panel = of_drm_find_panel(remote); if (IS_ERR(lvds->panel)) { - ret = PTR_ERR(lvds->panel); + ret = -EPROBE_DEFER; goto done; } } @@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
done: of_node_put(local_output); - of_node_put(remote_input); of_node_put(remote);
switch (ret) {
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:03AM +0100, Fabrizio Castro wrote:
We may be connected to a dual LVDS display, therefore checking if node != remote_input for identifying bridges is not going to work anymore. We could try to match the ports on the remote end to the LVDS encoders, however the companion LVDS encoder instance doesn't hold a reference to the primary LVDS encoder instance. We know we could be connected to either a bridge, or a panel, therefore look through the registered bridges and panels, until we have a match.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index c306fab..2d54ae5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL;
struct device_node *remote_input = NULL; struct device_node *remote = NULL;
struct device_node *node;
bool is_bridge = false; int ret = 0;
local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
@@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) goto done; }
- remote_input = of_graph_get_remote_endpoint(local_output);
- for_each_endpoint_of_node(remote, node) {
if (node != remote_input) {/** We've found one endpoint other than the input, this* must be a bridge.*/is_bridge = true;of_node_put(node);break;}- }
- if (is_bridge) {
lvds->next_bridge = of_drm_find_bridge(remote);if (!lvds->next_bridge) {ret = -EPROBE_DEFER;goto done;}
- lvds->next_bridge = of_drm_find_bridge(remote);
How about using drm_of_find_panel_or_bridge() ?
- if (lvds->next_bridge) { if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) lvds->dual_link = lvds->next_bridge->timings ? lvds->next_bridge->timings->dual_link
@@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } else { lvds->panel = of_drm_find_panel(remote); if (IS_ERR(lvds->panel)) {
ret = PTR_ERR(lvds->panel);
ret = -EPROBE_DEFER; goto done;@@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
done: of_node_put(local_output);
of_node_put(remote_input); of_node_put(remote);
switch (ret) {
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:08 Subject: Re: [PATCH/RFC 06/12] drm: rcar-du: lvds: Do not look at ports for identifying bridges
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:03AM +0100, Fabrizio Castro wrote:
We may be connected to a dual LVDS display, therefore checking if node != remote_input for identifying bridges is not going to work anymore. We could try to match the ports on the remote end to the LVDS encoders, however the companion LVDS encoder instance doesn't hold a reference to the primary LVDS encoder instance. We know we could be connected to either a bridge, or a panel, therefore look through the registered bridges and panels, until we have a match.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index c306fab..2d54ae5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -711,10 +711,7 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL;
struct device_node *remote_input = NULL; struct device_node *remote = NULL;
struct device_node *node;
bool is_bridge = false; int ret = 0;
local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);
@@ -742,27 +739,8 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) goto done; }
- remote_input = of_graph_get_remote_endpoint(local_output);
- for_each_endpoint_of_node(remote, node) {
if (node != remote_input) {/** We've found one endpoint other than the input, this* must be a bridge.*/is_bridge = true;of_node_put(node);break;}- }
- if (is_bridge) {
lvds->next_bridge = of_drm_find_bridge(remote);if (!lvds->next_bridge) {ret = -EPROBE_DEFER;goto done;}
- lvds->next_bridge = of_drm_find_bridge(remote);
How about using drm_of_find_panel_or_bridge() ?
It sounds like drm_of_find_panel_or_bridge is exactly what we need here, I'll give it a try, thank you for the pointer!
Thanks, Fab
- if (lvds->next_bridge) { if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) lvds->dual_link = lvds->next_bridge->timings ? lvds->next_bridge->timings->dual_link
@@ -770,7 +748,7 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) } else { lvds->panel = of_drm_find_panel(remote); if (IS_ERR(lvds->panel)) {
ret = PTR_ERR(lvds->panel);
ret = -EPROBE_DEFER; goto done;@@ -784,7 +762,6 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
done: of_node_put(local_output);
of_node_put(remote_input); of_node_put(remote);
switch (ret) {
-- Regards,
Laurent Pinchart
If the display comes with two ports, assume it supports dual link.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 2d54ae5..97c51c2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; goto done; } + if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) + lvds->dual_link = of_graph_get_endpoint_count(remote) + == 2; }
if (lvds->dual_link) {
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
If the display comes with two ports, assume it supports dual link.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 2d54ae5..97c51c2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; goto done; }
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = of_graph_get_endpoint_count(remote)== 2;
This is a bit of a hack, as I think the information should be queried from the panel, like we do for bridges. I'd say we can live with this for now, but as the data swap flag should come from the panel as well, we will need infrastructure for that, and we can as well through the dual link flag there at the same time.
I think we should use the drm_bridge_timings structure for this purpose, as it would make life more difficult for users of drm_bridge and drm_panel to have two different structures (especially when wrapping a drm_panel with drm_panel_bridge_add()). The structure could be renamed if desired.
}
if (lvds->dual_link) {
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:20 Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
If the display comes with two ports, assume it supports dual link.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 2d54ae5..97c51c2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; goto done; }
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = of_graph_get_endpoint_count(remote)== 2;This is a bit of a hack, as I think the information should be queried from the panel, like we do for bridges. I'd say we can live with this for now, but as the data swap flag should come from the panel as well, we will need infrastructure for that, and we can as well through the dual link flag there at the same time.
I totally agree, this is a nasty hack, my series is missing the infrastructure for describing this information
I think we should use the drm_bridge_timings structure for this purpose, as it would make life more difficult for users of drm_bridge and drm_panel to have two different structures (especially when wrapping a drm_panel with drm_panel_bridge_add()). The structure could be renamed if desired.
I am not too sure using drm_bridge_timings for panels would make everybody happy? Is there any alternative? Perhaps this calls for a new struct we could embed in both drm_bridge_timings and some drm_panel_<whatever> data structure?
Thanks, Fab
}
if (lvds->dual_link) {
-- Regards,
Laurent Pinchart
Hi Fabrizio,
On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:20 Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
If the display comes with two ports, assume it supports dual link.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 2d54ae5..97c51c2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; goto done; }
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = of_graph_get_endpoint_count(remote)== 2;This is a bit of a hack, as I think the information should be queried from the panel, like we do for bridges. I'd say we can live with this for now, but as the data swap flag should come from the panel as well, we will need infrastructure for that, and we can as well through the dual link flag there at the same time.
I totally agree, this is a nasty hack, my series is missing the infrastructure for describing this information
I think we should use the drm_bridge_timings structure for this purpose, as it would make life more difficult for users of drm_bridge and drm_panel to have two different structures (especially when wrapping a drm_panel with drm_panel_bridge_add()). The structure could be renamed if desired.
I am not too sure using drm_bridge_timings for panels would make everybody happy? Is there any alternative? Perhaps this calls for a new struct we could embed in both drm_bridge_timings and some drm_panel_<whatever> data structure?
I think we could simply rename the structure, all its fields apply to panels too.
}
if (lvds->dual_link) {
Hi Laurent,
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 05 August 2019 10:49 Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
Hi Fabrizio,
On Mon, Aug 05, 2019 at 09:12:34AM +0000, Fabrizio Castro wrote:
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:20 Subject: Re: [PATCH/RFC 07/12] drm: rcar-du: lvds: Add support for dual link panels
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:04AM +0100, Fabrizio Castro wrote:
If the display comes with two ports, assume it supports dual link.
Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 2d54ae5..97c51c2 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -751,6 +751,9 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) ret = -EPROBE_DEFER; goto done; }
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = of_graph_get_endpoint_count(remote)== 2;This is a bit of a hack, as I think the information should be queried from the panel, like we do for bridges. I'd say we can live with this for now, but as the data swap flag should come from the panel as well, we will need infrastructure for that, and we can as well through the dual link flag there at the same time.
I totally agree, this is a nasty hack, my series is missing the infrastructure for describing this information
I think we should use the drm_bridge_timings structure for this purpose, as it would make life more difficult for users of drm_bridge and drm_panel to have two different structures (especially when wrapping a drm_panel with drm_panel_bridge_add()). The structure could be renamed if desired.
I am not too sure using drm_bridge_timings for panels would make everybody happy? Is there any alternative? Perhaps this calls for a new struct we could embed in both drm_bridge_timings and some drm_panel_<whatever> data structure?
I think we could simply rename the structure, all its fields apply to panels too.
Ok, will give this a try.
Thanks, Fab
}
if (lvds->dual_link) {
-- Regards,
Laurent Pinchart
Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't work when the pointer name used by the caller is not "bridge". Rename the argument to "bridge_ptr" to allow for any pointer name.
Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 97c51c2..edd63f5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -72,8 +72,8 @@ struct rcar_lvds { bool stripe_swap_data; };
-#define bridge_to_rcar_lvds(bridge) \ - container_of(bridge, struct rcar_lvds, bridge) +#define bridge_to_rcar_lvds(bridge_ptr) \ + container_of(bridge_ptr, struct rcar_lvds, bridge)
#define connector_to_rcar_lvds(connector) \ container_of(connector, struct rcar_lvds, connector)
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:05AM +0100, Fabrizio Castro wrote:
Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't work when the pointer name used by the caller is not "bridge". Rename the argument to "bridge_ptr" to allow for any pointer name.
Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 97c51c2..edd63f5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -72,8 +72,8 @@ struct rcar_lvds { bool stripe_swap_data; };
-#define bridge_to_rcar_lvds(bridge) \
- container_of(bridge, struct rcar_lvds, bridge)
+#define bridge_to_rcar_lvds(bridge_ptr) \
- container_of(bridge_ptr, struct rcar_lvds, bridge)
How about just 'b' instead of 'bridge_ptr' ? If that's fine with you I'll take the modified patch in my tree, no need to resubmit.
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
#define connector_to_rcar_lvds(connector) \ container_of(connector, struct rcar_lvds, connector)
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:22 Subject: Re: [PATCH/RFC 08/12] drm: rcar-du: lvds: Fix bridge_to_rcar_lvds
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:05AM +0100, Fabrizio Castro wrote:
Using name "bridge" for macro bridge_to_rcar_lvds argument doesn't work when the pointer name used by the caller is not "bridge". Rename the argument to "bridge_ptr" to allow for any pointer name.
Fixes: c6a27fa41fab ("drm: rcar-du: Convert LVDS encoder code to bridge driver") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 97c51c2..edd63f5 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -72,8 +72,8 @@ struct rcar_lvds { bool stripe_swap_data; };
-#define bridge_to_rcar_lvds(bridge) \
- container_of(bridge, struct rcar_lvds, bridge)
+#define bridge_to_rcar_lvds(bridge_ptr) \
- container_of(bridge_ptr, struct rcar_lvds, bridge)
How about just 'b' instead of 'bridge_ptr' ? If that's fine with you I'll take the modified patch in my tree, no need to resubmit.
That's fine by me, thank your fixing.
Thanks, Fab
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
#define connector_to_rcar_lvds(connector) \ container_of(connector, struct rcar_lvds, connector)
-- Regards,
Laurent Pinchart
The companion encoder needs to be told to use the same mode as the primary encoder.
Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index edd63f5..7944ae9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) return;
/* Enable the companion LVDS encoder in dual-link mode. */ - if (lvds->dual_link && lvds->companion) + if (lvds->dual_link && lvds->companion) { + struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds( + lvds->companion); + companion_lvds->mode = lvds->mode; lvds->companion->funcs->enable(lvds->companion); + }
/* * Hardcode the channels and control signals routing for now.
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:06AM +0100, Fabrizio Castro wrote:
The companion encoder needs to be told to use the same mode as the primary encoder.
Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index edd63f5..7944ae9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) return;
/* Enable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
- if (lvds->dual_link && lvds->companion) {
struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(lvds->companion);companion_lvds->mode = lvds->mode;- }
Would it make sense to do this in rcar_lvds_mode_set() instead, to keep the mode set code grouped in a single place ?
/* * Hardcode the channels and control signals routing for now.
Hi Laurent,
Thank you for your feedback!
From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 02 August 2019 09:26 Subject: Re: [PATCH/RFC 09/12] drm: rcar-du: lvds: Fix companion's mode
Hi Fabrizio,
Thank you for the patch.
On Fri, Aug 02, 2019 at 08:34:06AM +0100, Fabrizio Castro wrote:
The companion encoder needs to be told to use the same mode as the primary encoder.
Fixes: e9e8798ab7b8 ("drm: rcar-du: lvds: Add support for dual-link mode") Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com
drivers/gpu/drm/rcar-du/rcar_lvds.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index edd63f5..7944ae9 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -415,8 +415,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) return;
/* Enable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
- if (lvds->dual_link && lvds->companion) {
struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(lvds->companion);companion_lvds->mode = lvds->mode;- }
Would it make sense to do this in rcar_lvds_mode_set() instead, to keep the mode set code grouped in a single place ?
I need to looks into this, as the initialization of lvds1 is a bit special. I'll get back to you.
Thanks, Fab
/* * Hardcode the channels and control signals routing for now.
-- Regards,
Laurent Pinchart
dri-devel@lists.freedesktop.org
-
Fabrizio Castro -
Geert Uytterhoeven -
Laurent Pinchart