On Wed, May 27, 2026 at 02:21:50PM +0200, Hans de Goede wrote: > >> clk_prepare_enable() is fine, the clocks are already on so no > >> ordering worries and it ensures that the clocks and their parents cannot > >> get turned off by incrementing their enable, prepare and protect counts. > >> > >> clk_disable_unprepare() is a problem though since it actually turns the > >> clocks off. Instead something is needed which only decrements those counts. > >> > >> This series introduces a new clk-core function called > >> __clk_disable_unprepare_counts_only() (1) which does just that. Prefixed > >> with '__' to indicate that normally drivers should not use this. > >> > >> Michael, Stephen sorry for needing to add a new clk-core function for this, > >> but I see no other way of solving this (2)(3). The changes are not that > >> big / not too bad. > >> > >> I've also considered making __clk_disable_unprepare_counts_only() implement > >> all the logic itself instead of adding the extra parameter to > >> clk_core_unprepare() and clk_core_disable() but that leads in duplicating > >> quite a bit of logic (4) so this seems better. > >> > >> The other 2 patches just replace the clk_disable_unprepare() calls in > >> the simple[fb|drm] driver with the new helper. > >> > >> This series fixes the display not coming up after switching to the msm > >> driver when a simple-framebuffer node with clocks listed is used. > >> > >> 1) I'm open to changing the name, this is the best I could come up with. > >> > >> 2) One option considered was detaching the simple-framebuffer driver later, > >> after the real display driver has had a chance to claim the clocks. But > >> this won't work in cases where the real display driver picks different > >> parent clocks then the boot firmware did and needs to reparent clocks. > >> > >> Basically the goal is for things to behave as if the simple-framebuffer > >> driver was not there at all, because that leaves the hw in the state > >> the real display driver expects. > > > > So I know it's not really what you had in mind, but if you are only > > concerned about the transition from the bootloader to the DRM driver, > > then I think supporting the following work would help: > > > > https://lore.kernel.org/r/[email protected] > > > > With that series, we can build the initial KMS state from the hardware > > state, which means that if you were to change the resolution at boot, it > > would be executed just like any mode change in KMS. > > Hmm, so your suggestion would be to have the initial KMS driver probe() > only do a read back without touching anything. Then claim clks matching > the read back config and then only release the simple* driver and thus > the clocks after this?
Almost. Part of the call to drm_mode_config_create_initial_state() if needed to make it grab its resources. See atomic_install_state in that series. So probe wouldn't have anything more to do. > That is certainly an interesting proposal but IMHO almost orthogonal > to this one (1). Why do you think it's orthogonal? It would completely fix your problem. > Even if it may fix the issue in the end, it seems that that work is > still quite a way from going upstream It's likely to be merged in the next few weeks. > and even then initially it only potentially fixes this for the TIDSS > driver since that solution needs a lot of per driver work. I mean, yeah, but the good thing is that you only have one driver to care about, right? > Where as my proposed fix here is much simpler and fixes the issue > for all drivers in one go. At the expense of exposing an unsafe function for only a single user and usecase. Maxime
signature.asc
Description: PGP signature
