[3/8] dt-bindings: display: mxsfb: Add a bus-width endpoint property
diff mbox series

Message ID 20200813012910.13576-4-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • drm: mxsfb: Allow overriding bus width
Related show

Commit Message

Laurent Pinchart Aug. 13, 2020, 1:29 a.m. UTC
When the PCB routes the display data signals in an unconventional way,
the output bus width may differ from the bus width of the connected
panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0],
G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10]
and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the
signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].

Add a bus-width property to describe this data routing.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/display/mxsfb.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Guido Günther Aug. 15, 2020, 9:28 p.m. UTC | #1
Hi Laurent,
On Thu, Aug 13, 2020 at 04:29:05AM +0300, Laurent Pinchart wrote:
> When the PCB routes the display data signals in an unconventional way,
> the output bus width may differ from the bus width of the connected
> panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0],
> G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10]
> and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the
> signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
> 
> Add a bus-width property to describe this data routing.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/display/mxsfb.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
> index ec6533b1d4a3..d15bb8edc29f 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.yaml
> +++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
> @@ -58,6 +58,18 @@ properties:
>          type: object
>  
>          properties:
> +          data-shift:
Shouldn't that be bus-width ?
 -- Guido

> +            enum: [16, 18, 24]
> +            description: |
> +              The output bus width. This value overrides the configuration
> +              derived from the connected device (encoder or panel). It should
> +              only be specified when PCB routing of the data signals require a
> +              different bus width on the LCDIF and the connected device. For
> +              instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
> +              B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and
> +              LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
> +              LCD_DATA[17:12], bus-width should be set to 24.
> +
>            remote-endpoint:
>              $ref: /schemas/types.yaml#/definitions/phandle
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>
Sam Ravnborg Aug. 16, 2020, 7:25 a.m. UTC | #2
Hi Laurent.

On Thu, Aug 13, 2020 at 04:29:05AM +0300, Laurent Pinchart wrote:
> When the PCB routes the display data signals in an unconventional way,
> the output bus width may differ from the bus width of the connected
> panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0],
> G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10]
> and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the
> signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
> 
> Add a bus-width property to describe this data routing.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Some general comments.
We have the bus format - for example MEDIA_BUS_FMT_RGB666_1X18.
I was under the impression that the bus format defined the wiring used,
so for example the bus format would tell if on used 18 bits as above.
So with the bus format available the bus-width is not needed.

Today this detail is not expressed in DT but comes based on the
compatible for the panel - so what this patch does is to add the bus
format information to DT.

But then to do so would we not need to have something so we can specify
all relevant MEDIA_BUS_FMT's? bus-width will not cut it.

Also the bus-width property (and data-shift property you accidentally
reference) are both described in media/video-interfaces.txt.
If we need bus-witdh - is it possible to re-use video-interfaces?
It may need a conversion to yaml to get full validation, but a lot
of .yaml files refer to the text file today so conversion can come later.

	Sam


> ---
>  Documentation/devicetree/bindings/display/mxsfb.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
> index ec6533b1d4a3..d15bb8edc29f 100644
> --- a/Documentation/devicetree/bindings/display/mxsfb.yaml
> +++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
> @@ -58,6 +58,18 @@ properties:
>          type: object
>  
>          properties:
> +          data-shift:
> +            enum: [16, 18, 24]
> +            description: |
> +              The output bus width. This value overrides the configuration
> +              derived from the connected device (encoder or panel). It should
> +              only be specified when PCB routing of the data signals require a
> +              different bus width on the LCDIF and the connected device. For
> +              instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
> +              B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and
> +              LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
> +              LCD_DATA[17:12], bus-width should be set to 24.
> +
>            remote-endpoint:
>              $ref: /schemas/types.yaml#/definitions/phandle
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Laurent Pinchart Aug. 17, 2020, 12:09 a.m. UTC | #3
Hi Guido,

On Sat, Aug 15, 2020 at 11:28:38PM +0200, Guido Günther wrote:
> On Thu, Aug 13, 2020 at 04:29:05AM +0300, Laurent Pinchart wrote:
> > When the PCB routes the display data signals in an unconventional way,
> > the output bus width may differ from the bus width of the connected
> > panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0],
> > G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10]
> > and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the
> > signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
> > 
> > Add a bus-width property to describe this data routing.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  Documentation/devicetree/bindings/display/mxsfb.yaml | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > index ec6533b1d4a3..d15bb8edc29f 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.yaml
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > @@ -58,6 +58,18 @@ properties:
> >          type: object
> >  
> >          properties:
> > +          data-shift:
>
> Shouldn't that be bus-width ?

Absolutely. I'll fix that.

