Hi, On 28/10/2025 16:27, Linus Walleij wrote: > On Tue, Oct 28, 2025 at 11:32 AM Tomi Valkeinen > <[email protected]> wrote: > >>> As the CRTC is no longer online at bridge_pre_enable() >>> and gone at brige_post_disable() which maps to the panel >>> bridge .prepare()/.unprepare() callbacks, any CRTC that >>> enable/disable the DSI transmitter in it's enable/disable >>> callbacks will be unable to send any DSI commands in the >>> .prepare() and .unprepare() callbacks. >> >> Well, that wasn't the intention... >> >> We also have this pre_enable_prev_first hack in the drm_bridge, but I >> guess that doesn't help in this case? > > I don't know what that's for, the panel bridge isn't > using it so it doesn't help any panel driver?
What do you mean? drivers/gpu/drm/bridge/panel.c uses it, see "panel->prepare_prev_first". I do find it confusing, and I'm not sure if I should say more about it until I read the code to refresh my memory =) But I think the idea is that the panel's prepare is called after the DSI hosts pre_enable. Probably that doesn't help here, as you need the crtc to be active, and, afaik, the point with the whole pre-enable/prepare sequence is that the video is not active at that point: * The display pipe (i.e. clocks and timing signals) feeding this bridge * will not yet be running when the @atomic_pre_enable is called. >> More generally speaking, I think the core issue is that we have the DSI >> video stream as a dependency for DSI commands. > > That's right, I think. > >> It would be better if the >> DSI hosts dealt with DSI commands independent of the video stream, in >> other words, one could send a DSI command at any time. >> >> Unfortunately in some cases that might be impossible, if the DSI host >> depends on the incoming pixel clock to function... > > This is the case with the MCDE driver. It even cannot use the > existing .enable/.disable callbacks in it's bridge because it really > needs to bring the DSI transmission block up at the right time > in the initialization sequence. > > I'm not making it up: I really, really tried hard to just initialized > it independently. It just won't, it's too tightly coupled with the > DSI video/command stream generator. > >> In this particular case, if the same driver is dealing with the crtc and >> the dsi host, wouldn't it be possible to make the >> mipi_dsi_host_ops.transfer() work regardless of the enable/disable status? > > Sadly no. > > FWIW I think it is fine to require the DSI panels to only send > DSI commands in .enable/.disable and not in .prepare/.unprepare, > it's intuitive in a way, if we go for this semantic I can send a doc > update after the fixes and try to look over the panel drivers a bit > and see if there are more offenders. No, I don't think that works generally. In panel's enable() the video stream is already on, and the DSI link is in HS mode. You might need to configure the panel first (at least the first part of the setup) in LP mode. Say, to set the number of lanes. Or, see "[email protected]", tc358768 can send long DSI commands, but only if the video stream is not enabled. Conceptually it makes sense to do configuration in panel's prepare(), and do the final enablement in panel's enable(). But unfortunately I don't have any good ideas how to sort this out. Feels like whatever we do, it's not ok for some bridge/panel... Tomi
