[PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling

[Boris Brezillon]

On Mon, 9 Mar 2020 21:59:26 +0200
Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:

> Hi Boris,
> 
> On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote:
> > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote:  
> > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote:  
> > > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote:    
> > > > > The bus_flags and bus_format handling logic does not seem to cover
> > > > > all potential usecases. Specifically, this seems to fail with an
> > > > > "edt,etm0700g0edh6" display attached to an 24bit display interface,
> > > > > with interface-pix-fmt = "rgb24" set in DT.    
> > > > 
> > > > interface-pix-fmt is a legacy property that was never intended to be
> > > > used as an override for the panel bus format. The bus flags were
> > > > supposed to be set from the display-timings node, back when there was no
> > > > of-graph connected panel at all.
> > > > 
> > > > That being said, there isn't really a proper alternative that allows to
> > > > override the bus format requested by the panel driver in the device tree
> > > > to account for weird wiring. We could reuse the bus-width endpoint
> > > > property documented in [1], but that wouldn't completely specify how the
> > > > RGB components are to be mapped onto the parallel bus.
> > > > 
> > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt    
> > > 
> > > Things are funny sometimes, I've run into the exact same problem with a
> > > different display controller today.
> > > 
> > > Shouldn't we use the data-shift property from [1] to specify this ?
> > > Combined with Boris' bus format negotiation for bridges, I think we
> > > would have all the components in place to solve this problem properly.  
> > 
> > I wonder if we shouldn't take more complex pin mappings into account
> > now and go directly for a data-mapping property describing those
> > mappings using a string. This way we'd have a single property that
> > would work for both fully parallel buses (DPI/RGB) and serial (or
> > partially parallel) ones (LVDS).  
> 
> I'm all for standardization, but I'm not sure data-mapping is the right
> property, at least with its current definition. It's really meant to
> describe how individual bits are mapped to the LVDS time slots. I'm fine
> extending it, but we need to define it clearly. How would you envision
> it being used in this case ?
> 

Well, clearly the data-width/data-shift approach does not solve all
problems: what do you do if the source R pins are connected to the sink
B pins? Well, the first answer would probably be 'have a serious
discussion with the HW designer responsible for this insanity' :-), but
once you've passed this 'WTF' stage, you'll have to find a way to tell
the source component it should use RGBxyx while the sink should use
BGRxyx (or vice-versa). This is something you can't extract that from
those width/shift props though. My suggestion would be to have one
string per MEDIA_BUS_FMT definition, so we can force things at the DT
level if we really have to. That's basically what the interface-pix-fmt
property was doing, except we would standardize the prop and values and
probably provide helpers so bridge elements don't have to parse this
prop manually.