[PATCH 2/2] modesetting: add dynamic connector hotplug support (MST)

Daniel Martin consume.noise at gmail.com
Wed Jan 28 00:32:11 PST 2015


On 28 January 2015 at 08:40, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> This is ported from the same code in the ati and intel drivers,
>
> It uses the same option name as nvidia and the other DDXes to
> disable tearing down outputs as it is hard to avoid racing with clients.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---

I wrote a similar patch,
    http://lists.x.org/archives/xorg-devel/2014-December/044754.html
though not using an option.

Keith wrote that we can't merge it without testing, i.e. possible on
none-MST maschines with:
    http://lists.x.org/archives/xorg-devel/2014-December/044755.html

But, as nobody tested it I was lurking around for 1.17 to send a merge
request afterwards.

>  hw/xfree86/drivers/modesetting/driver.c          |   6 +
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 212 +++++++++++++++++++++--
>  hw/xfree86/drivers/modesetting/drmmode_display.h |   1 +
>  3 files changed, 206 insertions(+), 13 deletions(-)
>
> diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c
> index d52517d..aaf40c9 100644
> --- a/hw/xfree86/drivers/modesetting/driver.c
> +++ b/hw/xfree86/drivers/modesetting/driver.c
> @@ -123,6 +123,7 @@ typedef enum {
>      OPTION_DEVICE_PATH,
>      OPTION_SHADOW_FB,
>      OPTION_ACCEL_METHOD,
> +    OPTION_DELETE_DP_12_DISP,
>  } modesettingOpts;
>
>  static const OptionInfoRec Options[] = {
> @@ -130,6 +131,7 @@ static const OptionInfoRec Options[] = {
>      {OPTION_DEVICE_PATH, "kmsdev", OPTV_STRING, {0}, FALSE},
>      {OPTION_SHADOW_FB, "ShadowFB", OPTV_BOOLEAN, {0}, FALSE},
>      {OPTION_ACCEL_METHOD, "AccelMethod", OPTV_STRING, {0}, FALSE},
> +    {OPTION_DELETE_DP_12_DISP, "DeleteUnusedDP12Displays", OPTV_BOOLEAN, {0}, FALSE},
>      {-1, NULL, OPTV_NONE, {0}, FALSE}
>  };
>
> @@ -768,6 +770,10 @@ PreInit(ScrnInfoPtr pScrn, int flags)
>          ms->drmmode.sw_cursor = TRUE;
>      }
>
> +    if (xf86ReturnOptValBool(ms->Options, OPTION_DELETE_DP_12_DISP, FALSE)) {
> +        ms->drmmode.delete_dp_12_displays = TRUE;
> +    }
> +
>      ms->cursor_width = 64;
>      ms->cursor_height = 64;
>      ret = drmGetCap(ms->fd, DRM_CAP_CURSOR_WIDTH, &value);
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 8dc6b32..ee2fc2f 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -326,6 +326,8 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>                  continue;
>
>              drmmode_output = output->driver_private;
> +            if (drmmode_output->output_id == -1)
> +                continue;
>              output_ids[output_count] =
>                  drmmode_output->mode_output->connector_id;
>              output_count++;
> @@ -366,10 +368,14 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode,
>          /* go through all the outputs and force DPMS them back on? */
>          for (i = 0; i < xf86_config->num_output; i++) {
>              xf86OutputPtr output = xf86_config->output[i];
> +            drmmode_output_private_ptr drmmode_output;
>
>              if (output->crtc != crtc)
>                  continue;
>
> +            drmmode_output = output->driver_private;
> +            if (drmmode_output->output_id == -1)
> +                continue;
>              output->funcs->dpms(output, DPMSModeOn);
>          }
>      }
> @@ -712,6 +718,9 @@ drmmode_output_detect(xf86OutputPtr output)
>      drmmode_ptr drmmode = drmmode_output->drmmode;
>      xf86OutputStatus status;
>
> +    if (drmmode_output->output_id == -1)
> +        return XF86OutputStatusDisconnected;
> +
>      drmModeFreeConnector(drmmode_output->mode_output);
>
>      drmmode_output->mode_output =
> @@ -1111,22 +1120,134 @@ static const char *const output_names[] = {
>      "DSI",
>  };
>
> +static xf86OutputPtr find_output(ScrnInfoPtr pScrn, int id)
> +{
> +    xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
> +    int i;
> +    for (i = 0; i < xf86_config->num_output; i++) {
> +        xf86OutputPtr output = xf86_config->output[i];
> +        drmmode_output_private_ptr drmmode_output;
> +
> +        drmmode_output = output->driver_private;
> +        if (drmmode_output->output_id == id)
> +            return output;
> +    }
> +    return NULL;
> +}
> +
> +static int parse_path_blob(drmModePropertyBlobPtr path_blob, int *conn_base_id, char **path)
> +{
> +    char *conn;
> +    char conn_id[5];
> +    int id, len;
> +    char *blob_data;
> +
> +    if (!path_blob)
> +        return -1;
> +
> +    blob_data = path_blob->data;
> +    /* we only handle MST paths for now */
> +    if (strncmp(blob_data, "mst:", 4))
> +        return -1;
> +
> +    conn = strchr(blob_data + 4, '-');
> +    if (!conn)
> +        return -1;
> +    len = conn - (blob_data + 4);
> +    if (len + 1> 5)
> +        return -1;
> +    memcpy(conn_id, blob_data + 4, len);
> +    conn_id[len + 1] = '\0';
> +    id = strtoul(conn_id, NULL, 10);
> +
> +    *conn_base_id = id;
> +
> +    *path = conn + 1;
> +    return 0;
> +}
> +
>  static void
> -drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res, int num)
> +drmmode_create_name(ScrnInfoPtr pScrn, drmModeConnectorPtr koutput, char *name,
> +                   drmModePropertyBlobPtr path_blob)
>  {
> +    int ret;
> +    char *extra_path;
> +    int conn_id;
>      xf86OutputPtr output;
> +
> +    ret = parse_path_blob(path_blob, &conn_id, &extra_path);
> +    if (ret == -1)
> +        goto fallback;
> +
> +    output = find_output(pScrn, conn_id);
> +    if (!output)
> +        goto fallback;
> +
> +    snprintf(name, 32, "%s-%s", output->name, extra_path);
> +    ErrorF("setting name to %s\n", name);
> +    return;
> +
> + fallback:
> +    if (koutput->connector_type >= MS_ARRAY_SIZE(output_names))
> +        snprintf(name, 32, "Unknown-%d", koutput->connector_type_id - 1);
> +#ifdef MODESETTING_OUTPUT_SLAVE_SUPPORT
> +    else if (pScrn->is_gpu)
> +        snprintf(name, 32, "%s-%d-%d", output_names[koutput->connector_type], pScrn->scrnIndex - GPU_SCREEN_OFFSET + 1, koutput->connector_type_id - 1);
> +#endif
> +    else
> +        snprintf(name, 32, "%s-%d", output_names[koutput->connector_type], koutput->connector_type_id - 1);
> +}
> +static void
> +drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_res, int num, Bool dynamic)
> +{
> +    xf86OutputPtr output;
> +    xf86CrtcConfigPtr   xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
>      drmModeConnectorPtr koutput;
>      drmModeEncoderPtr *kencoders = NULL;
>      drmmode_output_private_ptr drmmode_output;
>      drmModePropertyPtr props;
>      char name[32];
>      int i;
> +    drmModePropertyBlobPtr path_blob = NULL;
>
>      koutput =
>          drmModeGetConnector(drmmode->fd, mode_res->connectors[num]);
>      if (!koutput)
>          return;
>
> +    for (i = 0; i < koutput->count_props; i++) {
> +        props = drmModeGetProperty(drmmode->fd, koutput->props[i]);
> +        if (props && (props->flags & DRM_MODE_PROP_BLOB)) {
> +            if (!strcmp(props->name, "PATH")) {
> +                path_blob = drmModeGetPropertyBlob(drmmode->fd, koutput->prop_values[i]);
> +                drmModeFreeProperty(props);
> +                break;
> +            }
> +            drmModeFreeProperty(props);
> +        }
> +    }
> +
> +    drmmode_create_name(pScrn, koutput, name, path_blob);
> +
> +    if (path_blob)
> +        drmModeFreePropertyBlob(path_blob);
> +
> +    if (path_blob && dynamic) {
> +        /* see if we have an output with this name already
> +           and hook stuff up */
> +        for (i = 0; i < xf86_config->num_output; i++) {
> +            output = xf86_config->output[i];
> +
> +            if (strncmp(output->name, name, 32))
> +                continue;
> +
> +            drmmode_output = output->driver_private;
> +            drmmode_output->output_id = mode_res->connectors[num];
> +            drmmode_output->mode_output = koutput;
> +            return;
> +        }
> +    }
> +
>      kencoders = calloc(sizeof(drmModeEncoderPtr), koutput->count_encoders);
>      if (!kencoders) {
>          goto out_free_encoders;
> @@ -1139,17 +1260,6 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r
>          }
>      }
>
> -    /* need to do smart conversion here for compat with non-kms ATI driver */
> -    if (koutput->connector_type >= MS_ARRAY_SIZE(output_names))
> -        snprintf(name, 32, "Unknown-%d", koutput->connector_type_id - 1);
> -    else if (pScrn->is_gpu)
> -        snprintf(name, 32, "%s-%d-%d", output_names[koutput->connector_type],
> -                 pScrn->scrnIndex - GPU_SCREEN_OFFSET + 1,
> -                 koutput->connector_type_id - 1);
> -    else
> -        snprintf(name, 32, "%s-%d", output_names[koutput->connector_type],
> -                 koutput->connector_type_id - 1);
> -
>      output = xf86OutputCreate(pScrn, &drmmode_output_funcs, name);
>      if (!output) {
>          goto out_free_encoders;
> @@ -1192,6 +1302,8 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r
>          }
>      }
>
> +    if (dynamic)
> +        output->randr_output = RROutputCreate(xf86ScrnToScreen(pScrn), output->name, strlen(output->name), output);
>      return;
>   out_free_encoders:
>      if (kencoders) {
> @@ -1451,7 +1563,7 @@ drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
>              drmmode_crtc_init(pScrn, drmmode, mode_res, i);
>
>      for (i = 0; i < mode_res->count_connectors; i++)
> -        drmmode_output_init(pScrn, drmmode, mode_res, i);
> +        drmmode_output_init(pScrn, drmmode, mode_res, i, FALSE);
>
>      /* workout clones */
>      drmmode_clones_init(pScrn, drmmode, mode_res);
> @@ -1620,11 +1732,85 @@ drmmode_handle_uevents(int fd, void *closure)
>      drmmode_ptr drmmode = closure;
>      ScrnInfoPtr scrn = drmmode->scrn;
>      struct udev_device *dev;
> +    drmModeResPtr mode_res;
> +    xf86CrtcConfigPtr  config = XF86_CRTC_CONFIG_PTR(scrn);
> +    int i, j;
> +    Bool found;
> +    Bool changed = FALSE;
>
>      dev = udev_monitor_receive_device(drmmode->uevent_monitor);
>      if (!dev)
>          return;
>
> +    mode_res = drmModeGetResources(drmmode->fd);
> +    if (!mode_res)
> +        goto out;
> +
> +    if (mode_res->count_crtcs != config->num_crtc) {
> +        ErrorF("number of CRTCs changed - failed to handle, %d vs %d\n", mode_res->count_crtcs, config->num_crtc);
> +        goto out_free_res;
> +    }
> +
> +    /* figure out if we have gotten rid of any connectors
> +       traverse old output list looking for outputs */
> +restart_destroy:
> +    for (i = 0; i < config->num_output; i++) {
> +        xf86OutputPtr output = config->output[i];
> +        drmmode_output_private_ptr drmmode_output;
> +
> +        drmmode_output = output->driver_private;
> +        found = FALSE;
> +        for (j = 0; j < mode_res->count_connectors; j++) {
> +            if (mode_res->connectors[j] == drmmode_output->output_id) {
> +                found = TRUE;
> +                break;
> +            }
> +        }
> +        if (found)
> +            continue;
> +
> +        drmModeFreeConnector(drmmode_output->mode_output);
> +        drmmode_output->mode_output = NULL;
> +        drmmode_output->output_id = -1;
> +
> +        changed = TRUE;
> +        if (drmmode->delete_dp_12_displays) {
> +             RROutputDestroy(output->randr_output);

In my patch I had to remove the output from crtcs using it. Otherwise
the server died in a horrible fire.
You don't need this? Looks like I have to study your patch and compare
what I did wrong.

> +            xf86OutputDestroy(output);
> +            goto restart_destroy;
> +        }
> +    }
> +
> +    /* find new output ids we don't have outputs for */
> +    for (i = 0; i < mode_res->count_connectors; i++) {
> +        found = FALSE;
> +
> +        for (j = 0; j < config->num_output; j++) {
> +            xf86OutputPtr output = config->output[j];
> +            drmmode_output_private_ptr drmmode_output;
> +
> +            drmmode_output = output->driver_private;
> +            if (mode_res->connectors[i] == drmmode_output->output_id) {
> +                found = TRUE;
> +                break;
> +            }
> +        }
> +        if (found)
> +            continue;
> +
> +        changed = TRUE;
> +        drmmode_output_init(scrn, drmmode, mode_res, i, 1);
> +
> +    }
> +
> +    if (changed) {
> +        RRSetChanged(xf86ScrnToScreen(scrn));
> +        RRTellChanged(xf86ScrnToScreen(scrn));
> +    }
> +
> +out_free_res:
> +    drmModeFreeResources(mode_res);
> +out:
>      RRGetInfo(xf86ScrnToScreen(scrn), TRUE);
>      udev_device_unref(dev);
>  }
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
> index f4989b8..769842d 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.h
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
> @@ -79,6 +79,7 @@ typedef struct {
>      uint32_t triple_buffer_name;
>
>      DevPrivateKeyRec pixmapPrivateKeyRec;
> +    Bool delete_dp_12_displays;
>  } drmmode_rec, *drmmode_ptr;
>
>  typedef struct {
> --
> 2.1.0
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel


More information about the xorg-devel mailing list