Re: [PATCH v10 09/12] dt-bindings: display: bridge: lvds-codec: Add new bus-width prop

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: dri-devel@lists.freedesktop.org,
	Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Andrey Smirnov <andrew.smirnov@gmail.com>,
	Jonas Karlman <jonas@kwiboo.se>, Rob Herring <robh+dt@kernel.org>,
	Andrzej Hajda <a.hajda@samsung.com>,
	devicetree@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	intel-gfx-trybot@lists.freedesktop.org, kernel@collabora.com,
	Sam Ravnborg <sam@ravnborg.org>, Chris Healy <cphealy@gmail.com>
Subject: Re: [PATCH v10 09/12] dt-bindings: display: bridge: lvds-codec: Add new bus-width prop
Date: Mon, 9 Mar 2020 11:35:43 +0100	[thread overview]
Message-ID: <20200309113543.24ab8a0d@collabora.com> (raw)
In-Reply-To: <20200225101354.5f621ccb@collabora.com>

Hi Laurent,

On Tue, 25 Feb 2020 10:13:54 +0100
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> > > diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > index 8f373029f5d2..7c4e42f4de61 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > @@ -55,6 +55,14 @@ properties:
> > >          description: |
> > >            For LVDS encoders, port 0 is the parallel input
> > >            For LVDS decoders, port 0 is the LVDS input
> > > +        properties:
> > > +          bus-width:
> > > +            allOf:
> > > +              - $ref: /schemas/types.yaml#/definitions/uint32
> > > +              - enum: [18, 24]
> > > +              - default: 24
> > > +          description:
> > > +            Number of data lines used to transmit the RGB data.    
> > 
> > This is a bit unclear. First of all, depending on whether the node is an
> > LVDS encoder or decoder, port@0 is either a parallel input or an LVDS
> > input. The property mentiones RGB data, does it mean it apply to LVDS
> > encoders only ? Or should it be in port@1 for LVDS decoders ?  
> 
> Right, I only considered the encoder case here. For the decoder case, we
> don't need a bus-width prop yet, as the bus format output is currently
> enforced by the bus format input of the next component in the chain
> (panel/next-bridge), but that might change if we start dealing with
> panel/bridges supporting several input formats and expecting the LVDS
> encoder/decoder to select one. What we do need for the decoder case
> though, is a data-mapping prop, otherwise this LVDS bridge exposes a
> FIXED in-format and the previous element in the chain has to use its
> 'default' output format (which might not be appropriate).
> 
> Maybe we should go for Sam's approach and expose a data-mapping prop
> on both ends of the bridge (that implies describing RGB/DPI bus width
> using the data-mapping prop), this way we wouldn't have to distinguish
> the encoder/decoder case.

Gentle ping.

Regards,

Boris