Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sam Ravnborg <sam@ravnborg.org>
Cc: devicetree@vger.kernel.org, Jacopo Mondi <jacopo@jmondi.org>,
	Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	dri-devel@lists.freedesktop.org,
	linux-renesas-soc@vger.kernel.org,
	Jacopo Mondi <jacopo+renesas@jmondi.org>
Subject: Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas, companion property
Date: Thu, 6 Jun 2019 10:54:35 +0300	[thread overview]
Message-ID: <20190606075435.GD4931@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20190528165900.GA10262@ravnborg.org>

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.

-- 
Regards,

Laurent Pinchart