On 2/24/22 16:40, Maxime Ripard wrote:
Hi,
Hi,
On Sat, Feb 19, 2022 at 01:28:37AM +0100, Marek Vasut wrote:This patch series attempts to address a problem of missing support for DSI bridge-to-bridge or panel-to-bridge clock frequency negotiation. The problem has two variants. First, a DSI->to->x bridge derives its own internal clock from DSI HS clock, but the DSI HS clock cannot be set to arbitrary values. TS358767 is one such bridge in case it operates without Xtal. In that case, the TC358767 driver must be able to negotiate the specific suitable DSI HS clock frequency for the chip. Second, both DSI->to->x bridges and DSI hosts currently calculate, or rather guess and hope they both guess the same number as their neighbor, the DSI HS clock frequency from form of PLL=(width * height * bpp / lanes / 2). This is dangerous, since the PLL capabilities on both ends of the DSI bus might differ and the DSI host could easily end up generating wildly different clock than what the DSI bridge/panel expects to receive. This series attempts to address these negotiation problems by extending the existing .atomic_get_input_bus_fmts callback into .atomic_get_input_bus_cfgs callback in struct drm_bridge_funcs {}. The extended version returns not only a list of a list of bus formats supported by a bridge, but the entire list of struct drm_bus_cfg, which currently contains format and bus flags, but can be extended with other members, like desired clock frequency, as required. This series demonstrates such extension by adding the support for negotiating the DSI clock and by implementing such support in DW DSI Host and TC358767 DSI bridge.We discussed it a bit on IRC as well but there's another issue with this, let's imagine this setup: encoder -> DSI-to-DPI bridge -> DPI-to-HDMI bridge -> HDMI Monitor HDMI is fairly favorable, and it would probably use pixel clocks of either 148.5, 297 or 594MHz. Let's simplify this a bit and assume your DSI-to-DPI bridge can only operate at a frequency equivalent to 297MHz. 594Mhz is going to be used by those new fancy monitors, and thus the preferred mode is likely to be using 594MHz. With your solution, it effectively means that when the system will boot up, the preferred mode will be reported to the userspace (and the fbdev emulation), whatever is coming next is going to use it, and you're just going to... refuse it because it never worked in the first place. You'll leave a blank display, and that's it. That's not a great behavior, really.
If you cannot support such a panel with this kind of scanout engine, what else would you do than blank screen ?
And since you don't get a state until you start a commit, this would need to be able to work without one. Of course, some state parameters will affect the clock (like the bpc count) so it won't be perfect, but we can at least try. Another thing is that the clock that needs to be negociated is likely to be device specific. It's probably going to be fairly similar across similar devices (like all the DSI bridges you mentioned are using the HS clock), but I'm not sure we can make that assumption.
The bridge (data sink) should be able to figure out what kind of clock it needs from the source and then request those, yes. With DSI you can make an assumption about what kind of clock frequencies each link mode would require, but in general, you cannot assume much.
I think we could make something that work by asking the previous bridge in the chain for a given clock rate with a given mode, and then filter out / adjust anything we don't like. It would then be able to first check if it can provide that clock in the first place, and then the rate it has, and would be free to forward the query up to the encoder. And since it's tied to the mode, it would work with mode_valid too.
It seems to me this is similar to this solution, except it must happen when the mode is available ? But then the question also comes to mind, should select_bus_fmt_recursive() be called only after mode is available too ?
