diff mbox series

[1/2] dt-bindings: display: bridge: lvds-codec: Document LVDS data mapping select

Message ID 20210515204656.367442-1-marex@denx.de
State Changes Requested
Headers show
Series [1/2] dt-bindings: display: bridge: lvds-codec: Document LVDS data mapping select | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Marek Vasut May 15, 2021, 8:46 p.m. UTC
Decoder input LVDS format is a property of the decoder chip or even
its strapping. Add DT property data-mapping the same way lvds-panel
does, to define the LVDS data mapping.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: devicetree@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
 .../bindings/display/bridge/lvds-codec.yaml   | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Laurent Pinchart May 17, 2021, 11:02 p.m. UTC | #1
Hi Marek,

Thank you for the patch.

On Sat, May 15, 2021 at 10:46:55PM +0200, Marek Vasut wrote:
> Decoder input LVDS format is a property of the decoder chip or even
> its strapping. Add DT property data-mapping the same way lvds-panel
> does, to define the LVDS data mapping.

The information could also be derived by the driver from the compatible
string in the case when this is an intrinsic property of the device (or
when it's configurable by software), but the fact that it can also be
controlled by strapping makes a DT property needed. We may want to limit
the usage of the DT property to the strapping case though, but I don't
have a real preference here, so I'm fine with this approach.

> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: devicetree@vger.kernel.org
> To: dri-devel@lists.freedesktop.org
> ---
>  .../bindings/display/bridge/lvds-codec.yaml   | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index f4dd16bd69d2..f0abb94f8f2e 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -64,6 +64,15 @@ properties:
>        - port@0
>        - port@1
>  
> +  data-mapping:
> +    enum:
> +      - jeida-18
> +      - jeida-24
> +      - vesa-24
> +    description: |
> +      The color signals mapping order. See details in
> +      Documentation/devicetree/bindings/display/panel/lvds.yaml
> +
>    pclk-sample:
>      description:
>        Data sampling on rising or falling edge.
> @@ -79,6 +88,16 @@ properties:
>  
>    power-supply: true
>  
> +if:
> +  not:
> +    properties:
> +      compatible:
> +        contains:
> +          const: lvds-decoder
> +then:
> +  properties:
> +    data-mapping: false
> +
>  if:
>    not:
>      properties:

Unless I'm mistaken, you can't have identically named properties at the
same level (multiple 'if' at the root level). You can group them in a
'allOf' property.
Laurent Pinchart May 17, 2021, 11:03 p.m. UTC | #2
And another thing:

On Tue, May 18, 2021 at 02:02:24AM +0300, Laurent Pinchart wrote:
> On Sat, May 15, 2021 at 10:46:55PM +0200, Marek Vasut wrote:
> > Decoder input LVDS format is a property of the decoder chip or even
> > its strapping. Add DT property data-mapping the same way lvds-panel
> > does, to define the LVDS data mapping.
> 
> The information could also be derived by the driver from the compatible
> string in the case when this is an intrinsic property of the device (or
> when it's configurable by software), but the fact that it can also be
> controlled by strapping makes a DT property needed. We may want to limit
> the usage of the DT property to the strapping case though, but I don't
> have a real preference here, so I'm fine with this approach.
> 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: devicetree@vger.kernel.org
> > To: dri-devel@lists.freedesktop.org
> > ---
> >  .../bindings/display/bridge/lvds-codec.yaml   | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > index f4dd16bd69d2..f0abb94f8f2e 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > @@ -64,6 +64,15 @@ properties:
> >        - port@0
> >        - port@1
> >  
> > +  data-mapping:
> > +    enum:
> > +      - jeida-18
> > +      - jeida-24
> > +      - vesa-24
> > +    description: |
> > +      The color signals mapping order. See details in
> > +      Documentation/devicetree/bindings/display/panel/lvds.yaml
> > +
> >    pclk-sample:
> >      description:
> >        Data sampling on rising or falling edge.
> > @@ -79,6 +88,16 @@ properties:
> >  
> >    power-supply: true
> >  
> > +if:
> > +  not:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          const: lvds-decoder
> > +then:
> > +  properties:
> > +    data-mapping: false

Should we make the property required for lvds-decoder ? We need to
support backward compatibility in the driver, but from a DT bindings
point of view, should all new DTs require the property ?

> > +
> >  if:
> >    not:
> >      properties:
> 
> Unless I'm mistaken, you can't have identically named properties at the
> same level (multiple 'if' at the root level). You can group them in a
> 'allOf' property.
Marek Vasut May 25, 2021, 10:31 a.m. UTC | #3
On 5/18/21 1:02 AM, Laurent Pinchart wrote:
> Hi Marek,

Hi,

> Thank you for the patch.
> 
> On Sat, May 15, 2021 at 10:46:55PM +0200, Marek Vasut wrote:
>> Decoder input LVDS format is a property of the decoder chip or even
>> its strapping. Add DT property data-mapping the same way lvds-panel
>> does, to define the LVDS data mapping.
> 
> The information could also be derived by the driver from the compatible
> string in the case when this is an intrinsic property of the device (or
> when it's configurable by software), but the fact that it can also be
> controlled by strapping makes a DT property needed. We may want to limit
> the usage of the DT property to the strapping case though, but I don't
> have a real preference here, so I'm fine with this approach.

You'd soon end up piling up compatible strings like the panel-simple 
does, I don't think that scales. Besides, there are bridges where you 
can pick the mapping with a strap, and then the compatible string is no 
longer enough.

[...]
Marek Vasut May 25, 2021, 10:33 a.m. UTC | #4
On 5/18/21 1:03 AM, Laurent Pinchart wrote:

[...]

>>> +if:
>>> +  not:
>>> +    properties:
>>> +      compatible:
>>> +        contains:
>>> +          const: lvds-decoder
>>> +then:
>>> +  properties:
>>> +    data-mapping: false
> 
> Should we make the property required for lvds-decoder ? We need to
> support backward compatibility in the driver, but from a DT bindings
> point of view, should all new DTs require the property ?

I think so.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
index f4dd16bd69d2..f0abb94f8f2e 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
@@ -64,6 +64,15 @@  properties:
       - port@0
       - port@1
 
+  data-mapping:
+    enum:
+      - jeida-18
+      - jeida-24
+      - vesa-24
+    description: |
+      The color signals mapping order. See details in
+      Documentation/devicetree/bindings/display/panel/lvds.yaml
+
   pclk-sample:
     description:
       Data sampling on rising or falling edge.
@@ -79,6 +88,16 @@  properties:
 
   power-supply: true
 
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          const: lvds-decoder
+then:
+  properties:
+    data-mapping: false
+
 if:
   not:
     properties: