[igt-dev] [PATCH i-g-t v2 9/9] lib/igt_chamelium: autodiscover Chamelium port mappings
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Mon Jun 24 13:22:57 UTC 2019
On Wed, Jun 19, 2019 at 06:55:18PM +0300, Simon Ser wrote:
> This removes the need for configuring Chamelium ports in .igtrc, making it both
> easier and less error-prone to setup Chamelium boards.
>
> A new function chamelium_autodiscover sets a different EDID on each Chamelium
> port and plugs them. We then walk the list of DRM connectors, read back the
> EDID and infer the mapping.
>
> The EDID serial number is used to tell each port apart. In case a mapping is
> already configured in the .igtrc file, we check that it's consistent.
>
> MST is not handled yet (requires refactoring existing tests since connector IDs
> aren't stable).
>
> Signed-off-by: Simon Ser <simon.ser at intel.com>
> ---
> lib/igt_chamelium.c | 205 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index dcc59e9f0b04..ddea1ba47d4e 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -358,7 +358,7 @@ static xmlrpc_value *chamelium_rpc(struct chamelium *chamelium,
> */
> void chamelium_plug(struct chamelium *chamelium, struct chamelium_port *port)
> {
> - igt_debug("Plugging %s\n", port->name);
> + igt_debug("Plugging %s (Chamelium port ID %d)\n", port->name, port->id);
> xmlrpc_DECREF(chamelium_rpc(chamelium, NULL, "Plug", "(i)", port->id));
> }
>
> @@ -1828,6 +1828,204 @@ out:
> return ret;
> }
>
> +/**
> + * autodiscover_ports: list of ports which can be auto-discovered
> + *
> + * See this file for a list of Chamelium ports:
> + * https://chromium.googlesource.com/chromiumos/platform/chameleon/+/master/chameleond/utils/ids.py
> + */
> +static const int autodiscover_ports[] = {1, 2, 3, 4};
> +
> +#define AUTODISCOVER_PORTS_LEN (sizeof(autodiscover_ports) / sizeof(autodiscover_ports[0]))
We also have CHAMELIUM_MAX_PORTS which counts the number of display
ports. Are we anticipating any non-autodiscoverable ports? Any other
reasons to keep this separate?
> +static int port_id_from_edid(int drm_fd, drmModeConnector *connector)
> +{
> + int port_id = -1;
> + bool ok;
> + uint64_t edid_blob_id;
> + drmModePropertyBlobRes *edid_blob;
> + const struct edid *edid;
> + char mfg[3];
> +
> + if (connector->connection != DRM_MODE_CONNECTED) {
> + igt_debug("Skipping auto-discovery for connector %s-%d: "
> + "connector status is not connected\n",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id);
> + return -1;
> + }
> +
> + ok = kmstest_get_property(drm_fd, connector->connector_id,
> + DRM_MODE_OBJECT_CONNECTOR, "EDID",
> + NULL, &edid_blob_id, NULL);
> + if (!ok) {
> + igt_debug("Skipping auto-discovery for connector %s-%d: "
> + "missing the EDID property\n",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id);
> + return -1;
> + }
> +
> + edid_blob = drmModeGetPropertyBlob(drm_fd, edid_blob_id);
> + igt_assert(edid_blob);
> +
> + edid = (const struct edid *) edid_blob->data;
> +
> + edid_get_mfg(edid, mfg);
> + if (memcmp(mfg, "IGT", 3) != 0) {
> + igt_debug("Skipping connector %s-%d for auto-discovery: "
> + "manufacturer is %.3s, not IGT\n",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id, mfg);
> + goto out;
> + }
> +
> + if (edid->prod_code[0] != 'C' || edid->prod_code[1] != 'H') {
> + igt_warn("Invalid EDID for IGT connector %s-%d: "
> + "invalid product code\n",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id);
> + goto out;
> + }
> +
> + port_id = *(uint32_t *) &edid->serial;
> + igt_debug("Auto-discovery mapped connector %s-%d to Chamelium "
> + "port ID %d\n",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id, port_id);
> +
> +out:
> + drmModeFreePropertyBlob(edid_blob);
> + return port_id;
> +}
> +
> +/**
> + * chamelium_autodiscover: TODO
> + */
Are you planning to get this description done?
> +static bool chamelium_autodiscover(struct chamelium *chamelium, int drm_fd)
> +{
> + drmModeRes *res;
> + drmModeConnector *connector;
> + struct chamelium_port *port;
> + size_t i, j, port_index;
> + int port_id;
> + uint32_t conn_id;
> + struct chamelium_edid *edid;
> + bool found;
> + uint32_t autodiscovered[AUTODISCOVER_PORTS_LEN] = {0};
> + char conn_name[64];
> +
> + igt_debug("Starting Chamelium port auto-discovery\n");
> +
> + edid = chamelium_new_edid(chamelium, igt_kms_get_base_edid());
> +
> + /* Set EDID and plug ports we want to auto-discover */
> + port_index = chamelium->port_count;
> + for (i = 0; i < AUTODISCOVER_PORTS_LEN; i++) {
> + port_id = autodiscover_ports[i];
> +
> + /* No need to auto-discover ports explicitly configured in the
> + * .igtrc file. */
> + found = false;
> + for (j = 0; j < chamelium->port_count; j++) {
> + if (chamelium->ports[j].id == port_id) {
> + found = true;
> + break;
> + }
> + }
> + if (found)
> + continue;
we are skipping seting edid and pluging the port if it was defined in .igtrc
> +
> + igt_assert(port_index < CHAMELIUM_MAX_PORTS);
> + port = &chamelium->ports[port_index];
> + port_index++;
> +
> + port->id = port_id;
> +
> + chamelium_port_set_edid(chamelium, port, edid);
> + chamelium_plug(chamelium, port);
> + }
> +
> + /* Reprobe connectors and build the mapping */
> + res = drmModeGetResources(drm_fd);
> + if (!res)
> + return false;
> +
> + for (i = 0; i < res->count_connectors; i++) {
> + conn_id = res->connectors[i];
> +
> + /* Read the EDID and parse the Chamelium port ID we stored
> + * there. */
> + connector = drmModeGetConnector(drm_fd, res->connectors[i]);
> + port_id = port_id_from_edid(drm_fd, connector);
> + drmModeFreeConnector(connector);
> + if (port_id < 0)
> + continue;
here we won't get a sensible value and continue;
> +
> + /* If we already have a mapping from the config file, check
> + * that it's consistent. */
> + found = false;
> + for (j = 0; j < chamelium->port_count; j++) {
> + port = &chamelium->ports[j];
> + if (port->connector_id == conn_id) {
> + found = true;
> + igt_assert_f(port->id == port_id,
> + "Inconsistency detected in .igtrc: "
> + "connector %s is configured with "
> + "Chamelium port %d, but is "
> + "connected to port %d\n",
> + port->name, port->id, port_id);
> + break;
so this whole section checking consistency seems to make no sense
Or am I reading this wrong?
> + }
> + }
> + if (found)
> + continue;
> +
> + /* Get got a new mapping */
Get got?
> + found = false;
> + for (j = 0; j < AUTODISCOVER_PORTS_LEN; j++) {
> + if (port_id == autodiscover_ports[j]) {
> + found = true;
> + autodiscovered[j] = conn_id;
> + break;
> + }
> + }
> + igt_assert_f(found, "Auto-discovered a port (%d) we haven't "
> + "setup\n", port_id);
> + }
> +
> + drmModeFreeResources(res);
> +
> + /* We now have a Chamelium port ID ↔ DRM connector ID mapping:
> + * autodiscover_ports contains the Chamelium port IDs and
> + * autodiscovered contains the DRM connector IDs. */
> + for (i = 0; i < AUTODISCOVER_PORTS_LEN; i++) {
> + port_id = autodiscover_ports[i];
> + conn_id = autodiscovered[i];
> + if (!conn_id) {
> + continue;
> + }
> +
> + port = &chamelium->ports[chamelium->port_count];
> + chamelium->port_count++;
> +
> + port->id = port_id;
> + port->type = chamelium_get_port_type(chamelium, port);
> + port->connector_id = conn_id;
> +
> + connector = drmModeGetConnectorCurrent(drm_fd, conn_id);
> + snprintf(conn_name, sizeof(conn_name), "%s-%u",
> + kmstest_connector_type_str(connector->connector_type),
> + connector->connector_type_id);
> + drmModeFreeConnector(connector);
> + port->name = strdup(conn_name);
> + }
> +
> + chamelium_reset(chamelium);
> +
> + return true;
> +}
> +
> static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
> {
> GError *error = NULL;
> @@ -1845,7 +2043,10 @@ static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
> return false;
> }
>
> - return chamelium_read_port_mappings(chamelium, drm_fd);
> + if (!chamelium_read_port_mappings(chamelium, drm_fd)) {
> + return false;
> + }
> + return chamelium_autodiscover(chamelium, drm_fd);
> }
>
> /**
> --
> 2.22.0
>
More information about the igt-dev
mailing list