Re: [PATCH v2 17/21] drm/imx: pd: Use bus format/flags provided by the bridge when available

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 27 Aug 2019 14:51:20 +0200
Philipp Zabel <p.zabel@pengutronix.de> wrote:
 
> [...]
> > > > I can do that if you like. Note that we are forwarding
> > > > the ->output_bus_cfg.fmt value to the IPU DI, not ->input_bus_cfg.fmt.
> > > > I just assumed that input format wouldn't be used in the dummy bridge
> > > > element (the one embedded in the encoder) since encoders only have an
> > > > output end (input port is likely to be a SoC specific link between the
> > > > CRTC and the encoder which we probably don't need/want to expose).    
> > > 
> > > Then why (would this driver have to) implement get_input_fmts at all?  
> > 
> > That's the only way we can check if an output format is supported: bus
> > format negotiation is based on a trial and error logic that starts from
> > the end of the pipeline (last bridge element) and goes backward until
> > it reaches the first bridge (the dummy encoder bridge). A bus format
> > setting is validated when all ->get_input_bus_fmts() hooks returned at
> > least one possible format (*num_input_formats > 0) for the output format
> > being tested by the next element in the chain. And that's why even the
> > dummy encoder bridge has to implement this hook unless it only supports
> > one output format (the core returns MEDIA_BUS_FMT_FIXED when  
> > ->get_input_bus_fmts is NULL).  
> 
> This function (currently) also only returns MEDIA_BUS_FMT_FIXED, so
> there is no difference in return value (if queried with a supported
> output_fmt).

There's a small difference actually: when output_fmt !=
MEDIA_BUS_FMT_FIXED, we make sure output_fmt is something we support. If
you don't implement ->get_input_bus_fmts() you'll just accept any format
requested by the next element in the pipeline.

> select_bus_fmt_recursive could just check atomic_get_output_bus_fmts if
> that is implemented, but atomic_get_input_bus_fmts isn't.

I'd like to use ->get_output_bus_fmts() only to retrieve supported
output formats on the last bridge element, otherwise it makes the
whole thing even more complex.

> 
> That point is moot though, if we propagate the output format to the
> input format.

I'll do that then (and mandate that all encoder drivers do the same).

> 
> [...]
> > > > As said above, we need a way to support bridge chains where not all
> > > > elements support negotiating the bus format, that's what this fallback
> > > > is for, but maybe 0 is not an appropriate value to mean 'pick the
> > > > default format'.    
> > > 
> > > We'd actually have to pick one here. If set, that should be
> > > imxpd->bus_format.  
> > 
> > What if imxpd->bus_format is 0? Should I return -EINVAL?  
> 
> I think so. That would certainly be easier to debug than "strange
> colours" when hooking up a bridge that is incompatible with whatever
> we'd choose.

Okay.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel