• Nicholas Kazlauskas's avatar
    drm/amd/display: Don't replace the dc_state for fast updates · bd200d19
    Nicholas Kazlauskas authored
    [Why]
    DRM private objects have no hw_done/flip_done fencing mechanism on their
    own and cannot be used to sequence commits accordingly.
    
    When issuing commits that don't touch the same set of hardware resources
    like page-flips on different CRTCs we can run into the issue below
    because of this:
    
    1. Client requests non-blocking Commit #1, has a new dc_state #1,
    state is swapped, commit tail is deferred to work queue
    
    2. Client requests non-blocking Commit #2, has a new dc_state #2,
    state is swapped, commit tail is deferred to work queue
    
    3. Commit #2 work starts, commit tail finishes,
    atomic state is cleared, dc_state #1 is freed
    
    4. Commit #1 work starts,
    commit tail encounters null pointer deref on dc_state #1
    
    In order to change the DC state as in the private object we need to
    ensure that we wait for all outstanding commits to finish and that
    any other pending commits must wait for the current one to finish as
    well.
    
    We do this for MEDIUM and FULL updates. But not for FAST updates, nor
    would we want to since it would cause stuttering from the delays.
    
    FAST updates that go through dm_determine_update_type_for_commit always
    create a new dc_state and lock the DRM private object if there are
    any changed planes.
    
    We need the old state to validate, but we don't actually need the new
    state here.
    
    [How]
    If the commit isn't a full update then the use after free can be
    resolved by simply discarding the new state entirely and retaining
    the existing one instead.
    
    With this change the sequence above can be reexamined. Commit #2 will
    still free Commit #1's reference, but before this happens we actually
    added an additional reference as part of Commit #2.
    
    If an update comes in during this that needs to change the dc_state
    it will need to wait on Commit #1 and Commit #2 to finish. Then it'll
    swap the state, finish the work in commit tail and drop the last
    reference on Commit #2's dc_state.
    
    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204181
    Fixes: 004b3938 ("drm/amd/display: Check scaling info when determing update type")
    Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
    Acked-by: default avatarAlex Deucher <alexander.deucher@amd.com>
    Reviewed-by: default avatarDavid Francis <david.francis@amd.com>
    Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
    bd200d19
amdgpu_dm.c 206 KB