• Lyude Paul's avatar
    drm/display/dp_mst: Move all payload info into the atomic state · 4d07b0bc
    Lyude Paul authored
    Now that we've finally gotten rid of the non-atomic MST users leftover in
    the kernel, we can finally get rid of all of the legacy payload code we
    have and move as much as possible into the MST atomic state structs. The
    main purpose of this is to make the MST code a lot less confusing to work
    on, as there's a lot of duplicated logic that doesn't really need to be
    here. As well, this should make introducing features like fallback link
    retraining and DSC support far easier.
    
    Since the old payload code was pretty gnarly and there's a Lot of changes
    here, I expect this might be a bit difficult to review. So to make things
    as easy as possible for reviewers, I'll sum up how both the old and new
    code worked here (it took me a while to figure this out too!).
    
    The old MST code basically worked by maintaining two different payload
    tables - proposed_vcpis, and payloads. proposed_vcpis would hold the
    modified payload we wanted to push to the topology, while payloads held the
    payload table that was currently programmed in hardware. Modifications to
    proposed_vcpis would be handled through drm_dp_allocate_vcpi(),
    drm_dp_mst_deallocate_vcpi(), and drm_dp_mst_reset_vcpi_slots(). Then, they
    would be pushed via drm_dp_mst_update_payload_step1() and
    drm_dp_mst_update_payload_step2().
    
    Furthermore, it's important to note how adding and removing VC payloads
    actually worked with drm_dp_mst_update_payload_step1(). When a VC payload
    is removed from the VC table, all VC payloads which come after the removed
    VC payload's slots must have their time slots shifted towards the start of
    the table. The old code handles this by looping through the entire payload
    table and recomputing the start slot for every payload in the topology from
    scratch. While very much overkill, this ends up doing the right thing
    because we always order the VCPIs for payloads from first to last starting
    timeslot.
    
    It's important to also note that drm_dp_mst_update_payload_step2() isn't
    actually limited to updating a single payload - the driver can use it to
    queue up multiple payload changes so that as many of them can be sent as
    possible before waiting for the ACT. This is -technically- not against
    spec, but as Wayne Lin has pointed out it's not consistently implemented
    correctly in hubs - so it might as well be.
    
    drm_dp_mst_update_payload_step2() is pretty self explanatory and basically
    the same between the old and new code, save for the fact we don't have a
    second step for deleting payloads anymore -and thus rename it to
    drm_dp_mst_add_payload_step2().
    
    The new payload code stores all of the current payload info within the MST
    atomic state and computes as much of the state as possible ahead of time.
    This has the one exception of the starting timeslots for payloads, which
    can't be determined at atomic check time since the starting time slots will
    vary depending on what order CRTCs are enabled in the atomic state - which
    varies from driver to driver. These are still stored in the atomic MST
    state, but are only copied from the old MST state during atomic commit
    time. Likewise, this is when new start slots are determined.
    
    Adding/removing payloads now works much more closely to how things are
    described in the spec. When we delete a payload, we loop through the
    current list of payloads and update the start slots for any payloads whose
    time slots came after the payload we just deleted. Determining the starting
    time slots for new payloads being added is done by simply keeping track of
    where the end of the VC table is in
    drm_dp_mst_topology_mgr->next_start_slot. Additionally, it's worth noting
    that we no longer have a single update_payload() function. Instead, we now
    have drm_dp_mst_add_payload_step1|2() and drm_dp_mst_remove_payload(). As
    such, it's now left it up to the driver to figure out when to add or remove
    payloads. The driver already knows when it's disabling/enabling CRTCs, so
    it also already knows when payloads should be added or removed.
    
    Changes since v1:
    * Refactor around all of the completely dead code changes that are
      happening in amdgpu for some reason when they really shouldn't even be
      there in the first place… :\
    * Remove mention of sending one ACT per series of payload updates. As Wayne
      Lin pointed out, there are apparently hubs on the market that don't work
      correctly with this scheme and require a separate ACT per payload update.
    * Fix accidental drop of mst_mgr.lock - Wayne Lin
    * Remove mentions of allowing multiple ACT updates per payload change,
      mention that this is a result of vendors not consistently supporting this
      part of the spec and requiring a unique ACT for each payload change.
    * Get rid of reference to drm_dp_mst_port in DC - turns out I just got
      myself confused by DC and we don't actually need this.
    Changes since v2:
    * Get rid of fix for not sending payload deallocations if ddps=0 and just
      go back to wayne's fix
    Signed-off-by: default avatarLyude Paul <lyude@redhat.com>
    Cc: Wayne Lin <Wayne.Lin@amd.com>
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: Fangzhi Zuo <Jerry.Zuo@amd.com>
    Cc: Jani Nikula <jani.nikula@intel.com>
    Cc: Imre Deak <imre.deak@intel.com>
    Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
    Cc: Sean Paul <sean@poorly.run>
    Acked-by: default avatarJani Nikula <jani.nikula@intel.com>
    Link: https://patchwork.freedesktop.org/patch/msgid/20220817193847.557945-18-lyude@redhat.com
    4d07b0bc
intel_dp_mst.c 31.6 KB