Hello everybody,
This patch series implements support for LVDS dual-link mode in the R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 LVDS decoder driver.
LVDS dual-link is a mode of operation where two individual LVDS links are operated together to carry even- and odd-numbered pixels separately. This doubles the possible bandwidth of the video transmission. Both the transmitter and the receiver need to support this mode of operation.
The R-Car D3 and E3 SoCs include two independent LVDS encoders that can be grouped together to operate in dual-link mode. When used separately, the LVDS encoders are connected to two different CRTCs and transmit independent video streams. When used in dual-link mode, the first LVDS encoder is connected to the first CRTC, and split even- and odd-numbered pixels. It transmits half of the pixels on its LVDS output, and sends the other half to the second LVDS encoder for transmittion over the second LVDS link. The second LVDS encoder thus operates under control of the first one, and isn't connected directly to a CRTC.
On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two LVDS inputs and two parallel outputs. It can operate in four different modes:
- Single-in, single-out: The first LVDS input receives the video stream, and the bridge outputs it on the first parallel output. The second LVDS input and the second parallel output are not used.
- Single-in, dual-out: The first LVDS input receives the video stream, and the bridge splits even- and odd-numbered pixels and outputs them on the first and second parallel outputs. The second LVDS input is not used.
- Dual-in, single-out: The two LVDS inputs are used in dual-link mode, and the bridge combines the even- and odd-numbered pixels and outputs them on the first parallel output. The second parallel output is not used.
- Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, and the bridge outputs the even- and odd-numbered pixels on the first parallel output.
The operating mode is selected by two input pins of the bridge, which are connected to DIP switches on the development boards I use. The mode is thus fixed from a Linux point of view.
Patch 01/10 adds a new dual_link boolen field to the drm_bridge_timings structure to let bridges report their LVDS mode of operation. Patch 02/10 clarifies the THC63LVD1024 DT bindings to document dual-link operation, and patch 03/10 implements dual-link support in the thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link field according to the mode selected through DT.
Patch 04/10 extends the R-Car LVDS DT bindings to specify the companion LVDS encoder for dual-link operation. Patches 05/10 then performs a small cleanup in the LVDS encoder driver. Patch 06/10 implements dual-link support in the LVDS encoder driver, which involves retrieving the operation mode from the LVDS receiver, locating the companion LVDS encoder, and configuring both encoders when dual-link operation is desired. The API towards the DU driver is also extended to report the mode of operation.
Patch 07/10 implements dual-link mode support in the DU driver. There is no specific configuration to be performed there, as dual-link is fully implemented in the LVDS encoder driver, but the DU driver has to skip creation of the DRM encoder and connector related to the second LVDS encoder when dual-link is used, as the second LVDS encoder operates as a slave of the first one, transparently from a CRTC (and thus userspace) perspective.
Patch 08/10 specifies the companion LVDS encoder in the D3 and E3 DT bindings. This by itself doesn't enable dual-link mode, the LVDS0 encoder is still connected to the HDMI output through LVDS receiver, and the LVDS1 encoder is not used. Patches 09/10 and 10/10, not intended to be merged, enable dual-link operation for the D3 and E3 boards for testing and require flipping DIP switches on the boards.
The patches are based on top of my drm/du/next branch, and are available for convenience at
git://linuxtv.org/pinchartl/media.git drm/du/lvds/dual-link
They have been tested successfully on the D3 Draak board. I expect them to work on E3 as well, but I don't have access to an Ebisu board to test this.
Laurent Pinchart (10): drm: bridge: Add dual_link field to the drm_bridge_timings structure dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation drm: bridge: thc63: Report input bus mode through bridge timings dt-bindings: display: renesas: lvds: Add renesas,companion property drm: rcar-du: lvds: Remove LVDS double-enable checks drm: rcar-du: lvds: Add support for dual-link mode drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1 [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation
.../bindings/display/bridge/renesas,lvds.txt | 6 + .../display/bridge/thine,thc63lvd1024.txt | 6 + .../arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 ++- arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 + .../arm64/boot/dts/renesas/r8a77995-draak.dts | 21 ++- arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 + drivers/gpu/drm/bridge/thc63lvd1024.c | 54 ++++++-- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 123 +++++++++++++----- drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 + include/drm/drm_bridge.h | 8 ++ 12 files changed, 214 insertions(+), 48 deletions(-)
Extend the drm_bridge_timings structure with a new dual_link field to indicate that the bridge's input bus carries data on two separate physical links. The first use case is LVDS dual-link mode where even- and odd-numbered pixels are transferred on separate LVDS links.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Kieran Bingham kieran.bingham+renesas@ideasonboard.com --- include/drm/drm_bridge.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index d4428913a4e1..aea1fcfd92a7 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -265,6 +265,14 @@ struct drm_bridge_timings { * input signal after the clock edge. */ u32 hold_time_ps; + /** + * @dual_link: + * + * True if the bus operates in dual-link mode. The exact meaning is + * dependent on the bus type. For LVDS buses, this indicates that even- + * and odd-numbered pixels are received on separate links. + */ + bool dual_link; };
/**
The THC63LVD1024 LVDS decoder can operate in two modes, single-link or dual-link. In dual-link mode both input ports are used to carry even- and odd-numbered pixels separately. Document this in the DT bindings, along with the related rules governing port and usage.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org --- .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt index 37f0c04d5a28..d17d1e5820d7 100644 --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -28,6 +28,12 @@ Optional video port nodes: - port@1: Second LVDS input port - port@3: Second digital CMOS/TTL parallel output
+The device can operate in single-link mode or dual-link mode. In single-link +mode, all pixels are received on port@0, and port@1 shall not contain any +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain +endpoints. + Example: --------
Hi Laurent,
On Sat, May 11, 2019 at 11:07 PM Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com wrote:
The THC63LVD1024 LVDS decoder can operate in two modes, single-link or dual-link. In dual-link mode both input ports are used to carry even- and odd-numbered pixels separately. Document this in the DT bindings, along with the related rules governing port and usage.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
.../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt index 37f0c04d5a28..d17d1e5820d7 100644 --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -28,6 +28,12 @@ Optional video port nodes:
- port@1: Second LVDS input port
- port@3: Second digital CMOS/TTL parallel output
+The device can operate in single-link mode or dual-link mode. In single-link +mode, all pixels are received on port@0, and port@1 shall not contain any +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain +endpoints.
This describes single/dual input. Does single/dual output need to be described, too?
BTW, I see the second input/output set is optional, wile the first set is required. Could it happen the hardware is wired for the second set only?
Gr{oetje,eeting}s,
Geert
Hi Geert,
On Sun, May 12, 2019 at 10:58:54AM +0200, Geert Uytterhoeven wrote:
On Sat, May 11, 2019 at 11:07 PM Laurent Pinchart wrote:
The THC63LVD1024 LVDS decoder can operate in two modes, single-link or dual-link. In dual-link mode both input ports are used to carry even- and odd-numbered pixels separately. Document this in the DT bindings, along with the related rules governing port and usage.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
.../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt index 37f0c04d5a28..d17d1e5820d7 100644 --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -28,6 +28,12 @@ Optional video port nodes:
- port@1: Second LVDS input port
- port@3: Second digital CMOS/TTL parallel output
+The device can operate in single-link mode or dual-link mode. In single-link +mode, all pixels are received on port@0, and port@1 shall not contain any +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain +endpoints.
This describes single/dual input. Does single/dual output need to be described, too?
Jacopo asked the same question on v1 :-) Dual-output should be described as well, but as I have no hardware setup where to test that, I decided to leave it out of the DT bindings to start with, as it's generally a bad idea to specify untested DT bindings (as in having no end-to-end implementation). I don't think it will be a big deal though, there is already a port for the second output, it should just be a matter of connecting it.
BTW, I see the second input/output set is optional, wile the first set is required. Could it happen the hardware is wired for the second set only?
Not to my knowledge. In dual-in, dual-out the two input/output pairs are not independent, the two inputs are used together to create a higher bandwidth link, and the odd- and even-pixels are then sent to separate routes.
On Sun, 12 May 2019 00:06:54 +0300, Laurent Pinchart wrote:
The THC63LVD1024 LVDS decoder can operate in two modes, single-link or dual-link. In dual-link mode both input ports are used to carry even- and odd-numbered pixels separately. Document this in the DT bindings, along with the related rules governing port and usage.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
.../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++++++ 1 file changed, 6 insertions(+)
Reviewed-by: Rob Herring robh@kernel.org
Set a drm_bridge_timings in the drm_bridge, and use it to report the input bus mode (single-link or dual-link). The other fields of the timings structure are kept to 0 as they do not apply to LVDS buses.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v1:
- Ignore disabled remote device --- drivers/gpu/drm/bridge/thc63lvd1024.c | 54 +++++++++++++++++++++------ 1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c index b083a740565c..709dd28b43d6 100644 --- a/drivers/gpu/drm/bridge/thc63lvd1024.c +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -31,6 +31,8 @@ struct thc63_dev {
struct drm_bridge bridge; struct drm_bridge *next; + + struct drm_bridge_timings timings; };
static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge) static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge, const struct drm_display_mode *mode) { + struct thc63_dev *thc63 = to_thc63(bridge); + unsigned int min_freq; + unsigned int max_freq; + /* - * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in - * mode. Note that the limits are different in dual-in, single-out mode, - * and will need to be adjusted accordingly. + * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but + * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out + * isn't supported by the driver yet, simply derive the limits from the + * input mode. */ - if (mode->clock < 8000) + if (thc63->timings.dual_link) { + min_freq = 40000; + max_freq = 150000; + } else { + min_freq = 8000; + max_freq = 135000; + } + + if (mode->clock < min_freq) return MODE_CLOCK_LOW;
- if (mode->clock > 135000) + if (mode->clock > max_freq) return MODE_CLOCK_HIGH;
return MODE_OK; @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
static int thc63_parse_dt(struct thc63_dev *thc63) { - struct device_node *thc63_out; + struct device_node *endpoint; struct device_node *remote;
- thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node, - THC63_RGB_OUT0, -1); - if (!thc63_out) { + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node, + THC63_RGB_OUT0, -1); + if (!endpoint) { dev_err(thc63->dev, "Missing endpoint in port@%u\n", THC63_RGB_OUT0); return -ENODEV; }
- remote = of_graph_get_remote_port_parent(thc63_out); - of_node_put(thc63_out); + remote = of_graph_get_remote_port_parent(endpoint); + of_node_put(endpoint); if (!remote) { dev_err(thc63->dev, "Endpoint in port@%u unconnected\n", THC63_RGB_OUT0); @@ -132,6 +147,22 @@ static int thc63_parse_dt(struct thc63_dev *thc63) if (!thc63->next) return -EPROBE_DEFER;
+ endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node, + THC63_LVDS_IN1, -1); + if (endpoint) { + remote = of_graph_get_remote_port_parent(endpoint); + of_node_put(endpoint); + + if (remote) { + if (of_device_is_available(remote)) + thc63->timings.dual_link = true; + of_node_put(remote); + } + } + + dev_dbg(thc63->dev, "operating in %s-link mode\n", + thc63->timings.dual_link ? "dual" : "single"); + return 0; }
@@ -188,6 +219,7 @@ static int thc63_probe(struct platform_device *pdev) thc63->bridge.driver_private = thc63; thc63->bridge.of_node = pdev->dev.of_node; thc63->bridge.funcs = &thc63_bridge_func; + thc63->bridge.timings = &thc63->timings;
drm_bridge_add(&thc63->bridge);
Hi Laurent,
On Sun, May 12, 2019 at 12:06:55AM +0300, Laurent Pinchart wrote:
Set a drm_bridge_timings in the drm_bridge, and use it to report the input bus mode (single-link or dual-link). The other fields of the timings structure are kept to 0 as they do not apply to LVDS buses.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Changes since v1:
- Ignore disabled remote device
drivers/gpu/drm/bridge/thc63lvd1024.c | 54 +++++++++++++++++++++------ 1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c index b083a740565c..709dd28b43d6 100644 --- a/drivers/gpu/drm/bridge/thc63lvd1024.c +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -31,6 +31,8 @@ struct thc63_dev {
struct drm_bridge bridge; struct drm_bridge *next;
- struct drm_bridge_timings timings;
};
static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge) static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge, const struct drm_display_mode *mode) {
- struct thc63_dev *thc63 = to_thc63(bridge);
- unsigned int min_freq;
- unsigned int max_freq;
- /*
* The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in* mode. Note that the limits are different in dual-in, single-out mode,* and will need to be adjusted accordingly.
* The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but* dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out* isn't supported by the driver yet, simply derive the limits from the* input mode.
- if (mode->clock < 8000)
- if (thc63->timings.dual_link) {
min_freq = 40000;max_freq = 150000;- } else {
min_freq = 8000;max_freq = 135000;- }
- if (mode->clock < min_freq) return MODE_CLOCK_LOW;
- if (mode->clock > 135000)
- if (mode->clock > max_freq) return MODE_CLOCK_HIGH;
I would have made a separate patch for this bit, anyway, verified against the THC631024 manual, and this matches my understanding.
return MODE_OK; @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func = {
static int thc63_parse_dt(struct thc63_dev *thc63) {
- struct device_node *thc63_out;
- struct device_node *endpoint; struct device_node *remote;
- thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
THC63_RGB_OUT0, -1);- if (!thc63_out) {
- endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
THC63_RGB_OUT0, -1);- if (!endpoint) { dev_err(thc63->dev, "Missing endpoint in port@%u\n", THC63_RGB_OUT0); return -ENODEV; }
- remote = of_graph_get_remote_port_parent(thc63_out);
- of_node_put(thc63_out);
- remote = of_graph_get_remote_port_parent(endpoint);
- of_node_put(endpoint); if (!remote) { dev_err(thc63->dev, "Endpoint in port@%u unconnected\n", THC63_RGB_OUT0);
@@ -132,6 +147,22 @@ static int thc63_parse_dt(struct thc63_dev *thc63) if (!thc63->next) return -EPROBE_DEFER;
- endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
THC63_LVDS_IN1, -1);- if (endpoint) {
remote = of_graph_get_remote_port_parent(endpoint);of_node_put(endpoint);if (remote) {if (of_device_is_available(remote))thc63->timings.dual_link = true;of_node_put(remote);}- }
- dev_dbg(thc63->dev, "operating in %s-link mode\n",
thc63->timings.dual_link ? "dual" : "single");
Fine, thanks for having addressed comments on the RFC.
Reviewed-by: Jacopo Mondi jacopo@jmondi.org
Thanks j
return 0; }
@@ -188,6 +219,7 @@ static int thc63_probe(struct platform_device *pdev) thc63->bridge.driver_private = thc63; thc63->bridge.of_node = pdev->dev.of_node; thc63->bridge.funcs = &thc63_bridge_func;
thc63->bridge.timings = &thc63->timings;
drm_bridge_add(&thc63->bridge);
-- Regards,
Laurent Pinchart
Add a new optional renesas,companion property to point to the companion LVDS encoder. This is used to support dual-link operation where the main LVDS encoder splits even-numbered and odd-numbered pixels between the two LVDS encoders.
The new property doesn't control the mode of operation, it only describes the relationship between the master and companion LVDS encoders.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- Changes since v1:
- Fixed typo --- .../devicetree/bindings/display/bridge/renesas,lvds.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index 900a884ad9f5..f2cc01d54cbd 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
Each port shall have a single endpoint.
+Optional properties: + +- renesas,companion : phandle to the companion LVDS encoder. This property is + valid for the first LVDS encoder on D3 and E3 SoCs only, and points to the + second encoder to be used as a companion in dual-link mode. +
Example:
Hi Laurent,
On Sun, May 12, 2019 at 12:06:56AM +0300, Laurent Pinchart wrote:
Add a new optional renesas,companion property to point to the companion LVDS encoder. This is used to support dual-link operation where the main LVDS encoder splits even-numbered and odd-numbered pixels between the two LVDS encoders.
The new property doesn't control the mode of operation, it only describes the relationship between the master and companion LVDS encoders.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Changes since v1:
- Fixed typo
.../devicetree/bindings/display/bridge/renesas,lvds.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index 900a884ad9f5..f2cc01d54cbd 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
Each port shall have a single endpoint.
+Optional properties:
+- renesas,companion : phandle to the companion LVDS encoder. This property is
- valid for the first LVDS encoder on D3 and E3 SoCs only, and points to the
- second encoder to be used as a companion in dual-link mode.
If I got this right, the property does not enable dual-link operations by itself, but it needs the next bridge to be operating in dual link mode (ie. has both LVDS0 and LVDS1 output connected to its input ports). Is it worth describing it here (or at least clarify the the property alone does not enable dual link operations).
Apart from that Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
Thanks j
Example:
-- Regards,
Laurent Pinchart
Hi Jacopo,
On Tue, May 28, 2019 at 11:28:47AM +0200, Jacopo Mondi wrote:
On Sun, May 12, 2019 at 12:06:56AM +0300, Laurent Pinchart wrote:
Add a new optional renesas,companion property to point to the companion LVDS encoder. This is used to support dual-link operation where the main LVDS encoder splits even-numbered and odd-numbered pixels between the two LVDS encoders.
The new property doesn't control the mode of operation, it only describes the relationship between the master and companion LVDS encoders.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Changes since v1:
- Fixed typo
.../devicetree/bindings/display/bridge/renesas,lvds.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt index 900a884ad9f5..f2cc01d54cbd 100644 --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -45,6 +45,12 @@ OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
Each port shall have a single endpoint.
+Optional properties:
+- renesas,companion : phandle to the companion LVDS encoder. This property is
- valid for the first LVDS encoder on D3 and E3 SoCs only, and points to the
- second encoder to be used as a companion in dual-link mode.
If I got this right, the property does not enable dual-link operations by itself, but it needs the next bridge to be operating in dual link mode (ie. has both LVDS0 and LVDS1 output connected to its input ports).
Correct.
Is it worth describing it here (or at least clarify the the property alone does not enable dual link operations).
How about the following ?
- 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.
Apart from that Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
The DRM core and DU driver guarantee that the LVDS bridge will not be double-enabled or double-disabled. Remove the corresponding unnecessary checks.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ------------------- 1 file changed, 19 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 620b51aab291..a331f0c32187 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -63,7 +63,6 @@ struct rcar_lvds { struct clk *extal; /* External clock */ struct clk *dotclkin[2]; /* External DU clocks */ } clocks; - bool enabled;
struct drm_display_mode display_mode; enum rcar_lvds_mode mode; @@ -368,15 +367,12 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq)
dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
- WARN_ON(lvds->enabled); - ret = clk_prepare_enable(lvds->clocks.mod); if (ret < 0) return ret;
__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
- lvds->enabled = true; return 0; } EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable); @@ -390,13 +386,9 @@ void rcar_lvds_clk_disable(struct drm_bridge *bridge)
dev_dbg(lvds->dev, "disabling LVDS PLL\n");
- WARN_ON(!lvds->enabled); - rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod); - - lvds->enabled = false; } EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
@@ -417,8 +409,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) u32 lvdcr0; int ret;
- WARN_ON(lvds->enabled); - ret = clk_prepare_enable(lvds->clocks.mod); if (ret < 0) return; @@ -507,16 +497,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) drm_panel_prepare(lvds->panel); drm_panel_enable(lvds->panel); } - - lvds->enabled = true; }
static void rcar_lvds_disable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- WARN_ON(!lvds->enabled); - if (lvds->panel) { drm_panel_disable(lvds->panel); drm_panel_unprepare(lvds->panel); @@ -527,8 +513,6 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod); - - lvds->enabled = false; }
static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge, @@ -592,8 +576,6 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge, { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- WARN_ON(lvds->enabled); - lvds->display_mode = *adjusted_mode;
rcar_lvds_get_lvds_mode(lvds); @@ -793,7 +775,6 @@ static int rcar_lvds_probe(struct platform_device *pdev)
lvds->dev = &pdev->dev; lvds->info = of_device_get_match_data(&pdev->dev); - lvds->enabled = false;
ret = rcar_lvds_parse_dt(lvds); if (ret < 0)
Hi Laurent,
On Sun, May 12, 2019 at 12:06:57AM +0300, Laurent Pinchart wrote:
The DRM core and DU driver guarantee that the LVDS bridge will not be double-enabled or double-disabled. Remove the corresponding unnecessary checks.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
Thanks j
drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ------------------- 1 file changed, 19 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 620b51aab291..a331f0c32187 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -63,7 +63,6 @@ struct rcar_lvds { struct clk *extal; /* External clock */ struct clk *dotclkin[2]; /* External DU clocks */ } clocks;
bool enabled;
struct drm_display_mode display_mode; enum rcar_lvds_mode mode;
@@ -368,15 +367,12 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq)
dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
WARN_ON(lvds->enabled);
ret = clk_prepare_enable(lvds->clocks.mod); if (ret < 0) return ret;
__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
lvds->enabled = true; return 0;
} EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable); @@ -390,13 +386,9 @@ void rcar_lvds_clk_disable(struct drm_bridge *bridge)
dev_dbg(lvds->dev, "disabling LVDS PLL\n");
WARN_ON(!lvds->enabled);
rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod);
lvds->enabled = false;
} EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
@@ -417,8 +409,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) u32 lvdcr0; int ret;
- WARN_ON(lvds->enabled);
- ret = clk_prepare_enable(lvds->clocks.mod); if (ret < 0) return;
@@ -507,16 +497,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) drm_panel_prepare(lvds->panel); drm_panel_enable(lvds->panel); }
- lvds->enabled = true;
}
static void rcar_lvds_disable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- WARN_ON(!lvds->enabled);
- if (lvds->panel) { drm_panel_disable(lvds->panel); drm_panel_unprepare(lvds->panel);
@@ -527,8 +513,6 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDPLLCR, 0);
clk_disable_unprepare(lvds->clocks.mod);
- lvds->enabled = false;
}
static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge, @@ -592,8 +576,6 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge, { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
WARN_ON(lvds->enabled);
lvds->display_mode = *adjusted_mode;
rcar_lvds_get_lvds_mode(lvds);
@@ -793,7 +775,6 @@ static int rcar_lvds_probe(struct platform_device *pdev)
lvds->dev = &pdev->dev; lvds->info = of_device_get_match_data(&pdev->dev);
lvds->enabled = false;
ret = rcar_lvds_parse_dt(lvds); if (ret < 0)
-- Regards,
Laurent Pinchart
In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and sends odd-numbered pixels to the LVDS1 encoder for transmission on a separate link.
To implement support for this mode of operation, determine if the LVDS connection operates in dual-link mode by querying the next device in the pipeline, locate the companion encoder, and control it directly through its bridge operations.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org --- drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++---- drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ 2 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index a331f0c32187..f7e4710fe33f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -66,6 +66,9 @@ struct rcar_lvds {
struct drm_display_mode display_mode; enum rcar_lvds_mode mode; + + struct drm_bridge *companion; + bool dual_link; };
#define bridge_to_rcar_lvds(bridge) \ @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); const struct drm_display_mode *mode = &lvds->display_mode; - /* - * FIXME: We should really retrieve the CRTC through the state, but how - * do we get a state pointer? - */ - struct drm_crtc *crtc = lvds->bridge.encoder->crtc; u32 lvdhcr; u32 lvdcr0; int ret; @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) if (ret < 0) return;
+ /* Enable the companion LVDS encoder in dual-link mode. */ + if (lvds->dual_link && lvds->companion) + lvds->companion->funcs->enable(lvds->companion); + /* * Hardcode the channels and control signals routing for now. * @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) { - /* Disable dual-link mode. */ - rcar_lvds_write(lvds, LVDSTRIPE, 0); + /* + * 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); }
- /* PLL clock configuration. */ - lvds->info->pll_setup(lvds, mode->clock * 1000); + /* + * PLL clock configuration on all instances but the companion in + * dual-link mode. + */ + if (!lvds->dual_link || lvds->companion) + lvds->info->pll_setup(lvds, mode->clock * 1000);
/* Set the LVDS mode and select the input. */ lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT; - if (drm_crtc_index(crtc) == 2) - lvdcr0 |= LVDCR0_DUSEL; + + if (lvds->bridge.encoder) { + /* + * FIXME: We should really retrieve the CRTC through the state, + * but how do we get a state pointer? + */ + if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2) + lvdcr0 |= LVDCR0_DUSEL; + } + rcar_lvds_write(lvds, LVDCR0, lvdcr0);
/* Turn all the channels on. */ @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCR1, 0); rcar_lvds_write(lvds, LVDPLLCR, 0);
+ /* Disable the companion LVDS encoder in dual-link mode. */ + if (lvds->dual_link && lvds->companion) + lvds->companion->funcs->disable(lvds->companion); + clk_disable_unprepare(lvds->clocks.mod); }
@@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = { .mode_set = rcar_lvds_mode_set, };
+bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{ + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); + + return lvds->dual_link; +} +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link); + /* ----------------------------------------------------------------------------- * Probe & Remove */
+static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) +{ + const struct of_device_id *match; + struct device_node *companion; + struct device *dev = lvds->dev; + int ret = 0; + + /* Locate the companion LVDS encoder for dual-link operation, if any. */ + companion = of_parse_phandle(dev->of_node, "renesas,companion", 0); + if (!companion) + return -ENODEV; + + /* + * Sanity check: the companion encoder must have the same compatible + * string. + */ + match = of_match_device(dev->driver->of_match_table, dev); + if (!of_device_is_compatible(companion, match->compatible)) { + ret = -ENODEV; + goto done; + } + + lvds->companion = of_drm_find_bridge(companion); + if (!lvds->companion) { + ret = -EPROBE_DEFER; + goto done; + } + + dev_dbg(dev, "Found companion encoder %pOF\n", companion); + +done: + of_node_put(companion); + + return ret; +} + static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL; @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
if (is_bridge) { lvds->next_bridge = of_drm_find_bridge(remote); - if (!lvds->next_bridge) + if (!lvds->next_bridge) { ret = -EPROBE_DEFER; + goto done; + } + + if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) + lvds->dual_link = lvds->next_bridge->timings + ? lvds->next_bridge->timings->dual_link + : false; } else { lvds->panel = of_drm_find_panel(remote); - if (IS_ERR(lvds->panel)) + if (IS_ERR(lvds->panel)) { ret = PTR_ERR(lvds->panel); + goto done; + } }
+ if (lvds->dual_link) + ret = rcar_lvds_parse_dt_companion(lvds); + done: of_node_put(local_output); of_node_put(remote_input); diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h index a709cae1bc32..222ec0e60785 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h @@ -15,6 +15,7 @@ struct drm_bridge; #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS) int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq); void rcar_lvds_clk_disable(struct drm_bridge *bridge); +bool rcar_lvds_dual_link(struct drm_bridge *bridge); #else static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq) @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, return -ENOSYS; } static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { } +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{ + return false; +} #endif /* CONFIG_DRM_RCAR_LVDS */
#endif /* __RCAR_LVDS_H__ */
Hi Laurent, a small note.
On Sun, May 12, 2019 at 12:06:58AM +0300, Laurent Pinchart wrote:
In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and sends odd-numbered pixels to the LVDS1 encoder for transmission on a separate link.
To implement support for this mode of operation, determine if the LVDS connection operates in dual-link mode by querying the next device in the pipeline, locate the companion encoder, and control it directly through its bridge operations.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++---- drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ 2 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index a331f0c32187..f7e4710fe33f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -66,6 +66,9 @@ struct rcar_lvds {
struct drm_display_mode display_mode; enum rcar_lvds_mode mode;
- struct drm_bridge *companion;
- bool dual_link;
};
#define bridge_to_rcar_lvds(bridge) \ @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); const struct drm_display_mode *mode = &lvds->display_mode;
- /*
* FIXME: We should really retrieve the CRTC through the state, but how* do we get a state pointer?*/- struct drm_crtc *crtc = lvds->bridge.encoder->crtc; u32 lvdhcr; u32 lvdcr0; int ret;
@@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) if (ret < 0) return;
- /* Enable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
lvds->companion->funcs->enable(lvds->companion);- /*
- Hardcode the channels and control signals routing for now.
@@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/* Disable dual-link mode. */rcar_lvds_write(lvds, LVDSTRIPE, 0);
/** 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);
- /* PLL clock configuration. */
- lvds->info->pll_setup(lvds, mode->clock * 1000);
/*
* PLL clock configuration on all instances but the companion in* dual-link mode.*/if (!lvds->dual_link || lvds->companion)
lvds->info->pll_setup(lvds, mode->clock * 1000);/* Set the LVDS mode and select the input. */ lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
- if (drm_crtc_index(crtc) == 2)
lvdcr0 |= LVDCR0_DUSEL;
if (lvds->bridge.encoder) {
/** FIXME: We should really retrieve the CRTC through the state,* but how do we get a state pointer?*/if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)lvdcr0 |= LVDCR0_DUSEL;}
rcar_lvds_write(lvds, LVDCR0, lvdcr0);
/* Turn all the channels on. */
@@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCR1, 0); rcar_lvds_write(lvds, LVDPLLCR, 0);
- /* Disable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
lvds->companion->funcs->disable(lvds->companion);- clk_disable_unprepare(lvds->clocks.mod);
}
@@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = { .mode_set = rcar_lvds_mode_set, };
+bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{
- struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- return lvds->dual_link;
+} +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
/* -----------------------------------------------------------------------------
- Probe & Remove
*/
+static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) +{
- const struct of_device_id *match;
- struct device_node *companion;
- struct device *dev = lvds->dev;
- int ret = 0;
- /* Locate the companion LVDS encoder for dual-link operation, if any. */
- companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
- if (!companion)
return -ENODEV;- /*
* Sanity check: the companion encoder must have the same compatible* string.*/- match = of_match_device(dev->driver->of_match_table, dev);
- if (!of_device_is_compatible(companion, match->compatible)) {
ret = -ENODEV;goto done;- }
- lvds->companion = of_drm_find_bridge(companion);
- if (!lvds->companion) {
ret = -EPROBE_DEFER;goto done;- }
- dev_dbg(dev, "Found companion encoder %pOF\n", companion);
+done:
- of_node_put(companion);
- return ret;
+}
static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL; @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
if (is_bridge) { lvds->next_bridge = of_drm_find_bridge(remote);
if (!lvds->next_bridge)
if (!lvds->next_bridge) { ret = -EPROBE_DEFER;goto done;}if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = lvds->next_bridge->timings? lvds->next_bridge->timings->dual_link: false;
if (IS_ERR(lvds->panel))
if (IS_ERR(lvds->panel)) { ret = PTR_ERR(lvds->panel);goto done;}}
if (lvds->dual_link)
ret = rcar_lvds_parse_dt_companion(lvds);
Looking at the error path here below, for E3/D3, -ENODEV gets sanitized to return 0 as we want this method to return success even if no endpoint is there, when using the LVDS encoder provided clock to back-feed the DU.
This is not the case for the companion property imho. If the property is specified, it should be sane and -ENODEV is worth being propagated to the caller.
You could move the error handling bits in the error path:
/* * On D3/E3 the LVDS encoder provides a clock to the DU, which can be * used for the DPAD output even when the LVDS output is not connected. * Don't fail probe in that case as the DU will need the bridge to * control the clock. */ if (lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL) return ret == -ENODEV ? 0 : ret;
before the of_node_put() sequences, and add one more label, to skip the above part in case parse_dt_companion() fails.
Apart from this you can retain my tag if you like. Thanks j
done: of_node_put(local_output); of_node_put(remote_input); diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h index a709cae1bc32..222ec0e60785 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h @@ -15,6 +15,7 @@ struct drm_bridge; #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS) int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq); void rcar_lvds_clk_disable(struct drm_bridge *bridge); +bool rcar_lvds_dual_link(struct drm_bridge *bridge); #else static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq) @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, return -ENOSYS; } static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { } +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{
- return false;
+} #endif /* CONFIG_DRM_RCAR_LVDS */
#endif /* __RCAR_LVDS_H__ */
Regards,
Laurent Pinchart
Hi Jacopo,
On Tue, May 28, 2019 at 11:35:50AM +0200, Jacopo Mondi wrote:
Hi Laurent, a small note.
On Sun, May 12, 2019 at 12:06:58AM +0300, Laurent Pinchart wrote:
In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and sends odd-numbered pixels to the LVDS1 encoder for transmission on a separate link.
To implement support for this mode of operation, determine if the LVDS connection operates in dual-link mode by querying the next device in the pipeline, locate the companion encoder, and control it directly through its bridge operations.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
drivers/gpu/drm/rcar-du/rcar_lvds.c | 104 ++++++++++++++++++++++++---- drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++ 2 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index a331f0c32187..f7e4710fe33f 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -66,6 +66,9 @@ struct rcar_lvds {
struct drm_display_mode display_mode; enum rcar_lvds_mode mode;
- struct drm_bridge *companion;
- bool dual_link;
};
#define bridge_to_rcar_lvds(bridge) \ @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) { struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge); const struct drm_display_mode *mode = &lvds->display_mode;
- /*
* FIXME: We should really retrieve the CRTC through the state, but how* do we get a state pointer?*/- struct drm_crtc *crtc = lvds->bridge.encoder->crtc; u32 lvdhcr; u32 lvdcr0; int ret;
@@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) if (ret < 0) return;
- /* Enable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
lvds->companion->funcs->enable(lvds->companion);- /*
- Hardcode the channels and control signals routing for now.
@@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
/* Disable dual-link mode. */rcar_lvds_write(lvds, LVDSTRIPE, 0);
/** 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);
- /* PLL clock configuration. */
- lvds->info->pll_setup(lvds, mode->clock * 1000);
/*
* PLL clock configuration on all instances but the companion in* dual-link mode.*/if (!lvds->dual_link || lvds->companion)
lvds->info->pll_setup(lvds, mode->clock * 1000);/* Set the LVDS mode and select the input. */ lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
- if (drm_crtc_index(crtc) == 2)
lvdcr0 |= LVDCR0_DUSEL;
if (lvds->bridge.encoder) {
/** FIXME: We should really retrieve the CRTC through the state,* but how do we get a state pointer?*/if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)lvdcr0 |= LVDCR0_DUSEL;}
rcar_lvds_write(lvds, LVDCR0, lvdcr0);
/* Turn all the channels on. */
@@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge) rcar_lvds_write(lvds, LVDCR1, 0); rcar_lvds_write(lvds, LVDPLLCR, 0);
- /* Disable the companion LVDS encoder in dual-link mode. */
- if (lvds->dual_link && lvds->companion)
lvds->companion->funcs->disable(lvds->companion);- clk_disable_unprepare(lvds->clocks.mod);
}
@@ -628,10 +650,54 @@ static const struct drm_bridge_funcs rcar_lvds_bridge_ops = { .mode_set = rcar_lvds_mode_set, };
+bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{
- struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
- return lvds->dual_link;
+} +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
/* -----------------------------------------------------------------------------
- Probe & Remove
*/
+static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) +{
- const struct of_device_id *match;
- struct device_node *companion;
- struct device *dev = lvds->dev;
- int ret = 0;
- /* Locate the companion LVDS encoder for dual-link operation, if any. */
- companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
- if (!companion)
return -ENODEV;- /*
* Sanity check: the companion encoder must have the same compatible* string.*/- match = of_match_device(dev->driver->of_match_table, dev);
- if (!of_device_is_compatible(companion, match->compatible)) {
ret = -ENODEV;goto done;- }
- lvds->companion = of_drm_find_bridge(companion);
- if (!lvds->companion) {
ret = -EPROBE_DEFER;goto done;- }
- dev_dbg(dev, "Found companion encoder %pOF\n", companion);
+done:
- of_node_put(companion);
- return ret;
+}
static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) { struct device_node *local_output = NULL; @@ -682,14 +748,26 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds)
if (is_bridge) { lvds->next_bridge = of_drm_find_bridge(remote);
if (!lvds->next_bridge)
if (!lvds->next_bridge) { ret = -EPROBE_DEFER;goto done;}if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK)lvds->dual_link = lvds->next_bridge->timings? lvds->next_bridge->timings->dual_link: false;
if (IS_ERR(lvds->panel))
if (IS_ERR(lvds->panel)) { ret = PTR_ERR(lvds->panel);goto done;}}
if (lvds->dual_link)
ret = rcar_lvds_parse_dt_companion(lvds);Looking at the error path here below, for E3/D3, -ENODEV gets sanitized to return 0 as we want this method to return success even if no endpoint is there, when using the LVDS encoder provided clock to back-feed the DU.
This is not the case for the companion property imho. If the property is specified, it should be sane and -ENODEV is worth being propagated to the caller.
But in that case the LVDS encoder will fail to probe, and so the DU channel will not be able to output on the DPAD. Isn't it worth supporting that ? Otherwise I can simply make rcar_lvds_parse_dt_companion() return -EINVAL or -ENXIO and it should be fine with the existing error handling path.
You could move the error handling bits in the error path:
/* * On D3/E3 the LVDS encoder provides a clock to the DU, which can be * used for the DPAD output even when the LVDS output is not connected. * Don't fail probe in that case as the DU will need the bridge to * control the clock. */ if (lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL) return ret == -ENODEV ? 0 : ret;
before the of_node_put() sequences, and add one more label, to skip the above part in case parse_dt_companion() fails.
Apart from this you can retain my tag if you like. Thanks j
done: of_node_put(local_output); of_node_put(remote_input); diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.h b/drivers/gpu/drm/rcar-du/rcar_lvds.h index a709cae1bc32..222ec0e60785 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.h +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.h @@ -15,6 +15,7 @@ struct drm_bridge; #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS) int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq); void rcar_lvds_clk_disable(struct drm_bridge *bridge); +bool rcar_lvds_dual_link(struct drm_bridge *bridge); #else static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, unsigned long freq) @@ -22,6 +23,10 @@ static inline int rcar_lvds_clk_enable(struct drm_bridge *bridge, return -ENOSYS; } static inline void rcar_lvds_clk_disable(struct drm_bridge *bridge) { } +static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge) +{
- return false;
+} #endif /* CONFIG_DRM_RCAR_LVDS */
#endif /* __RCAR_LVDS_H__ */
In dual-link LVDS mode, the LVDS1 encoder is used as a companion for LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1 can't be used in that case, don't create an encoder and connector for it.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++++++++++++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index 6c91753af7bc..fe046d194944 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -16,6 +16,7 @@ #include "rcar_du_drv.h" #include "rcar_du_encoder.h" #include "rcar_du_kms.h" +#include "rcar_lvds.h"
/* ----------------------------------------------------------------------------- * Encoder @@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, } }
+ /* + * On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a + * companion for LVDS0 in dual-link mode. + */ + if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) { + if (bridge && rcar_lvds_dual_link(bridge)) { + ret = -ENOLINK; + goto done; + } + } + ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); if (ret < 0) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f8f7fff34dff..95c81e59e2f1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -378,7 +378,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, }
ret = rcar_du_encoder_init(rcdu, output, entity); - if (ret && ret != -EPROBE_DEFER) + if (ret && ret != -EPROBE_DEFER && ret != -ENOLINK) dev_warn(rcdu->dev, "failed to initialize encoder %pOF on output %u (%d), skipping\n", entity, output, ret);
HI Laurent,
On Sun, May 12, 2019 at 12:06:59AM +0300, Laurent Pinchart wrote:
In dual-link LVDS mode, the LVDS1 encoder is used as a companion for LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1 can't be used in that case, don't create an encoder and connector for it.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++++++++++++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index 6c91753af7bc..fe046d194944 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c @@ -16,6 +16,7 @@ #include "rcar_du_drv.h" #include "rcar_du_encoder.h" #include "rcar_du_kms.h" +#include "rcar_lvds.h"
/* -----------------------------------------------------------------------------
- Encoder
@@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu, } }
- /*
* On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a* companion for LVDS0 in dual-link mode.*/- if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
if (bridge && rcar_lvds_dual_link(bridge)) {
If you get here, 'bridge' is guaranteed to be valid.
ret = -ENOLINK;goto done;}- }
- ret = drm_encoder_init(rcdu->ddev, encoder, &encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); if (ret < 0)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f8f7fff34dff..95c81e59e2f1 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -378,7 +378,7 @@ static int rcar_du_encoders_init_one(struct rcar_du_device *rcdu, }
ret = rcar_du_encoder_init(rcdu, output, entity);
- if (ret && ret != -EPROBE_DEFER)
- if (ret && ret != -EPROBE_DEFER && ret != -ENOLINK)
Apart from the minor above Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
Thanks j
dev_warn(rcdu->dev, "failed to initialize encoder %pOF on output %u (%d), skipping\n", entity, output, ret);-- Regards,
Laurent Pinchart
Add the new renesas,companion property to the LVDS0 node to point to the companion LVDS encoder LVDS1.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 ++ arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index d2ad665fe2d9..b52e3fdb5fca 100644 --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi @@ -1731,6 +1731,8 @@ resets = <&cpg 727>; status = "disabled";
+ renesas,companion = <&lvds1>; + ports { #address-cells = <1>; #size-cells = <0>; diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index 5bf3af246e14..94b5177eb152 100644 --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi @@ -1038,6 +1038,8 @@ resets = <&cpg 727>; status = "disabled";
+ renesas,companion = <&lvds1>; + ports { #address-cells = <1>; #size-cells = <0>;
Hi Laurent,
On Sun, May 12, 2019 at 12:07:00AM +0300, Laurent Pinchart wrote:
Add the new renesas,companion property to the LVDS0 node to point to the companion LVDS encoder LVDS1.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com
Provided this does not enable dual-link operations by default: Reviewed-by: Jacopo Mondi jacopo+renesas@jmondi.org
Thanks j
arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 ++ arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index d2ad665fe2d9..b52e3fdb5fca 100644 --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi @@ -1731,6 +1731,8 @@ resets = <&cpg 727>; status = "disabled";
renesas,companion = <&lvds1>;ports { #address-cells = <1>; #size-cells = <0>;diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index 5bf3af246e14..94b5177eb152 100644 --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi @@ -1038,6 +1038,8 @@ resets = <&cpg 727>; status = "disabled";
renesas,companion = <&lvds1>;ports { #address-cells = <1>; #size-cells = <0>;-- Regards,
Laurent Pinchart
Enable and connect the second LVDS encoder to the second LVDS input of the THC63LVD1024 for dual-link LVDS operation. This requires changing the default settings of SW45 and SW47 to OFF and ON respectively.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- .../arm64/boot/dts/renesas/r8a77995-draak.dts | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index db2bed1751b8..6af0e891c1ec 100644 --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts @@ -77,11 +77,18 @@
port@0 { reg = <0>; - thc63lvd1024_in: endpoint { + thc63lvd1024_in0: endpoint { remote-endpoint = <&lvds0_out>; }; };
+ port@1 { + reg = <1>; + thc63lvd1024_in1: endpoint { + remote-endpoint = <&lvds1_out>; + }; + }; + port@2 { reg = <2>; thc63lvd1024_out: endpoint { @@ -349,17 +356,27 @@ ports { port@1 { lvds0_out: endpoint { - remote-endpoint = <&thc63lvd1024_in>; + remote-endpoint = <&thc63lvd1024_in0>; }; }; }; };
&lvds1 { + status = "okay"; + clocks = <&cpg CPG_MOD 727>, <&x12_clk>, <&extal_clk>; clock-names = "fck", "dclkin.0", "extal"; + + ports { + port@1 { + lvds1_out: endpoint { + remote-endpoint = <&thc63lvd1024_in1>; + }; + }; + }; };
&ohci0 {
Enable and connect the second LVDS encoder to the second LVDS input of the THC63LVD1024 for dual-link LVDS operation. This requires changing the default settings of SW45 and SW47 to OFF and ON respectively.
Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com --- .../arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts index 144c0820cf60..88bdbf4fc82c 100644 --- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts +++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts @@ -93,11 +93,18 @@
port@0 { reg = <0>; - thc63lvd1024_in: endpoint { + thc63lvd1024_in0: endpoint { remote-endpoint = <&lvds0_out>; }; };
+ port@1 { + reg = <1>; + thc63lvd1024_in1: endpoint { + remote-endpoint = <&lvds1_out>; + }; + }; + port@2 { reg = <2>; thc63lvd1024_out: endpoint { @@ -451,17 +458,27 @@ ports { port@1 { lvds0_out: endpoint { - remote-endpoint = <&thc63lvd1024_in>; + remote-endpoint = <&thc63lvd1024_in0>; }; }; }; };
&lvds1 { + status = "okay"; + clocks = <&cpg CPG_MOD 727>, <&x13_clk>, <&extal_clk>; clock-names = "fck", "dclkin.0", "extal"; + + ports { + port@1 { + lvds1_out: endpoint { + remote-endpoint = <&thc63lvd1024_in1>; + }; + }; + }; };
&ohci0 {
Hi Laurent,
On Sat, May 11, 2019 at 11:07 PM Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com wrote:
On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two LVDS inputs and two parallel outputs. It can operate in four different modes:
Single-in, single-out: The first LVDS input receives the video stream, and the bridge outputs it on the first parallel output. The second LVDS input and the second parallel output are not used.
Single-in, dual-out: The first LVDS input receives the video stream, and the bridge splits even- and odd-numbered pixels and outputs them on the first and second parallel outputs. The second LVDS input is not used.
Dual-in, single-out: The two LVDS inputs are used in dual-link mode, and the bridge combines the even- and odd-numbered pixels and outputs them on the first parallel output. The second parallel output is not used.
Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, and the bridge outputs the even- and odd-numbered pixels on the first parallel output.
and the second?
The operating mode is selected by two input pins of the bridge, which are connected to DIP switches on the development boards I use. The mode is thus fixed from a Linux point of view.
Can the state of these switches be read from software?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert,
On Sun, May 12, 2019 at 10:55:20AM +0200, Geert Uytterhoeven wrote:
On Sat, May 11, 2019 at 11:07 PM Laurent Pinchart wrote:
On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two LVDS inputs and two parallel outputs. It can operate in four different modes:
Single-in, single-out: The first LVDS input receives the video stream, and the bridge outputs it on the first parallel output. The second LVDS input and the second parallel output are not used.
Single-in, dual-out: The first LVDS input receives the video stream, and the bridge splits even- and odd-numbered pixels and outputs them on the first and second parallel outputs. The second LVDS input is not used.
Dual-in, single-out: The two LVDS inputs are used in dual-link mode, and the bridge combines the even- and odd-numbered pixels and outputs them on the first parallel output. The second parallel output is not used.
Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, and the bridge outputs the even- and odd-numbered pixels on the first parallel output.
and the second?
I should have read "on the first and second parallel outputs respectively" indeed.
The operating mode is selected by two input pins of the bridge, which are connected to DIP switches on the development boards I use. The mode is thus fixed from a Linux point of view.
Can the state of these switches be read from software?
Unfortunately not :-(
Hi Laurent,
On Sun, May 12, 2019 at 12:06:52AM +0300, Laurent Pinchart wrote:
Hello everybody,
This patch series implements support for LVDS dual-link mode in the R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 LVDS decoder driver.
LVDS dual-link is a mode of operation where two individual LVDS links are operated together to carry even- and odd-numbered pixels separately. This doubles the possible bandwidth of the video transmission. Both the transmitter and the receiver need to support this mode of operation.
The R-Car D3 and E3 SoCs include two independent LVDS encoders that can be grouped together to operate in dual-link mode. When used separately, the LVDS encoders are connected to two different CRTCs and transmit independent video streams. When used in dual-link mode, the first LVDS encoder is connected to the first CRTC, and split even- and odd-numbered pixels. It transmits half of the pixels on its LVDS output, and sends the other half to the second LVDS encoder for transmittion over the second LVDS link. The second LVDS encoder thus operates under control of the first one, and isn't connected directly to a CRTC.
On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two LVDS inputs and two parallel outputs. It can operate in four different modes:
Single-in, single-out: The first LVDS input receives the video stream, and the bridge outputs it on the first parallel output. The second LVDS input and the second parallel output are not used.
Single-in, dual-out: The first LVDS input receives the video stream, and the bridge splits even- and odd-numbered pixels and outputs them on the first and second parallel outputs. The second LVDS input is not used.
Dual-in, single-out: The two LVDS inputs are used in dual-link mode, and the bridge combines the even- and odd-numbered pixels and outputs them on the first parallel output. The second parallel output is not used.
Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, and the bridge outputs the even- and odd-numbered pixels on the first parallel output.
The operating mode is selected by two input pins of the bridge, which are connected to DIP switches on the development boards I use. The mode is thus fixed from a Linux point of view.
Patch 01/10 adds a new dual_link boolen field to the drm_bridge_timings structure to let bridges report their LVDS mode of operation. Patch 02/10 clarifies the THC63LVD1024 DT bindings to document dual-link operation, and patch 03/10 implements dual-link support in the thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link field according to the mode selected through DT.
Patch 04/10 extends the R-Car LVDS DT bindings to specify the companion LVDS encoder for dual-link operation. Patches 05/10 then performs a small cleanup in the LVDS encoder driver. Patch 06/10 implements dual-link support in the LVDS encoder driver, which involves retrieving the operation mode from the LVDS receiver, locating the companion LVDS encoder, and configuring both encoders when dual-link operation is desired. The API towards the DU driver is also extended to report the mode of operation.
Patch 07/10 implements dual-link mode support in the DU driver. There is no specific configuration to be performed there, as dual-link is fully implemented in the LVDS encoder driver, but the DU driver has to skip creation of the DRM encoder and connector related to the second LVDS encoder when dual-link is used, as the second LVDS encoder operates as a slave of the first one, transparently from a CRTC (and thus userspace) perspective.
Patch 08/10 specifies the companion LVDS encoder in the D3 and E3 DT bindings. This by itself doesn't enable dual-link mode, the LVDS0 encoder is still connected to the HDMI output through LVDS receiver, and the LVDS1 encoder is not used. Patches 09/10 and 10/10, not intended to be merged, enable dual-link operation for the D3 and E3 boards for testing and require flipping DIP switches on the boards.
The patches are based on top of my drm/du/next branch, and are available for convenience at
git://linuxtv.org/pinchartl/media.git drm/du/lvds/dual-linkThey have been tested successfully on the D3 Draak board. I expect them to work on E3 as well, but I don't have access to an Ebisu board to test this.
I have now tested the series on E3 Ebisu board using kms-tests using the HDMI output and I confirm it works as expected.
For the whole series: Tested-by: Jacopo Mondi jacopo+renesas@jmondi.org
Thanks j
Laurent Pinchart (10): drm: bridge: Add dual_link field to the drm_bridge_timings structure dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation drm: bridge: thc63: Report input bus mode through bridge timings dt-bindings: display: renesas: lvds: Add renesas,companion property drm: rcar-du: lvds: Remove LVDS double-enable checks drm: rcar-du: lvds: Add support for dual-link mode drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1 [HACK] arm64: dts: renesas: draak: Enable LVDS dual-link operation [HACK] arm64: dts: renesas: ebisu: Enable LVDS dual-link operation
.../bindings/display/bridge/renesas,lvds.txt | 6 + .../display/bridge/thine,thc63lvd1024.txt | 6 + .../arm64/boot/dts/renesas/r8a77990-ebisu.dts | 21 ++- arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 + .../arm64/boot/dts/renesas/r8a77995-draak.dts | 21 ++- arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 + drivers/gpu/drm/bridge/thc63lvd1024.c | 54 ++++++-- drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- drivers/gpu/drm/rcar-du/rcar_lvds.c | 123 +++++++++++++----- drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 + include/drm/drm_bridge.h | 8 ++ 12 files changed, 214 insertions(+), 48 deletions(-)
-- Regards,
Laurent Pinchart
dri-devel@lists.freedesktop.org
-
Geert Uytterhoeven -
Jacopo Mondi -
Laurent Pinchart -
Laurent Pinchart -
Rob Herring