• Lyude Paul's avatar
    drm/dp_mst: Rewrite and fix bandwidth limit checks · 047d4cd2
    Lyude Paul authored
    Sigh, this is mostly my fault for not giving commit cd82d82c
    ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
    enough scrutiny during review. The way we're checking bandwidth
    limitations here is mostly wrong:
    
    For starters, drm_dp_mst_atomic_check_bw_limit() determines the
    pbn_limit of a branch by simply scanning each port on the current branch
    device, then uses the last non-zero full_pbn value that it finds. It
    then counts the sum of the PBN used on each branch device for that
    level, and compares against the full_pbn value it found before.
    
    This is wrong because ports can and will have different PBN limitations
    on many hubs, especially since a number of DisplayPort hubs out there
    will be clever and only use the smallest link rate required for each
    downstream sink - potentially giving every port a different full_pbn
    value depending on what link rate it's trained at. This means with our
    current code, which max PBN value we end up with is not well defined.
    
    Additionally, we also need to remember when checking bandwidth
    limitations that the top-most device in any MST topology is a branch
    device, not a port. This means that the first level of a topology
    doesn't technically have a full_pbn value that needs to be checked.
    Instead, we should assume that so long as our VCPI allocations fit we're
    within the bandwidth limitations of the primary MSTB.
    
    We do however, want to check full_pbn on every port including those of
    the primary MSTB. However, it's important to keep in mind that this
    value represents the minimum link rate /between a port's sink or mstb,
    and the mstb itself/. A quick diagram to explain:
    
                                    MSTB #1
                                   /       \
                                  /         \
                               Port #1    Port #2
           full_pbn for Port #1 → |          | ← full_pbn for Port #2
                               Sink #1    MSTB #2
                                             |
                                           etc...
    
    Note that in the above diagram, the combined PBN from all VCPI
    allocations on said hub should not exceed the full_pbn value of port #2,
    and the display configuration on sink #1 should not exceed the full_pbn
    value of port #1. However, port #1 and port #2 can otherwise consume as
    much bandwidth as they want so long as their VCPI allocations still fit.
    
    And finally - our current bandwidth checking code also makes the mistake
    of not checking whether something is an end device or not before trying
    to traverse down it.
    
    So, let's fix it by rewriting our bandwidth checking helpers. We split
    the function into one part for handling branches which simply adds up
    the total PBN on each branch and returns it, and one for checking each
    port to ensure we're not going over its PBN limit. Phew.
    
    This should fix regressions seen, where we erroneously reject display
    configurations due to thinking they're going over our bandwidth limits
    when they're not.
    
    Changes since v1:
    * Took an even closer look at how PBN limitations are supposed to be
      handled, and did some experimenting with Sean Paul. Ended up rewriting
      these helpers again, but this time they should actually be correct!
    Changes since v2:
    * Small indenting fix
    * Fix pbn_used check in drm_dp_mst_atomic_check_port_bw_limit()
    Signed-off-by: default avatarLyude Paul <lyude@redhat.com>
    Fixes: cd82d82c ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
    Cc: Sean Paul <seanpaul@google.com>
    Acked-by: default avatarAlex Deucher <alexander.deucher@amd.com>
    Reviewed-by: default avatarMikita Lipski <mikita.lipski@amd.com>
    Tested-by: default avatarHans de Goede <hdegoede@redhat.com>
    Link: https://patchwork.freedesktop.org/patch/msgid/20200309210131.1497545-1-lyude@redhat.com
    047d4cd2
drm_dp_mst_topology.c 151 KB