On Tue, 27 Aug 2019 14:51:20 +0200 Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
