On Tue, 2019-08-27 at 15:20 +0200, Boris Brezillon wrote: > 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. I see. My whole point was predicated on the incorrect assumption that get_input_bus_fmts would not have to be fed usupported output_fmts. > > 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. Ok, I should have paid more attention to the documentation in patch 16. > > 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). Sounds good. At least then there aren't two classes of bridges that implement the negotiation callbacks differently. regards Philipp _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel