On Sun, Aug 04, 2024 at 06:45:43AM -0700, Manasi Navare wrote: > On Mon, Jul 22, 2024 at 9:55 AM Imre Deak <imre.d...@intel.com> wrote: > > > > In the > > if (old_ddps != port->ddps || !created) > > if (port->ddps && !port->input) > > ret = drm_dp_send_enum_path_resources(); > > > > sequence the first if's condition is true if the port exists already > > (!created) or the port was created anew (hence old_ddps==0) and it was > > in the plugged state (port->ddps==1). The second if's condition is true > > for output ports in the plugged state. So the function is called for an > > output port in the plugged state, regardless if it already existed or > > not and regardless of the old plugged state. In all other cases > > port->full_pbn can be zeroed as the port is either an input for which > > full_pbn is never set, or an output in the unplugged state for which > > full_pbn was already zeroed previously or the port was just created > > (with port->full_pbn==0). > > > > Simplify the condition, making it clear that the path resources are > > always enumerated for an output port in the plugged state. > > Would this take care of the cases where a branch device is present > between source and the sink and its properly allocating the resources > and advertising UHBR capability from branch to sink. This was a bug > earlier with UHBR on branch device/ MST hub
I suppose you refer to [1]. The patchset as a whole should ensure that the BW reported by branch devices via the ENUM_PATH_RESOURCES message is correct even across changing between UHBR <-> non-UHBR link rates between the source and the first downstream branch devices. More on this are detailed at [2] and [3]. It looks like this would also address the issue described in [1], but I couldn't test this yet, as I don't have any hub/dock supporting UHBR rates. --Imre [1] https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10970 [2] https://patchwork.freedesktop.org/patch/605424/?series=136347&rev=2 [3] https://patchwork.freedesktop.org/patch/605419/?series=136347&rev=2 > > Manasi > > > > > Cc: Lyude Paul <ly...@redhat.com> > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Imre Deak <imre.d...@intel.com> > > --- > > drivers/gpu/drm/display/drm_dp_mst_topology.c | 19 ++++++++----------- > > 1 file changed, 8 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > index 70e4bfc3532e0..bcc5bbed9bd04 100644 > > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > > @@ -2339,7 +2339,7 @@ drm_dp_mst_handle_link_address_port(struct > > drm_dp_mst_branch *mstb, > > { > > struct drm_dp_mst_topology_mgr *mgr = mstb->mgr; > > struct drm_dp_mst_port *port; > > - int old_ddps = 0, ret; > > + int ret; > > u8 new_pdt = DP_PEER_DEVICE_NONE; > > bool new_mcs = 0; > > bool created = false, send_link_addr = false, changed = false; > > @@ -2372,7 +2372,6 @@ drm_dp_mst_handle_link_address_port(struct > > drm_dp_mst_branch *mstb, > > */ > > drm_modeset_lock(&mgr->base.lock, NULL); > > > > - old_ddps = port->ddps; > > changed = port->ddps != port_msg->ddps || > > (port->ddps && > > (port->ldps != port_msg->legacy_device_plug_status > > || > > @@ -2407,15 +2406,13 @@ drm_dp_mst_handle_link_address_port(struct > > drm_dp_mst_branch *mstb, > > * Reprobe PBN caps on both hotplug, and when re-probing the link > > * for our parent mstb > > */ > > - if (old_ddps != port->ddps || !created) { > > - if (port->ddps && !port->input) { > > - ret = drm_dp_send_enum_path_resources(mgr, mstb, > > - port); > > - if (ret == 1) > > - changed = true; > > - } else { > > - port->full_pbn = 0; > > - } > > + if (port->ddps && !port->input) { > > + ret = drm_dp_send_enum_path_resources(mgr, mstb, > > + port); > > + if (ret == 1) > > + changed = true; > > + } else { > > + port->full_pbn = 0; > > } > > > > ret = drm_dp_port_set_pdt(port, new_pdt, new_mcs); > > -- > > 2.44.2 > >
