[PATCH v3 4/5] dt-bindings: display: add data-mapping to panel-dpi

[Laurent Pinchart]

Hi Sam,

On Tue, Feb 18, 2020 at 11:16:38PM +0100, Sam Ravnborg wrote:
> On Tue, Feb 18, 2020 at 02:13:45PM -0600, Rob Herring wrote:
> > On Sun, Feb 16, 2020 at 12:15 PM Sam Ravnborg <sam at ravnborg.org> wrote:
> > >
> > > Add data-mapping property that can be used to specify
> > > the media format used for the connection betwwen the
> > > display controller (connector) and the panel.
> > > Signed-off-by: Sam Ravnborg <sam at ravnborg.org>
> > 
> > Missing blank line.
> > 
> > > ---
> > >  .../devicetree/bindings/display/panel/panel-dpi.yaml | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > index 40079fc24a63..6a03d2449701 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > +++ b/Documentation/devicetree/bindings/display/panel/panel-dpi.yaml
> > > @@ -21,6 +21,16 @@ properties:
> > >        - {}
> > >        - const: panel-dpi
> > >
> > > +  data-mapping:
> > > +    enum:
> > > +      - rgb24
> > > +      - rgb565
> > > +      - bgr666
> > > +      - lvds666
> > 
> > Doesn't lvds666 come from i.MX IPU which as I remember has built-in
> > LVDS block? I'd think this format would be implicit when using the
> > LVDS block and panel. It doesn't seem this is actually used anywhere
> > either.
>
> I must admit that I just copied this list from Oleksandrs original
> patch. The MEDIA type it identifies(MEDIA_BUS_FMT_RGB666_1X24_CPADHI) looks special.
> I will drop lvds666 while applying, unless I get other feedback.
> (Note: travelling, earliest in the weekend)

There are different data mappings defined for LVDS, we should follow
them. lvds666 is wrong in any case, and doesn't apply to a DPI panel
anyway.

I don't like the name data-mapping much for DPI panels I'm afraid. It
made sense for LVDS as it's really about how the different data bits are
mapped to LVDS time slots, but for DPI, what we need to describe is the
format. I also wonder whether multiple formats wouldn't be required when
the panel supports more than one, but that may not apply to panels
covered by these bindings.

If a panel expects RGB888 and receives RGB666 with the two LSBs of each
component hardwired to GND on the PCB, should DT report RGB888 or RGB666
on the panel side ? I'm tempted by the former, and specifying the latter
on the transmitting side.

Please also note that this case is already described by
Documentation/devicetree/bindings/media/video-interfaces.txt through two
entirely different properties, bus-width and data-shift. I think we
should try to standardize mappings between display and capture. This new
property should be reconsidered in my opinion, I think it was merged too
soon.

> Btw. anyway I can add data-mapping to panel-common - and then list the
> allowed enum values in each binding?
> 
> I would love to have a central definition of data-mapping, and then let
> the users only allow the relevant subset so we catch errors in DT files
> early.

-- 
Regards,

Laurent Pinchart