[PATCH 03/14] drm/dp_mst: Simplify the condition when to enumerate path resources

[Imre Deak]

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.deak at 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 <lyude at redhat.com>
> > Cc: dri-devel at lists.freedesktop.org
> > Signed-off-by: Imre Deak <imre.deak at 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
> >