On Wed, Jun 24, 2026 at 11:06:49AM +0200, Hans de Goede wrote: > 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.
So I'm not really sure you need a hand-over, and you kind of rejected doing a proper one anyway. I see two issues there: - simpledrm doesn't safely disable its resources on QC SoCs - there's no way to claim the resources from simpledrm after the main drm driver is active. And you're trying to work around the first one with the second one. The second one is generic to me, so it shouldn't be an msm (or simpledrm) specific solution. The first one is SoC-specific, and fixing the second won't prevent completely the first. If you unbind simpledrm while it's active, you'll get the same issue, and you can't rely on the msm driver being always there to cover for you. So we need to fix both. We should discuss the second part with Thomas really, but for the first one, simple-framebuffer having its own compatible will allow you to trigger any coordinated poweroff sequence. Maxime
signature.asc
Description: PGP signature