> > +            enum: [16, 18, 24]
> > +            description: |
> > +              The output bus width. This value overrides the configuration
> > +              derived from the connected device (encoder or panel). It should
> > +              only be specified when PCB routing of the data signals require a
> > +              different bus width on the LCDIF and the connected device. For
> > +              instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
> > +              B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and
> > +              LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
> > +              LCD_DATA[17:12], bus-width should be set to 24.
> > +
> >            remote-endpoint:
> >              $ref: /schemas/types.yaml#/definitions/phandle
> >
Laurent Pinchart Aug. 17, 2020, 12:17 a.m. UTC | #4
Hi Sam,

On Sun, Aug 16, 2020 at 09:25:20AM +0200, Sam Ravnborg wrote:
> On Thu, Aug 13, 2020 at 04:29:05AM +0300, Laurent Pinchart wrote:
> > When the PCB routes the display data signals in an unconventional way,
> > the output bus width may differ from the bus width of the connected
> > panel or encoder. For instance, when a 18-bit RGB panel has its R[5:0],
> > G[5:0] and B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10]
> > and LCD_DATA[23:18], the output bus width is 24 instead of 18 when the
> > signals are routed to LCD_DATA[5:0], LCD_DATA[11:6] and LCD_DATA[17:12].
> > 
> > Add a bus-width property to describe this data routing.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Some general comments.
> We have the bus format - for example MEDIA_BUS_FMT_RGB666_1X18.
> I was under the impression that the bus format defined the wiring used,
> so for example the bus format would tell if on used 18 bits as above.
> So with the bus format available the bus-width is not needed.
> 
> Today this detail is not expressed in DT but comes based on the
> compatible for the panel - so what this patch does is to add the bus
> format information to DT.
> 
> But then to do so would we not need to have something so we can specify
> all relevant MEDIA_BUS_FMT's? bus-width will not cut it.

Different formats can be transported with a given wiring (for instance
when the full 24-bit RGB bus is wired, the display controller can ouput
24-bit RGB, or 18-bit RGB on the MSB with dithering enabled), and
different wirings can transport the same format
(MEDIA_BUS_FMT_RGB666_1X18 can be transported with a 24 bits wiring, or
a 18 bits wiring as described in the commit message). While we may in
this case be able to express the information through bus formats
(specifying MEDIA_BUS_FMT_RGB666_1X18 would imply that only 18 bits are
wired), I think it's conceptually speaking the wrong information to
provide, and would be confusing in some cases.

> Also the bus-width property (and data-shift property you accidentally
> reference) are both described in media/video-interfaces.txt.
> If we need bus-witdh - is it possible to re-use video-interfaces?
> It may need a conversion to yaml to get full validation, but a lot
> of .yaml files refer to the text file today so conversion can come later.

How do you mean reuse ? I can reference the file in the description, but
as video-interfaces.txt documents the property just as "number of data
lines actively used, valid for the parallel busses", I would need a
description here anyway, to explain how it applies to this device in
particular, even after video-interfaces.txt gets converted to YAML.

> > ---
> >  Documentation/devicetree/bindings/display/mxsfb.yaml | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > index ec6533b1d4a3..d15bb8edc29f 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.yaml
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > @@ -58,6 +58,18 @@ properties:
> >          type: object
> >  
> >          properties:
> > +          data-shift:
> > +            enum: [16, 18, 24]
> > +            description: |
> > +              The output bus width. This value overrides the configuration
> > +              derived from the connected device (encoder or panel). It should
> > +              only be specified when PCB routing of the data signals require a
> > +              different bus width on the LCDIF and the connected device. For
> > +              instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
> > +              B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and
> > +              LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
> > +              LCD_DATA[17:12], bus-width should be set to 24.
> > +
> >            remote-endpoint:
> >              $ref: /schemas/types.yaml#/definitions/phandle
> >

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
index ec6533b1d4a3..d15bb8edc29f 100644
--- a/Documentation/devicetree/bindings/display/mxsfb.yaml
+++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
@@ -58,6 +58,18 @@  properties:
         type: object
 
         properties:
+          data-shift:
+            enum: [16, 18, 24]
+            description: |
+              The output bus width. This value overrides the configuration
+              derived from the connected device (encoder or panel). It should
+              only be specified when PCB routing of the data signals require a
+              different bus width on the LCDIF and the connected device. For
+              instance, when a 18-bit RGB panel has its R[5:0], G[5:0] and
+              B[5:0] signals connected to LCD_DATA[7:2], LCD_DATA[15:10] and
+              LCD_DATA[23:18] instead of LCD_DATA[5:0], LCD_DATA[11:6] and
+              LCD_DATA[17:12], bus-width should be set to 24.
+
           remote-endpoint:
             $ref: /schemas/types.yaml#/definitions/phandle