| Message ID | 20200813012910.13576-4-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: mxsfb: Allow overriding bus width | expand |
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 >
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
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 > >
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 > >
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
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(+)