Hi, On 24-Jun-26 10:53, Maxime Ripard wrote: > On Thu, May 28, 2026 at 03:02:09PM +0200, Hans de Goede wrote: >> On 28-May-26 2:08 PM, Maxime Ripard wrote: >>> On Thu, May 28, 2026 at 12:12:10PM +0200, Hans de Goede wrote: >>>> Hi Brian, >>>> >>>> On 28-May-26 1:03 AM, Brian Masney wrote: >>>>> Hi Hans, >>>>> >>>>> On Wed, May 27, 2026 at 11:48:08AM +0200, Hans de Goede wrote: >>>>>> 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. >>>>> >>>>> Why won't that work in the case where different parent clocks are >>>>> selected? >>>>> I'll describe a scenario below. >>>>> >>>>>> >>>>>> 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. >>>>> >>>>> I think the deferred unbinding could have some potential here where >>>>> there is some kind of notification mechanism between simple-framebuffer >>>>> and the real drm driver. So: >>>>> >>>>> - simple-framebuffer driver takes reference(s) to the clk(s). >>>>> >>>>> - real drm driver eventually loads, takes reference(s) to the necessary >>>>> clk(s). >>>>> >>>>> - real drm driver sends a notification to simple-framebuffer that it's >>>>> done, and has control. >>>>> >>>>> - simple-framebuffer can unbind and release its references to the clks. >>>>> >>>>> No clks will be shutdown prematurely in this scenario. >>>>> >>>>> If the real drm driver needs a different parent, then presumably things >>>>> should be setup correctly, and simple-framebuffer can have the clocks >>>>> shut down when it calls clk_disable_unprepare(). >>>> >>>> If the real drm driver needs a different parent, then how does it >>>> do the reparenting while the simple-framebuffer driver is holding >>>> a reference to the clock ? In that case the clock might have >>>> a protected_count of non 0 (depends on the core-clk flags) and >>>> reparenting won't work. >>> >>> The only case where it should reparent you listed was that you might >>> need to pick up a different resolution. However, that can only be >>> enforced by an ioctl or a client. >>> >>> simplefb/drm is removed in msm_drm_kms_init. The device is published >>> drm_dev_register called right after msm_drm_kms_init, and the clients >>> are registered in msm_drm_kms_post_init, called after drm_dev_register. >>> >>> There's no way in the current msm architecture to have a modeset happen >>> while the simpledrm driver is still active. >> >> Ok, new plan, please let me know what you think about this: >> >> 1. Add a new "disable" callback argument to >> devm_aperture_acquire_for_platform_device() and store this in >> struct aperture_range >> >> 2. Add a new aperture_disable_conflicting_devices() which >> calls the disable callback for matching devices. >> >> 3. Have the simple[fd|drm] drivers implement a disable callback >> which unregisters the drm dev and releases any claims on the >> aperture mem-region, while keeping clks, regulators, etc. >> enabled. And have them check if disable was called on remove() >> and if not do the disable() things on remove(0 >> >> 4. Have msm call aperture_disable_conflicting_devices() where it >> now call aperture_remove_conflicting_devices() and call >> aperture_remove_conflicting_devices() at the point where it has >> claimed any clks it needs. >> >> Does this sound like something which would be acceptable ? > > Generally speaking, yes, but I'd also like to understand what you're > trying to fix exactly, because I don't see how it can be what you hinted > at before.
The basic problem is that on handover from simpledrm -> msm the clocks get turned off, without fully shutting down the display pipeline first (since simpledrm does not know how). Then when msm later tries to re-enable things, the hw is in a confused state and we hit this error: [ 2.980181] disp_cc_mdss_dptx3_pixel0_clk_src: rcg didn't update its configuration. [ 2.980272] WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xdc/0x100 So we need a way to handover things to the msm driver without anything getting disabled by simpledrm in an uncontrolled (not following proper pwr-down seq) manner. The idea behind the 2 step simpledrm removal process is that on the first step drm_dev_unplug() gets called for the simpledrm device and any struct resource claim on the aperture gets released. While keeping all the clocks, regulators, etc. claims. Then msm can take things over and call the second step when it has taken everything over. On the second step simpeldrm will release its enable count ref on the clocks, etc. in question. In the mean time I've actually already prototyped this, see: https://github.com/jwrdegoede/linux-sunxi/commits/main Note this is my main wip branch with all kinda stuff in there and this gets forced pushes regularly. Regards, Hans p.s. I've not posted this upstream yet because unlike the __clk_disable_unprepare_counts_only() approach this does not work atm, because it seems the msm driver only claims some DP related clocks on the first modeset from userspace rather then during probe(). This msm issue is something which we need to fix regardless, also for when the clock framework gets moved to using sync-state handling for turning off unused clocks, since any clocks consumed by the msm dt-node will then also get turned off directly after probe().
