• Maxime Ripard's avatar
    drm/vc4: Fix non-blocking commit getting stuck forever · 0c250c15
    Maxime Ripard authored
    In some situation, we can end up being stuck on a non-blocking that went
    through properly.
    
    The situation that seems to trigger it reliably is to first start a
    non-blocking commit, and then right after, and before we had any vblank
    interrupt), start a blocking commit.
    
    This will lead to the first commit workqueue to be scheduled, setup the
    display, while the second commit is waiting for the first one to be
    completed.
    
    The vblank interrupt will then be raised, vc4_crtc_handle_vblank() will
    run and will compare the active dlist in the HVS channel to the one
    associated with the crtc->state.
    
    However, at that point, the second commit is waiting using
    drm_atomic_helper_wait_for_dependencies that occurs after
    drm_atomic_helper_swap_state has been called, so crtc->state points to
    the second commit state. vc4_crtc_handle_vblank() will compare the two
    dlist addresses and since they don't match will ignore the interrupt.
    
    The vblank event will never be reported, and the first and second commit
    will wait for the first commit completion until they timeout.
    
    The underlying reason is that it was never safe to do so. Indeed,
    accessing the ->state pointer access synchronization is based on
    ownership guarantees that can only occur within the functions and hooks
    defined as part of the KMS framework, and obviously the irq handler
    isn't one of them. The rework to move to generic helpers only uncovered
    the underlying issue.
    
    However, since the code path between
    drm_atomic_helper_wait_for_dependencies() and
    drm_atomic_helper_wait_for_vblanks() is serialised and we can't get two
    commits in that path at the same time, we can work around this issue by
    setting a variable associated to struct drm_crtc to the dlist we expect,
    and then using it from the vc4_crtc_handle_vblank() function.
    
    Since that state is shared with the modesetting path, we also need to
    introduce a spinlock to protect the code shared between the interrupt
    handler and the modesetting path, protecting only our new variable for
    now.
    
    Link: https://lore.kernel.org/all/YWgteNaNeaS9uWDe@phenom.ffwll.local/
    Link: https://lore.kernel.org/r/20211025141113.702757-3-maxime@cerno.tech
    Fixes: 56d1fe09 ("drm/vc4: Make pageflip completion handling more robust.")
    Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: default avatarMaxime Ripard <maxime@cerno.tech>
    0c250c15
vc4_hvs.c 20.6 KB