Skip to content
  • Nicholas Kazlauskas's avatar
    lib/igt_kms: Fix commits for planes with multiple possible CRTCs · ed944b45
    Nicholas Kazlauskas authored and Harry Wentland's avatar Harry Wentland committed
    
    
    An igt_plane_t is defined per igt_pipe_t. It is treated as its
    own independent resource but DRM planes can be exposed to being used on
    a number of possible CRTCs - making it a shared resource.
    
    In IGT planes with multiple possible CRTCs are added to the plane list
    for each pipe that the plane supports. The internal state remains
    independent in IGT so when the same plane is modified for multiple
    pipes in a single commit the last pipe to modify it is the one whose
    state gets fully applied.
    
    This situation happens fairly often in practice - resetting the display
    at the start of the test before a commit will reset the CRTC ID and FB
    ID for each plane.
    
    For an example, consider the
    igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max test.
    
    This test will fail for any overlay plane exposed for multiple CRTCs.
    
    The test tries to set a framebuffer for pipe A but has all the other
    pipes reset the plane state in the same commit. If there are multiple
    pipes on the hardware then the last pipe will be the one to set all the
    plane properties. The driver will receive a commit with no overlay
    plane enabled since the last pipe set CRTC ID and FB ID to 0, disabling
    the plane. The reference CRC capture will be incorrect since not all
    the planes have been enabled and the subsequent CRC captures will
    not match, failing the test.
    
    The simplest (but hacky) fix to this problem is to only set the
    properties for the plane for the pipe that last had it bound.
    
    This patch introduces a global plane list on igt_display_t that keeps
    track of the pipe that pipe that last called igt_plane_set_fb. The
    properties for the plane will only be applied from that single pipe
    when commiting the state to DRM.
    
    No behavioral changes should be introduced by this patch for hardware
    whose planes are only ever exposed one CRTC.
    
    It would likely be best to eventually modify the igt_pipe_t plane list
    to be a list of pointers to planes instead (igt_plane_t**)
    instead of being the actual plane objects, but that can come later.
    
    Many areas of the code like to make use of the backpointer to the pipe
    on the plane which makes refactoring the code in that manner a little
    trickier.
    
    v2: Add igt_plane_set_fb, use igt_plane_t for global plane list (Daniel)
    v3: Leave TODO for filling in all state/props on global planes
    
    Cc: Daniel Vetter <daniel@ffwll.ch>
    Signed-off-by: default avatarNicholas Kazlauskas <nicholas.kazlauskas@amd.com>
    Reviewed-by: default avatarHarry Wentland <harry.wentland@amd.com>
    ed944b45