[v3,04/10] dt-bindings: display: renesas: lvds: Add renesas,companion property
diff mbox series

Message ID 20190528141234.15425-5-laurent.pinchart+renesas@ideasonboard.com
State New
Delegated to: Kieran Bingham
Headers show
Series
  • R-Car DU: LVDS dual-link mode support
Related show

Commit Message

Laurent Pinchart May 28, 2019, 2:12 p.m. UTC
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>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
Changes since v2:

- Clarify when the companion property is required or not allowed

Changes since v1:

- Fixed typo
---
 .../devicetree/bindings/display/bridge/renesas,lvds.txt    | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Sam Ravnborg May 28, 2019, 4:37 p.m. UTC | #1
Hi Laurent.

Reading through this nice series.

On Tue, May 28, 2019 at 05:12:28PM +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>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> Changes since v2:
> 
> - Clarify when the companion property is required or not allowed
> 
> Changes since v1:
> 
> - Fixed typo
> ---
>  .../devicetree/bindings/display/bridge/renesas,lvds.txt    | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index 900a884ad9f5..2d24bd8cbec5 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -45,6 +45,13 @@ 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
> +  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.

If the D3 and E3 socs do not mandate the use of dual-link, then what to
do in the DT? Because according to the above this property must be
specified for D3 and E3 SOC's.

> +
>  
>  Example:

Always good with examples, maybe it comes later.

	Sam
Laurent Pinchart May 28, 2019, 4:49 p.m. UTC | #2
Hi Sam,

On Tue, May 28, 2019 at 06:37:30PM +0200, Sam Ravnborg wrote:
> On Tue, May 28, 2019 at 05:12:28PM +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>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > Changes since v2:
> > 
> > - Clarify when the companion property is required or not allowed
> > 
> > Changes since v1:
> > 
> > - Fixed typo
> > ---
> >  .../devicetree/bindings/display/bridge/renesas,lvds.txt    | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > index 900a884ad9f5..2d24bd8cbec5 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> > @@ -45,6 +45,13 @@ 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
> > +  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.
> 
> If the D3 and E3 socs do not mandate the use of dual-link, then what to
> do in the DT? Because according to the above this property must be
> specified for D3 and E3 SOC's.

This property doesn't enable dual-link mode, it only specifies the
companion LVDS encoder used for dual-link mode, when enabled (through
communication between the LVDS encoder and the LVDS receiver at
runtime).

Jacopo had a similar comment so I suppose this isn't clear. How would
you word it to make it clear ?

> > +
> >  
> >  Example:
> 
> Always good with examples, maybe it comes later.

Good point, I'll fix that.
Sam Ravnborg May 28, 2019, 4:59 p.m. UTC | #3
Hi Laurent.

> > >  
> > > +Optional properties:
> > > +
> > > +- renesas,companion : phandle to the companion LVDS encoder. This property is
> > > +  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
> > > +  the second encoder to be used as a companion in dual-link mode. It shall not
> > > +  be set for any other LVDS encoder.
> > 
> > If the D3 and E3 socs do not mandate the use of dual-link, then what to
> > do in the DT? Because according to the above this property must be
> > specified for D3 and E3 SOC's.
> 
> This property doesn't enable dual-link mode, it only specifies the
> companion LVDS encoder used for dual-link mode, when enabled (through
> communication between the LVDS encoder and the LVDS receiver at
> runtime).
> 
> Jacopo had a similar comment so I suppose this isn't clear. How would
> you word it to make it clear ?
Let me try:


- renesas,companion : phandle to the companion LVDS encoder. This property is
  mandatory for the first LVDS encoder on D3 and E3 SoCs when dual-link mode
  is supported.
  The property shall pont to the phandle of the second encoder to be used as a
  companion in dual-link mode. It shall not be set for any other LVDS encoder.

The main difference is "when dual-link mode is supported".
As per my understanding this property is only relevant when the actual
HW supports / uses dual-link mode.
So for a board that do not even wire up dual-link, then setting the
property would be confusing.

I hope this better describes my understanding.

	Sam
Laurent Pinchart June 6, 2019, 7:54 a.m. UTC | #4
Hi Sam,

On Tue, May 28, 2019 at 06:59:00PM +0200, Sam Ravnborg wrote:
> Hi Laurent.
> 
> > > >  
> > > > +Optional properties:
> > > > +
> > > > +- renesas,companion : phandle to the companion LVDS encoder. This property is
> > > > +  mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to
> > > > +  the second encoder to be used as a companion in dual-link mode. It shall not
> > > > +  be set for any other LVDS encoder.
> > > 
> > > If the D3 and E3 socs do not mandate the use of dual-link, then what to
> > > do in the DT? Because according to the above this property must be
> > > specified for D3 and E3 SOC's.
> > 
> > This property doesn't enable dual-link mode, it only specifies the
> > companion LVDS encoder used for dual-link mode, when enabled (through
> > communication between the LVDS encoder and the LVDS receiver at
> > runtime).
> > 
> > Jacopo had a similar comment so I suppose this isn't clear. How would
> > you word it to make it clear ?
> 
> Let me try:
> 
> - renesas,companion : phandle to the companion LVDS encoder. This property is
>   mandatory for the first LVDS encoder on D3 and E3 SoCs when dual-link mode
>   is supported.
>   The property shall pont to the phandle of the second encoder to be used as a
>   companion in dual-link mode. It shall not be set for any other LVDS encoder.
> 
> The main difference is "when dual-link mode is supported".
> As per my understanding this property is only relevant when the actual
> HW supports / uses dual-link mode.
> So for a board that do not even wire up dual-link, then setting the
> property would be confusing.

That's not quite correct. The property shall be specified when the SoC
supports dual-link mode (which is the case for the D3 and E3 SoCs only),
regardless of whether the board is wired up in single-link or dual-link
mode. Selection of the mode is performed at runtime by looking at the
requirements of the LVDS sink, not based on the companion property in
DT. The renesas,companion property is thus SoC-specific, but not
board-specific.

> I hope this better describes my understanding.
Sam Ravnborg June 6, 2019, 9:27 a.m. UTC | #5
Hi Laurent.

> > 
> > The main difference is "when dual-link mode is supported".
> > As per my understanding this property is only relevant when the actual
> > HW supports / uses dual-link mode.
> > So for a board that do not even wire up dual-link, then setting the
> > property would be confusing.
> 
> That's not quite correct. The property shall be specified when the SoC
> supports dual-link mode (which is the case for the D3 and E3 SoCs only),
> regardless of whether the board is wired up in single-link or dual-link
> mode. Selection of the mode is performed at runtime by looking at the
> requirements of the LVDS sink, not based on the companion property in
> DT. The renesas,companion property is thus SoC-specific, but not
> board-specific.
Thanks for taking your time to clarify this.

	Sam
Kieran Bingham June 7, 2019, 10:33 p.m. UTC | #6
Hi Laurent,

On 28/05/2019 15:12, 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>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> Changes since v2:
> 
> - Clarify when the companion property is required or not allowed
> 
> Changes since v1:
> 
> - Fixed typo
> ---
>  .../devicetree/bindings/display/bridge/renesas,lvds.txt    | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> index 900a884ad9f5..2d24bd8cbec5 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
> @@ -45,6 +45,13 @@ 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
> +  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.
> +

I see Sam has already asked for an updated example, so with that:

I'm fine with the text above.

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

>  
>  Example:
>  
>

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index 900a884ad9f5..2d24bd8cbec5 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -45,6 +45,13 @@  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
+  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.
+
 
 Example: