[PATCH 2/2] modesetting: add dynamic connector hotplug support (MST)
Daniel Martin
consume.noise at gmail.com
Wed Jan 28 01:42:21 PST 2015
On 28 January 2015 at 09:32, Daniel Martin <consume.noise at gmail.com> wrote:
> 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.
Just tested your patches on top of current master:
df1b401 randr: attempt to fix primary on slave output (v2)
Given a
- Lenovo T440p + docking station (Pro Dock 40A1)
- set the option DeleteUnusedDP12Displays to true
- start the server
- plug in a monitor at the VGA port of the dock
-> the output DisplayPort-1-3 gets added.
Now, if I
- unplug the monitor (even without enabling it):
(EE) Backtrace:
(EE) 0: /opt/xorg-devel/bin/Xorg (OsSigHandler+0x3c) [0x6185d8]
(EE) 1: /usr/lib/libc.so.6 (__restore_rt+0x0) [0x7f9db51649ef]
(EE) 2: /opt/xorg-devel/lib/xorg/modules/drivers/modesetting_drv.so
(drmmode_output_destroy+0xd4) [0x7f9db01b9b49]
(EE) 3: /opt/xorg-devel/bin/Xorg (xf86OutputDestroy+0x51) [0x4d71b2]
(EE) 4: /opt/xorg-devel/lib/xorg/modules/drivers/modesetting_drv.so
(drmmode_handle_uevents+0x1a5) [0x7f9db01bc0a9]
(EE) 5: /opt/xorg-devel/bin/Xorg (xf86Wakeup+0x290) [0x48e83d]
(EE) 6: /opt/xorg-devel/bin/Xorg (WakeupHandler+0x83) [0x442289]
(EE) 7: /opt/xorg-devel/bin/Xorg (WaitForSomething+0x3fb) [0x60eb0e]
(EE) 8: /opt/xorg-devel/bin/Xorg (Dispatch+0x97) [0x433264]
(EE) 9: /opt/xorg-devel/bin/Xorg (dix_main+0x627) [0x441569]
(EE) 10: /opt/xorg-devel/bin/Xorg (main+0x28) [0x423efe]
(EE) 11: /usr/lib/libc.so.6 (__libc_start_main+0xf0) [0x7f9db5151040]
(EE) 12: /opt/xorg-devel/bin/Xorg (_start+0x29) [0x423e09]
(EE) 13: ? (?+0x29) [0x29]
(EE)
(EE) Segmentation fault at address 0x48
drmmode_output_destroy+0xd4 that is
drmmode_display.c:885
the for-loop over the encoders. In my patch I added an
if (drmmode_output->mode_output)
before the for-loop.
When adding this check to your patch, the server doesn't segfault
anymore when hotplugging a disabled monitor at the VGA port.
But, now let's:
- enable it: xrandr --output DisplayPort-1-3 --preferred
- unplug it
-> output DisplayPort-1-3 disappears
- re-plug it
-> output DisplayPort-1-3 is back
- try to enable it: xrandr --output DisplayPort-1-3 --preferred
-> "xrandr: cannot find output 0x0"
I would like to dig more into this, but time is thee enemy.
>> + 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