[igt-dev] [PATCH] lib/chamelium: Support MST connectors on reprobe by using connector name

[Mark Yacoub]

On Tue, Jul 26, 2022 at 7:26 PM Lyude Paul <lyude at redhat.com> wrote:
>
> To make sure I'm understanding this code correctly: it seems like we're
> looking for a downstream MST port by just checking to see if there's an
> identically named port connected?
So, what's happening here is that the DUT and Chamelium are required
to be in static condition, meaning the same ports are always connected
at all times.
Before we run any chamelium test, we set up the igt config and
specifically match the connected ports as shown in this doc:
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/1b5f7f139660ac27f3c8b4c4a2d23fdd9430ccab/docs/chamelium.txt#L112
Through my testing, i saw that even when the connectors were unplugged
and disappeared, the connector name assigned in igtrc remained the
same, hence my assumption.
Given this situation, let me know if my assumption can still hold true.
>
> That's unfortunately not very reliable because connector names can change as
> well, although the names of connectors that are built into the system
> shouldn't. What we could -possibly- do is go through connector's path
> properties, and see if they match the previous connector props of the MST
> port.
>
> If this isn't good enough still, then we probably should add a better
> connector prop to the kernel then the path prop - preferably one that uses a
> format like this:
>
> ${CONN_NAME}-${MST_RAD}
>
> On Wed, 2022-07-20 at 11:19 -0400, Mark Yacoub wrote:
> > [Why]
> > On unplug, a lone MST connector can become invalid and disappear from
> > the list of DRM connectors.
> > When this happens, we can't check for the connector ID as it's no longer
> > there to check.
> > Hence, a connector disappearance indicates a disconnect.
> >
> > Furthermore, on a replug, the connector ID can change as well. The only
> > thing that remains unchageable is the connector name.
> >
> > [How]
> > Do not check for connector connetion state using the connector ID.
> > Iterate through the current connectors and match with the name.
> > If a connector disappeared, this means it's now disconnected.
> > If it's there, return its connection status.
> > Update the port connector ID with the current connector ID in case it
> > changed on a replug.
> >
> > Tested on Volteer with Chamelium V3 connected via a 2-DP-port CableMatters
> > hub.
> > Test: kms_chamelium --run-subtest dp-hpd
> >
> > Signed-off-by: Mark Yacoub <markyacoub at chromium.org>
> > ---
> >  lib/igt_chamelium.c | 52 +++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 45 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index d78ddd61..b5e9bbfd 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -266,21 +266,59 @@ chamelium_reprobe_connector(igt_display_t *display,
> >                             struct chamelium_port *port)
> >  {
> >         drmModeConnector *connector;
> > -       drmModeConnection status;
> > +       drmModeConnection connection_status;
> >         igt_output_t *output;
> > +       drmModeRes* res = drmModeGetResources(display->drm_fd);
> > +       bool is_connector_found = false;
> > +       int i;
> >
> >         igt_debug("Reprobing %s...\n", chamelium_port_get_name(port));
> > -       connector = chamelium_port_get_connector(chamelium, port, true);
> > +       for (i = 0; i < res->count_connectors; ++i) {
> > +               connector = drmModeGetConnector(display->drm_fd, res-
> > >connectors[i]);
> > +               /* If the connector is MST and is now unplugged, the spawned
> > connectors will no
> > +                  longer be available but can still be counted as part of
> > res->count_connectors */
> > +               if (!connector) {
> > +                       drmModeFreeConnector(connector);
> > +                       connector = NULL;
> > +                       continue;
> > +               }
> > +
> > +               char connector_name[50];
> > +               snprintf(connector_name, 50, "%s-%u",
> > +                       kmstest_connector_type_str(connector-
> > >connector_type),
> > +                       connector->connector_type_id);
> > +               if (strcmp(connector_name, port->name) == 0) {
> > +                       is_connector_found = true;
> > +                       break;
> > +               }
> > +
> > +               drmModeFreeConnector(connector);
> > +               connector = NULL;
> > +       }
> > +       drmModeFreeResources(res);
> > +
> > +       if (!is_connector_found) {
> > +               igt_debug("Connector is not found. This indicates it's
> > disconnected.\n");
> > +               igt_assert(!connector);
> > +               return DRM_MODE_DISCONNECTED;
> > +       }
> > +
> >         igt_assert(connector);
> > -       status = connector->connection;
> > +       connection_status = connector->connection;
> >
> > -       /* let's make sure that igt_display is up to date too */
> > +       /* If we still have a connector, let's make sure that igt_display
> > and the port are up to date too */
> >         output = igt_output_from_connector(display, connector);
> > -       output->force_reprobe = true;
> > -       igt_output_refresh(output);
> > +       if (output) {
> > +               output->force_reprobe = true;
> > +               igt_output_refresh(output);
> > +       }
> > +
> > +       /* If the topology of the connector is MST, the connector ID could
> > have changed. Update the
> > +          chamelium port to the current connector ID. */
> > +       port->connector_id = connector->connector_id;
> >
> >         drmModeFreeConnector(connector);
> > -       return status;
> > +       return connection_status;
> >  }
> >
> >  /**
>
> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
>