| Message ID | 20190528141234.15425-5-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | R-Car DU: LVDS dual-link mode support | expand |
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
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.
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
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.
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
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: > >
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: