Hi, On Thu, 5 Nov 2026 at 01:01, Cristian Ciocaltea <[email protected]> wrote: > vop2_crtc_atomic_enable() prepares vp->dclk and, for the first CRTC, > calls vop2_enable(), incrementing the shared vop2->enable_count before > programming VOP2 registers. Two later failure points return early with > only vop2_unlock(), leaking the dclk prepare, the enable_count > increment, and the core clocks / PM runtime reference from > vop2_enable(). > > Since the disable path decrements enable_count unconditionally, the > leaked increment skews the count, causing an underflow or premature > shutdown of core clocks and PM runtime while another CRTC is active. > > Moreover, cleaning up on the enable error paths alone is insufficient. > A CRTC whose atomic_enable() bailed is still committed as active, so > vop2_crtc_atomic_disable() still runs its full teardown, releasing the > same dclk prepare and enable_count a second time, thus underflowing both > counts and, where vop2_enable() ran, touching VOP registers with core > clocks gated. > > Fix this by reverting exactly what was set up on failure, and track the > per-VP bring-up state so atomic_disable() skips teardown when > atomic_enable() never completed. Additionally, ensure any further > hardware programming is also skipped within the affected RPM commit > tail, i.e. in the callbacks gated by drm_atomic_helper_commit_planes(): > vop2_crtc_atomic_begin(), vop2_plane_atomic_update{update|disable}() and > vop2_crtc_atomic_flush().
This one is: Acked-by: Daniel Stone <[email protected]> I don't think it's the world's most complete fix which covers every corner case, but on the other hand, building out the tracking infrastructure to do that, for cases which will almost never occur in practice, is going to make the driver pretty unmaintainable. This is pretty much as far as I think it makes sense to go. Cheers, Daniel
