On Tue, 6 Oct 2020 10:07:39 +0300 Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote: > Adding Boris who added bus format negotiation. > > On 06/10/2020 00:31, Nikhil Devshatwar wrote: > > Hi all, > > > > I am trying to convert the upstream tidss drm driver to new > > connector model. > > The connector is getting created by the tidss driver and bridges are > > attached with flag DRM_BRIDGE_ATTACH_NO_CONNECTOR > > Here are some questions, regarding this: > > I was looking at this a bit, and below is my understanding. And I'm mostly talking about how things > should be with new code, not legacy code. Things are probably a bit more complex if you mix bridges > which implement different styles on how to deal with bus formats. > > > 1) Most of the info regarding bus_format and bus flags is coming from > > the bridges. Is it okay to not populate connector->display_info with > > bus_format and flags? > > drm_display_info describes the connected display and what goes on the wire to the display. > > For monitors that's quite clear, and the data in display_info would reflect what the last bridge > needs to output. Most of the data comes from EDID, but I think bus format and flags do not. So a > bridge would need to fill them in, which doesn't make sense when we have a chain of bridges (which > would be the bridge to fill the data?). So for monitors, I think bus flags and formats in > display_info are unused. > > For panels, I'm not sure. We have the bridge/panel.c which wraps the actual panel driver, so afaics > the panel is essentially the last bridge in the chain, and the connector is kind of a dummy > connector. But the panel driver fills in the display_info, and that's where the bridge/panel.c gets > the bus formats & flags for the negotiation. > > Probably the above could be changed so that the panels take part of the negotiation process, and > then the bus formats and flags fields in the display_info could be removed. Yep, that'd be better to have the bus format/flags info provided by the panel itself rather than passed through display info. > > > 2) The "drm_atomic_bridge_chain_select_bus_fmts" does the format > > negotiation. So is it okay for the encoder to simply pick the bus_format > > from the first bridge's state? > > Yes, I think that is the idea. The first bridge's input is what the display controller's encoder > should output, and the negotiation should take care to provide something in the first bridge's state > for the input. Exactly. > > > 3) What is the meaning of MEDIA_BUS_FMT_FIXED? Does it mean that the > > bridge does not change the format from input to output? > > I think it just means "undefined" here, and it's up to the drivers to decide what to do. I presume > this is mostly for drivers that don't support the new stuff, as each bridge should be able to tell > what formats & flags it supports. Correct. > > > 4) The bus_flags are available in bridge->timings->input_bus_flags and > > also in bridge_state->input_bus_cfg.flags. Which one should be used? > > I think bridge_state->input_bus_cfg. Although bridge->timings->input_bus_flags has some data that's > not in input_bus_cfg. If the drivers support the negotiation, I don't think > bridge->timings->input_bus_flags has any use. Oh, I didn't realize there was an input_bus_flags in the timings struct. We should probably propagate those in drm_atomic_bridge_propagate_bus_flags(). > > Probably bridge->timings->input_bus_flags should be used as a fallback. So if a bridge is asked to > use MEDIA_BUS_FMT_FIXED as output (i.e. the next bridge doesn't support negotiation), then the > bridge might use a default format and also see if the next bridge has bridge->timings->input_bus_flags. I think this could be automated in drm_atomic_bridge_propagate_bus_flags(). Right now we simply propagate the output bus flags to the input end [1], but it probably makes more sense to use the value in bridge->timings->input_bus_flags if present. [1]https://elixir.bootlin.com/linux/v5.9-rc8/source/drivers/gpu/drm/drm_bridge.c#L971 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
