Hi Laurent,

Thank you for your feedback!

From: linux-kernel-owner@vger.kernel.org linux-kernel-owner@vger.kernel.org On Behalf Of Laurent Pinchart Sent: 15 August 2019 12:45 Subject: Re: [PATCH v2 1/9] dt-bindings: panel: lvds: Add dual-link LVDS display support

Hi Fabrizio,

On Thu, Aug 15, 2019 at 12:04:25PM +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

---

v1->v2:

• Reworked the description of the ports property
• lvds0_panel_in in the example has been renamed to panel_in0
• lvds1_panel_in in the example has been renamed to panel_in1

Laurent,

in linux-next they are now working with: Documentation/devicetree/bindings/display/panel/lvds.yaml

Documentation/devicetree/bindings/display/panel/lvds.yaml is in drm-misc-next, so I would advise rebasing on top of that.

Will do.

Thanks, Fab

...

What should I do here?

.../bindings/display/panel/panel-lvds.txt | 95 ++++++++++++++++------ 1 file changed, 71 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..5231243 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt @@ -41,7 +41,12 @@ 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 panel) corresponding to the panel LVDS input, or two port
• subnodes (for a dual link panel) corresponding to the panel LVDS inputs.
• Dual-link LVDS panels expect even pixels (0, 2, 4, etc.) and odd pixels (1, 3,
• 5, etc.) on different input ports, it's up to the panel-specific bindings to
• specify what port is expecting even pixels, and what port is expecting odd
• pixels.

LVDS data mappings are defined as follows. @@ -92,30 +97,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>;
  

• 		panel_in0: endpoint {
  

• 			remote-endpoint = <&lvds0_out>;
  

• 		};
  

• 	};
  

• 	port@1 {
  

• 		reg = <1>;
  

• 		panel_in1: endpoint {
  

• 			remote-endpoint = <&lvds1_out>;
  

• 		};
  

• 	};
  

  }; };

-};

-- Regards,

Laurent Pinchart

Hi Fabrizio,

On Thu, Aug 15, 2019 at 12:04:26PM +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: https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-ID...

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• Reworked according to Laurent's feedback
• Renamed lvds0_panel_in to panel_in0
• Renamed lvds1_panel_in to panel_in1

Laurent,

Should this be a .yaml file already?

It's not a strict requirement, but I'm sure Rob would really appreciate. I've converted a DT binding to yaml recently (for a panel too), and I have to say I'm impressed by the validation yaml brings, both for DT sources and for DT bindings. It even validates the example in the bindings, which is great. Documentation/devicetree/writing-schema.md should give you a few pointers to get started (in particular make sure you run make dt_binding_check for your new bindings).

.../display/panel/advantech,idk-2121wr.txt | 56 ++++++++++++++++++++++ 1 file changed, 56 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..6ee1d1b --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.txt @@ -0,0 +1,56 @@ +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. +The panel operates in dual-link mode and thus requires two port nodes, +the first port node expects odd pixels (1, 3, 5, etc.) and the second port +expects even pixels (0, 2, 4, etc.).

+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 {
  

• 		/* Odd pixels */
  

• 		reg = <0>;
  

• 		panel_in0: endpoint {
  

• 			remote-endpoint = <&lvds0_out>;
  

• 		};
  

• 	};
  

• 	port@1 {
  

• 		/* Even pixels */
  

• 		reg = <1>;
  

• 		panel_in1: endpoint {
  

• 			remote-endpoint = <&lvds1_out>;
  

• 		};
  

• 	};
  

• };
  

• };

-- 2.7.4

Hello Laurent,

Thank you for your feedback!

From: linux-kernel-owner@vger.kernel.org linux-kernel-owner@vger.kernel.org On Behalf Of Laurent Pinchart Sent: 15 August 2019 14:19 Subject: Re: [PATCH v2 3/9] drm: Rename drm_bridge_timings to drm_timings

Hi Fabrizio,

(CC'ing Greg as the architect of the SPDX move)

On Thu, Aug 15, 2019 at 12:04:27PM +0100, Fabrizio Castro wrote:

...

The information represented by drm_bridge_timings is also needed by panels, therefore rename drm_bridge_timings to drm_timings.

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com Link: https://www.spinics.net/lists/linux-renesas-soc/msg43271.html

---

v1->v2:

• new patch

I have copied the license from include/drm/drm_bridge.h as that's where the struct originally came from. What's the right SPDX license to use in this case?

https://wiki.spdx.org/view/Legal_Team/Decisions/Dealing_with_Public_Domain_w...

Greg, any idea on how we should handle this ?

...

drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 ++-- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/thc63lvd1024.c | 2 +- drivers/gpu/drm/bridge/ti-tfp410.c | 6 ++-- drivers/gpu/drm/pl111/pl111_display.c | 2 +- include/drm/drm_bridge.h | 40 ++--------------------- include/drm/drm_timings.h | 60 +++++++++++++++++++++++++++++++++++ 7 files changed, 71 insertions(+), 47 deletions(-) create mode 100644 include/drm/drm_timings.h

diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c index d32885b..bb1d928 100644 --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c @@ -228,7 +228,7 @@ static int dumb_vga_remove(struct platform_device *pdev)

• NOTE: the ADV7123EP seems to have other timings and need a new timings
• set if used.

*/ -static const struct drm_bridge_timings default_dac_timings = { +static const struct drm_timings default_dac_timings = { /* Timing specifications, datasheet page 7 */ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, .setup_time_ps = 500, @@ -239,7 +239,7 @@ static const struct drm_bridge_timings default_dac_timings = {

• Information taken from the THS8134, THS8134A, THS8134B datasheet named
• "SLVS205D", dated May 1990, revised March 2000.

*/ -static const struct drm_bridge_timings ti_ths8134_dac_timings = { +static const struct drm_timings ti_ths8134_dac_timings = { /* From timing diagram, datasheet page 9 */ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, /* From datasheet, page 12 */ @@ -252,7 +252,7 @@ static const struct drm_bridge_timings ti_ths8134_dac_timings = {

• Information taken from the THS8135 datasheet named "SLAS343B", dated
• May 2001, revised April 2013.

*/ -static const struct drm_bridge_timings ti_ths8135_dac_timings = { +static const struct drm_timings ti_ths8135_dac_timings = { /* From timing diagram, datasheet page 14 */ .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE, /* From datasheet, page 16 */ diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index dd7aa46..0c63065 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -914,7 +914,7 @@ static int sii902x_i2c_bypass_deselect(struct i2c_mux_core *mux, u32 chan_id) return 0; }

-static const struct drm_bridge_timings default_sii902x_timings = { +static const struct drm_timings default_sii902x_timings = { .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE | DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE | DRM_BUS_FLAG_DE_HIGH, diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c index 3d74129b..9047a9e 100644 --- a/drivers/gpu/drm/bridge/thc63lvd1024.c +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -34,7 +34,7 @@ struct thc63_dev { struct drm_bridge bridge; struct drm_bridge *next;

• struct drm_bridge_timings timings;

• struct drm_timings timings;

};

static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index dbf35c7..c086b06c 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -32,7 +32,7 @@ struct tfp410 { struct delayed_work hpd_work; struct gpio_desc *powerdown;

• struct drm_bridge_timings timings;

• struct drm_timings timings;

  struct device *dev;

}; @@ -190,7 +190,7 @@ static irqreturn_t tfp410_hpd_irq_thread(int irq, void *arg) return IRQ_HANDLED; }

-static const struct drm_bridge_timings tfp410_default_timings = { +static const struct drm_timings tfp410_default_timings = { .input_bus_flags = DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE | DRM_BUS_FLAG_DE_HIGH, .setup_time_ps = 1200, @@ -199,7 +199,7 @@ static const struct drm_bridge_timings tfp410_default_timings = {

static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) {

• struct drm_bridge_timings *timings = &dvi->timings;

• struct drm_timings *timings = &dvi->timings; struct device_node *ep; u32 pclk_sample = 0; u32 bus_width = 24;

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c index 15d2755..c82b21f 100644 --- a/drivers/gpu/drm/pl111/pl111_display.c +++ b/drivers/gpu/drm/pl111/pl111_display.c @@ -188,7 +188,7 @@ static void pl111_display_enable(struct drm_simple_display_pipe *pipe, }

if (bridge) {

• const struct drm_bridge_timings *btimings = bridge->timings;
  

• const struct drm_timings *btimings = bridge->timings;
  
  

  /*

  • Here is when things get really fun. Sometimes the bridge

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 7616f65..8270a38 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -27,9 +27,9 @@ #include <linux/ctype.h> #include <drm/drm_mode_object.h> #include <drm/drm_modes.h> +#include <drm/drm_timings.h>

struct drm_bridge; -struct drm_bridge_timings; struct drm_panel;

/** @@ -337,42 +337,6 @@ struct drm_bridge_funcs { };

/**

• • struct drm_bridge_timings - timing information for the bridge
• */

-struct drm_bridge_timings {

• /**

• * @input_bus_flags:
  

• *
  

• * Tells what additional settings for the pixel data on the bus
  

• * this bridge requires (like pixel signal polarity). See also
  

• * &drm_display_info->bus_flags.
  

• */
  

• u32 input_bus_flags;
• /**

• * @setup_time_ps:
  

• *
  

• * Defines the time in picoseconds the input data lines must be
  

• * stable before the clock edge.
  

• */
  

• u32 setup_time_ps;
• /**

• * @hold_time_ps:
  

• *
  

• * Defines the time in picoseconds taken for the bridge to sample the
  

• * 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;

-};

-/**

• struct drm_bridge - central DRM bridge control structure

*/ struct drm_bridge { @@ -393,7 +357,7 @@ struct drm_bridge { * * the timing specification for the bridge, if any (may be NULL) */

• const struct drm_bridge_timings *timings;

• const struct drm_timings *timings; /** @funcs: control functions */ const struct drm_bridge_funcs *funcs; /** @driver_private: pointer to the bridge driver's internal context */

diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h new file mode 100644 index 0000000..4af8814 --- /dev/null +++ b/include/drm/drm_timings.h @@ -0,0 +1,60 @@ +/*

• • Permission to use, copy, modify, distribute, and sell this software and its
• • documentation for any purpose is hereby granted without fee, provided that
• • the above copyright notice appear in all copies and that both that copyright
• • notice and this permission notice appear in supporting documentation, and
• • that the name of the copyright holders not be used in advertising or
• • publicity pertaining to distribution of the software without specific,
• • written prior permission. The copyright holders make no representations
• • about the suitability of this software for any purpose. It is provided "as
• • is" without express or implied warranty.
• • THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
• • INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
• • EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
• • CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
• • DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
• • TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
• • OF THIS SOFTWARE.
• */

+#ifndef __DRM_TIMINGS_H__ +#define __DRM_TIMINGS_H__

+/**

• • struct drm_timings - timing information

This dangerously relates to video timings. I would name the structure drm_bus_timings, or drm_bus_params (or something similar) as it contains more than timings.

Will rename to drm_bus_timings.

Thanks, Fab

...

• */

+struct drm_timings {

• /**

• * @input_bus_flags:
  

• *
  

• * Tells what additional settings for the pixel data on the bus
  

• * are required (like pixel signal polarity). See also
  

• * &drm_display_info->bus_flags.
  

• */
  

• u32 input_bus_flags;
• /**

• * @setup_time_ps:
  

• *
  

• * Defines the time in picoseconds the input data lines must be
  

• * stable before the clock edge.
  

• */
  

• u32 setup_time_ps;
• /**

• * @hold_time_ps:
  

• *
  

• * Defines the time in picoseconds taken for the bridge to sample the
  

• * 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;

+};

+#endif /* __DRM_TIMINGS_H__ */

-- Regards,

Laurent Pinchart

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:28PM +0100, Fabrizio Castro wrote:

We need more information to describe dual-LVDS links, therefore introduce link_flags.

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• new patch

include/drm/drm_timings.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)

diff --git a/include/drm/drm_timings.h b/include/drm/drm_timings.h index 4af8814..58fbf1b 100644 --- a/include/drm/drm_timings.h +++ b/include/drm/drm_timings.h @@ -1,4 +1,6 @@ /*

• • Copyright (C) 2019 Renesas Electronics Corporation
• • Permission to use, copy, modify, distribute, and sell this software and its
  • documentation for any purpose is hereby granted without fee, provided that
  • the above copyright notice appear in all copies and that both that copyright

@@ -21,6 +23,24 @@ #ifndef __DRM_TIMINGS_H__ #define __DRM_TIMINGS_H__

+#include <linux/bits.h>

+/**

• • enum drm_link_flags - link_flags for &drm_timings
• • This enum defines the details of the link.
• • @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5,

• • 			etc.) coming through the first port, and
    

• • 			even pixels (0,2,4,etc.) coming through
    

• • 			the second port. If not specified for a
    

• • 			dual-LVDS panel, it is assumed that even
    

• • 			pixels are coming through the first port
    

• */

+enum drm_link_flags {

The text will be easier to read if you inline it here.

/** * @DRM_LINK_DUAL_LVDS_ODD_EVEN: Dual-LVDS link, with odd pixels (1,3,5, * etc.) coming through the first port, and even pixels (0,2,4,etc.) ...

• DRM_LINK_DUAL_LVDS_ODD_EVEN = BIT(0),

I would remove the dual_link field and add a DRM_LINK_DUAL_LVDS (or alternatively two flags, dual lvds odd-even and dual lvds even-odd).

+};

/**

• struct drm_timings - timing information

*/ @@ -55,6 +75,12 @@ struct drm_timings { * and odd-numbered pixels are received on separate links. */ bool dual_link;

• /**

• * @link_flags
  

• *
  

• * Provides detailed information about the link.
  

I think this calls for a bit more information than "detailed information". What information do you want to store in this field ?

• */
  

• enum drm_link_flags link_flags;

};

#endif /* __DRM_TIMINGS_H__ */

2.7.4

Hello Laurent,

Thank you for your feedback!

From: linux-renesas-soc-owner@vger.kernel.org linux-renesas-soc-owner@vger.kernel.org On Behalf Of Laurent Pinchart Sent: 15 August 2019 13:03 Subject: Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:

...

We need to know if the panel supports dual-link, similarly to bridges, therefore add a reference to drm_timings in drm_panel.

Panels may also need to report setup/hold time, so it's not about dual-link only. I would make this explicit in the commit message.

Ok

...

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• new patch

include/drm/drm_panel.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 8c738c0..cd6ff07 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -26,6 +26,7 @@

#include <linux/errno.h> #include <linux/list.h> +#include <drm/drm_timings.h>

You can just add a forward-declaration of struct drm_timing.

Ok

Thanks, Fab

...

struct device_node; struct drm_connector; @@ -81,6 +82,7 @@ struct drm_panel_funcs {

• struct drm_panel - DRM panel object
• @drm: DRM device owning the panel
• @connector: DRM connector that the panel is attached to

• • @timings: timing information
  • @dev: parent device of the panel
  • @link: link from panel device (supplier) to DRM device (consumer)
  • @funcs: operations that can be performed on the panel

@@ -89,6 +91,7 @@ struct drm_panel_funcs { struct drm_panel { struct drm_device *drm; struct drm_connector *connector;

• const struct drm_timings *timings; struct device *dev;

  const struct drm_panel_funcs *funcs;

-- Regards,

Laurent Pinchart

Hi Sam,

Thank you for your feedback!

From: Sam Ravnborg sam@ravnborg.org Sent: 15 August 2019 15:14 Subject: Re: [PATCH v2 5/9] drm/panel: Add timings field to drm_panel

Hi Fabrizio

On Thu, Aug 15, 2019 at 12:04:29PM +0100, Fabrizio Castro wrote:

...

We need to know if the panel supports dual-link, similarly to bridges, therefore add a reference to drm_timings in drm_panel.

Why do we need to know this?

The encoders connected to a dual-LVDS panels may need to do special arrangements for the dual-link setup, like initializing a companion encoder, and working out which pixels (even or odd) to send to which port (first LVDS input port or second LVDS input port). At least, this is within the scope of the implementation of this series, which is currently being discussed.

Why is it needed in drm_panel and not in some driver specific struct?

The other fields are still applicable to panels, so I think it makes sense for this to be included in struct drm_panels.

I cannot see the full series, as I was copied only on some mails. Awaiting dri-devel moderator before I can see the rest.

I am sorry about this, some people don't want to be bothered by the whole thing, I'll make sure I add you to the recipients list for the next iterations of this series.

Thanks, Fab

Sam

...

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• new patch

include/drm/drm_panel.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 8c738c0..cd6ff07 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -26,6 +26,7 @@

#include <linux/errno.h> #include <linux/list.h> +#include <drm/drm_timings.h>

struct device_node; struct drm_connector; @@ -81,6 +82,7 @@ struct drm_panel_funcs {

• struct drm_panel - DRM panel object
• @drm: DRM device owning the panel
• @connector: DRM connector that the panel is attached to

• • @timings: timing information
  • @dev: parent device of the panel
  • @link: link from panel device (supplier) to DRM device (consumer)
  • @funcs: operations that can be performed on the panel

@@ -89,6 +91,7 @@ struct drm_panel_funcs { struct drm_panel { struct drm_device *drm; struct drm_connector *connector;

• const struct drm_timings *timings; struct device *dev;

  const struct drm_panel_funcs *funcs;

-- 2.7.4

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:30PM +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

---

v1->v2:

• reworked according to Laurent's feedback

drivers/gpu/drm/rcar-du/rcar_lvds.c | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 3fe0b86..41d28f4 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -603,6 +603,11 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge, lvds->display_mode = *adjusted_mode;

rcar_lvds_get_lvds_mode(lvds);

• if (lvds->companion) {

• struct rcar_lvds *companion_lvds = bridge_to_rcar_lvds(
  

• 					lvds->companion);
  

• companion_lvds->mode = lvds->mode;
  

How about calling rcar_lvds_mode_set() on the companion instead ?

• }

}

static int rcar_lvds_attach(struct drm_bridge *bridge)

Hello Laurent,

Thank you for your feedback!

From: linux-renesas-soc-owner@vger.kernel.org linux-renesas-soc-owner@vger.kernel.org On Behalf Of Laurent Pinchart Sent: 15 August 2019 14:09 Subject: Re: [PATCH v2 7/9] drm: rcar-du: lvds: Add dual-LVDS panels support

Hi Fabrizio,

Thank you for the patch.

On Thu, Aug 15, 2019 at 12:04:31PM +0100, Fabrizio Castro wrote:

...

This patch adds support for dual-LVDS panels.

It's very important that we coordinate the efforts of both the primary encoder and the companion encoder to get the right picture on the panel, therefore this patch adds some code to work out if even and odd pixels need swapping. When the encoders are connected to a LVDS panel, the assumption is that by default the panel expects even pixels (0, 2, 4, etc.) on the first input port, and odd pixels (1, 3, 5, etc.) on the seconds port. When DRM_LINK_DUAL_LVDS_ODD_EVEN is found among the link flags, the display expects odd pixels on the first input port, and even pixels on the second port. As a result, the way the encoders are connected to the panel may trigger pixel (data) swapping.

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• new patch, resulting from Laurent's feedback

drivers/gpu/drm/rcar-du/rcar_lvds.c | 121 ++++++++++++++++++++++++------------ 1 file changed, 81 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 41d28f4..5c24884 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -22,6 +22,8 @@ #include <drm/drm_bridge.h> #include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> +#include <drm/drm_timings.h> +#include <drm/drm_of.h>

Please keep the headers alphabetically sorted.

Ok

...

#include "rcar_lvds.h" #include "rcar_lvds_regs.h" @@ -69,6 +71,7 @@ struct rcar_lvds {

struct drm_bridge *companion; bool dual_link;

• bool stripe_swap_data;

};

#define bridge_to_rcar_lvds(b) \ @@ -439,12 +442,20 @@ 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.
  

• 	 *
  

• 	 * ST_SWAP from LVD1STRIPE is reserved, do not set
  

• 	 * in the companion LVDS
  

• 	 */
  

• 	lvdstripe = LVDSTRIPE_ST_ON |
  

• 		(lvds->companion && lvds->stripe_swap_data ?
  

• 		 LVDSTRIPE_ST_SWAP : 0);
  

Let's sort out the alignment.

	lvdstripe = LVDSTRIPE_ST_ON
		  | (lvds->companion && lvds->stripe_swap_data ?
		     LVDSTRIPE_ST_SWAP : 0);

Ok

...

• rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
  

  }

  /*

@@ -706,13 +717,31 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) return ret; }

+static int rcar_lvds_get_remote_port_reg(struct device_node *np) +{

• struct device_node *endpoint_node, *remote_endpoint;
• struct of_endpoint endpoint;
• endpoint_node = of_graph_get_endpoint_by_regs(np, 1, 0);
• if (!endpoint_node)

• return -ENODEV;
  

• remote_endpoint = of_graph_get_remote_endpoint(endpoint_node);
• if (!remote_endpoint) {

• of_node_put(endpoint_node);
  

• return -ENODEV;
  

• }
• of_graph_parse_endpoint(remote_endpoint, &endpoint);
• of_node_put(endpoint_node);
• of_node_put(remote_endpoint);
• return endpoint.port;

+}

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;

• const struct drm_timings *timings = NULL; int ret = 0;

  local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);

@@ -740,45 +769,57 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) goto done; }

• remote_input = of_graph_get_remote_endpoint(local_output);

• ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,

• 			  &lvds->panel, &lvds->next_bridge);
  

• if (ret) {

• ret = -EPROBE_DEFER;
  

• goto done;
  

• }
• if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {

• if (lvds->next_bridge)
  

• 	timings = lvds->next_bridge->timings;
  

• else
  

• 	timings = lvds->panel->timings;
  

I wonder if we should use devm_drm_panel_bridge_add() (or drm_panel_bridge_add()) and use the bridge API only. It would require a small change in the drm_panel_bridge to copy the timings pointer, but apart from that I think it should be fine. If it creates too much churn due to connector handling then we can skip it for now and I'll handle it later (but I'd appreciate if you could copy the timings pointer in drm_panel_bridge already).

Will look into this.

...

• if (timings)
  

• 	lvds->dual_link = timings->dual_link;
  

• }

• for_each_endpoint_of_node(remote, node) {

• if (node != remote_input) {
  

• if (lvds->dual_link) {

• ret = rcar_lvds_parse_dt_companion(lvds);
  

• if (lvds->companion && timings) {
  

• 	int our_port, comp_port;
  

• 	bool odd_even_flag = timings->link_flags &
  

• 				DRM_LINK_DUAL_LVDS_ODD_EVEN;
  

• 	our_port = rcar_lvds_get_remote_port_reg(
  

• 				lvds->dev->of_node);
  

• 	if (our_port < 0) {
  

• 		ret = our_port;
  

• 		goto done;
  

• 	}
  

• 	comp_port = rcar_lvds_get_remote_port_reg(
  

• 				lvds->companion->of_node);
  

• 	if (comp_port < 0) {
  

• 		ret = comp_port;
  

• 		goto done;
  

• 	}
  /*
  

• 	 * We've found one endpoint other than the input, this
  

• 	 * must be a bridge.
  

• 	 * We need to match the port where we generate even
  

• 	 * pixels (0, 2, 4, etc.) to the port where the sink
  

• 	 * expects to receive even pixels, same thing for the
  

• 	 * odd pixels. Swap the generation of even and odd
  

• 	 * pixels if the connection requires it.
  

• 	 * By default (when DRM_LINK_DUAL_LVDS_ODD_EVEN is not
  

• 	 * specified) the sink expects even pixels on the
  

• 	 * first input port, and odd pixels on the second port.
  

I see what you're trying to do, but I'm not sure I like it much :-S

Peeking into the remove DT node like that creates a dependency between this driver and the DT bindings of all possible remote nodes. For this to work, you would need to ensure that the odd/even mapping to ports is common to all dual-link devices, and thus document that globally in the DT bindings. I'm not sure if there's a way around it as hardware connections could indeed switch the two lanes, so we need to model that somehow. It could be modelled with a swap property in DT, but that would still require a standard mapping of odd-even pixels to ports, so maybe the easiest option is to document globally that port 0 on the sink is for even pixels, and port 1 for odd pixels, and remove the DRM_LINK_DUAL_LVDS_ODD_EVEN flag completely. But what will then happen if you panel has more than two ports (for audio for instance, or for other types of video links) ? It may not be possible to always use port 0 and 1 for the LVDS even and odd pixels in DT bindings of a particular panel or bridge.

This is the stickiest point of the whole series. The implementation within this series allows for any number of ports on the sink, the LVDS ports don't have to be port 0 and port 1, it's enough that the port for the even pixels comes before the port of the odd pixels (but the logic can be inverted by means of DRM_LINK_DUAL_LVDS_ODD_EVEN), and if you swap the lvds0 and lvds1 OF graph connections around, the pixels will be swapped automatically. But of course, there is the dependency between the driver and dt-bindings you were mentioning, and of top of that every driver would need to work things out independently at this point in time.

A creative solution is needed here.

I may have an idea. What if we marked both ends of each OF graph link with either "even-pixels;" or "odd-pixels;", and exported a function that given the of_node of two endpoints returned if the link requires swapping? There'd be no need for the flag at that point, the numbering of the ports would not matter, and the DT would be comprehensive and very easy to read.

Let me please know your thoughts.

Thanks you for the patience Fab

...

	 */

• 	is_bridge = true;
  

• 	of_node_put(node);
  

• 	break;
  

• 	if (((comp_port - our_port > 0) &&  odd_even_flag) ||
  

• 	    ((comp_port - our_port < 0) && !odd_even_flag))
  

• 		lvds->stripe_swap_data = true;
  

  } }

• if (is_bridge) {

• lvds->next_bridge = of_drm_find_bridge(remote);
  

• 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)) {
  

• 	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); of_node_put(remote);

  /*

-- Regards,

Laurent Pinchart

Hi Laurent,

Thank you for getting back to me.

From: Laurent Pinchart laurent.pinchart@ideasonboard.com Sent: 20 August 2019 17:04 Subject: Re: [PATCH v2 7/9] drm: rcar-du: lvds: Add dual-LVDS panels support

Hi Fabrizio,

On Thu, Aug 15, 2019 at 03:36:53PM +0000, Fabrizio Castro wrote:

...

On 15 August 2019 14:09 Laurent Pinchart wrote:

...

On Thu, Aug 15, 2019 at 12:04:31PM +0100, Fabrizio Castro wrote:

...

This patch adds support for dual-LVDS panels.

It's very important that we coordinate the efforts of both the primary encoder and the companion encoder to get the right picture on the panel, therefore this patch adds some code to work out if even and odd pixels need swapping. When the encoders are connected to a LVDS panel, the assumption is that by default the panel expects even pixels (0, 2, 4, etc.) on the first input port, and odd pixels (1, 3, 5, etc.) on the seconds port. When DRM_LINK_DUAL_LVDS_ODD_EVEN is found among the link flags, the display expects odd pixels on the first input port, and even pixels on the second port. As a result, the way the encoders are connected to the panel may trigger pixel (data) swapping.

Signed-off-by: Fabrizio Castro fabrizio.castro@bp.renesas.com

---

v1->v2:

• new patch, resulting from Laurent's feedback

drivers/gpu/drm/rcar-du/rcar_lvds.c | 121 ++++++++++++++++++++++++------------ 1 file changed, 81 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c b/drivers/gpu/drm/rcar-du/rcar_lvds.c index 41d28f4..5c24884 100644 --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c @@ -22,6 +22,8 @@ #include <drm/drm_bridge.h> #include <drm/drm_panel.h> #include <drm/drm_probe_helper.h> +#include <drm/drm_timings.h> +#include <drm/drm_of.h>

Please keep the headers alphabetically sorted.

Ok

...

...

#include "rcar_lvds.h" #include "rcar_lvds_regs.h" @@ -69,6 +71,7 @@ struct rcar_lvds {

struct drm_bridge *companion; bool dual_link;

• bool stripe_swap_data;

};

#define bridge_to_rcar_lvds(b) \ @@ -439,12 +442,20 @@ 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.
  

• 	 *
  

• 	 * ST_SWAP from LVD1STRIPE is reserved, do not set
  

• 	 * in the companion LVDS
  

• 	 */
  

• 	lvdstripe = LVDSTRIPE_ST_ON |
  

• 		(lvds->companion && lvds->stripe_swap_data ?
  

• 		 LVDSTRIPE_ST_SWAP : 0);
  

Let's sort out the alignment.

	lvdstripe = LVDSTRIPE_ST_ON
		  | (lvds->companion && lvds->stripe_swap_data ?
		     LVDSTRIPE_ST_SWAP : 0);

Ok

...

...

• rcar_lvds_write(lvds, LVDSTRIPE, lvdstripe);
  

  }

  /*

@@ -706,13 +717,31 @@ static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds) return ret; }

+static int rcar_lvds_get_remote_port_reg(struct device_node *np) +{

• struct device_node *endpoint_node, *remote_endpoint;
• struct of_endpoint endpoint;
• endpoint_node = of_graph_get_endpoint_by_regs(np, 1, 0);
• if (!endpoint_node)

• return -ENODEV;
  

• remote_endpoint = of_graph_get_remote_endpoint(endpoint_node);
• if (!remote_endpoint) {

• of_node_put(endpoint_node);
  

• return -ENODEV;
  

• }
• of_graph_parse_endpoint(remote_endpoint, &endpoint);
• of_node_put(endpoint_node);
• of_node_put(remote_endpoint);
• return endpoint.port;

+}

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;

• const struct drm_timings *timings = NULL; int ret = 0;

  local_output = of_graph_get_endpoint_by_regs(lvds->dev->of_node, 1, 0);

@@ -740,45 +769,57 @@ static int rcar_lvds_parse_dt(struct rcar_lvds *lvds) goto done; }

• remote_input = of_graph_get_remote_endpoint(local_output);

• ret = drm_of_find_panel_or_bridge(lvds->dev->of_node, 1, 0,

• 			  &lvds->panel, &lvds->next_bridge);
  

• if (ret) {

• ret = -EPROBE_DEFER;
  

• goto done;
  

• }
• if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {

• if (lvds->next_bridge)
  

• 	timings = lvds->next_bridge->timings;
  

• else
  

• 	timings = lvds->panel->timings;
  

I wonder if we should use devm_drm_panel_bridge_add() (or drm_panel_bridge_add()) and use the bridge API only. It would require a small change in the drm_panel_bridge to copy the timings pointer, but apart from that I think it should be fine. If it creates too much churn due to connector handling then we can skip it for now and I'll handle it later (but I'd appreciate if you could copy the timings pointer in drm_panel_bridge already).

Will look into this.

...

...

• if (timings)
  

• 	lvds->dual_link = timings->dual_link;
  

• }

• for_each_endpoint_of_node(remote, node) {

• if (node != remote_input) {
  

• if (lvds->dual_link) {

• ret = rcar_lvds_parse_dt_companion(lvds);
  

• if (lvds->companion && timings) {
  

• 	int our_port, comp_port;
  

• 	bool odd_even_flag = timings->link_flags &
  

• 				DRM_LINK_DUAL_LVDS_ODD_EVEN;
  

• 	our_port = rcar_lvds_get_remote_port_reg(
  

• 				lvds->dev->of_node);
  

• 	if (our_port < 0) {
  

• 		ret = our_port;
  

• 		goto done;
  

• 	}
  

• 	comp_port = rcar_lvds_get_remote_port_reg(
  

• 				lvds->companion->of_node);
  

• 	if (comp_port < 0) {
  

• 		ret = comp_port;
  

• 		goto done;
  

• 	}
  /*
  

• 	 * We've found one endpoint other than the input, this
  

• 	 * must be a bridge.
  

• 	 * We need to match the port where we generate even
  

• 	 * pixels (0, 2, 4, etc.) to the port where the sink
  

• 	 * expects to receive even pixels, same thing for the
  

• 	 * odd pixels. Swap the generation of even and odd
  

• 	 * pixels if the connection requires it.
  

• 	 * By default (when DRM_LINK_DUAL_LVDS_ODD_EVEN is not
  

• 	 * specified) the sink expects even pixels on the
  

• 	 * first input port, and odd pixels on the second port.
  

I see what you're trying to do, but I'm not sure I like it much :-S

Peeking into the remove DT node like that creates a dependency between this driver and the DT bindings of all possible remote nodes. For this to work, you would need to ensure that the odd/even mapping to ports is common to all dual-link devices, and thus document that globally in the DT bindings. I'm not sure if there's a way around it as hardware connections could indeed switch the two lanes, so we need to model that somehow. It could be modelled with a swap property in DT, but that would still require a standard mapping of odd-even pixels to ports, so maybe the easiest option is to document globally that port 0 on the sink is for even pixels, and port 1 for odd pixels, and remove the DRM_LINK_DUAL_LVDS_ODD_EVEN flag completely. But what will then happen if you panel has more than two ports (for audio for instance, or for other types of video links) ? It may not be possible to always use port 0 and 1 for the LVDS even and odd pixels in DT bindings of a particular panel or bridge.

This is the stickiest point of the whole series. The implementation within this series allows for any number of ports on the sink, the LVDS ports don't have to be port 0 and port 1, it's enough that the port for the even pixels comes before the port of the odd pixels (but the logic can be inverted by means of DRM_LINK_DUAL_LVDS_ODD_EVEN), and if you swap the lvds0 and lvds1 OF graph connections around, the pixels will be swapped automatically. But of course, there is the dependency between the driver and dt-bindings you were mentioning, and of top of that every driver would need to work things out independently at this point in time.

...

A creative solution is needed here.

I may have an idea. What if we marked both ends of each OF graph link with either "even-pixels;" or "odd-pixels;", and exported a function that given the of_node of two endpoints returned if the link requires swapping? There'd be no need for the flag at that point, the numbering of the ports would not matter, and the DT would be comprehensive and very easy to read.

Let me please know your thoughts.

Taking one step back to look at the big picture, what we need is for the source to know what pixel (odd or even) to send on each LVDS output. For that it needs to know what pixel is expected by the sink the the inputs connected to the source's outputs. From each source output we can easily locate the DT nodes corresponding to the connected sink's input ports, but that doesn't give us enough information. From there, we could

• Infer the odd/even pixel expected by the source based on the port numbers. This would require common DT bindings for all dual-link LVDS sinks that specify the port ordering (for instance the bindings could mandate that lowest numbered port correspond to even pixels).

• Read the odd/even pixel expected by the source from an explicit DT property, as proposed above. This would also require common DT bindings for all dual-link LVDS sinks that define these new properties. This would I think be better than implicitly infering it from DT port numbers.

• Retrieve the information from the sink drm_bridge at runtime. This would require a way to query the bridge for the mapping from port number to odd/even pixel. The DRM_LINK_DUAL_LVDS_ODD_EVEN flag could be used for that, and would then be defined as "the lowest numbered port corresponds to odd pixels and the highest numbered port corresponds to even pixels".

The second and third options would both work I think. The third one is roughly what you've implemented, except that I think the flag description should be clarified.

I think I like the second option better, I will give it a shot. I was thinking, perhaps we could drop the 'dual_link' member altogether (from drm_<whatever>_timings) in this case, as if we found information about even and odd pixels in the DT we could work out if we the configuration is dual-link, so there'd be no need to mark this in drivers.

What do you think?

Thanks, Fab

Sorry for circling back to your original proposal, I didn't understand it properly :-)

...

Thanks you for the patience

...

...

	 */

• 	is_bridge = true;
  

• 	of_node_put(node);
  

• 	break;
  

• 	if (((comp_port - our_port > 0) &&  odd_even_flag) ||
  

• 	    ((comp_port - our_port < 0) && !odd_even_flag))
  

• 		lvds->stripe_swap_data = true;
  

  } }

• if (is_bridge) {

• lvds->next_bridge = of_drm_find_bridge(remote);
  

• 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)) {
  

• 	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); of_node_put(remote);

  /*

-- Regards,

Laurent Pinchart