Note with this patch, the previous patch is unnecessary: the configuration can be sent to all agents (gdm & user) the creation/deletion races will be ignored. <div class="gmail_extra"> <div class="gmail_quote"> On Thu, Nov 22, 2012 at 4:08 PM, Marc-André Lureau <<a href="mailto:marcandre.lureau@gmail.com" target="_blank">marcandre.lureau@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The agent creates a unique mode vdagent-N per output, that is deleted when the custom resolution is changed. In order to be deleted, it can't be the current active crtc config. Now, the vdagent just sets the mode to NULL, but that causes a change of resolution probably due to gnome-settings-daemon (in RHEL6, it switches to max resultion 2560x1600 before switching back to custom resolution). We can avoid deleting current mode by creating a different mode, switching to it, and then deleting the previous mode. Also, we can ignore some Create/Delete races with other XRandR clients that may modify the mode simultaneously. This shouldn't be fatal, as long as the rest of the resolution switching can take place. --- src/vdagent-x11-randr.c | 53 +++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/vdagent-x11-randr.c b/src/vdagent-x11-randr.c index 2ffd264..2c1819a 100644 --- a/src/vdagent-x11-randr.c +++ b/src/vdagent-x11-randr.c @@ -47,13 +47,16 @@ static void arm_error_handler(struct vdagent_x11 *x11) old_error_handler = XSetErrorHandler(error_handler); } -static void check_error_handler(struct vdagent_x11 *x11) +static int check_error_handler(struct vdagent_x11 *x11) { + int error; + XSync(x11->display, False); XSetErrorHandler(old_error_handler); - if (caught_error) { - x11->set_crtc_config_not_functional = 1; - } + error = caught_error; + caught_error = 0; + + return error; } static XRRModeInfo *mode_from_id(struct vdagent_x11 *x11, int id) @@ -196,7 +199,7 @@ find_mode_by_size (struct vdagent_x11 *x11, int width, int height) return ret; } -static void delete_mode(struct vdagent_x11 *x11, int output_index) +static void delete_mode(struct vdagent_x11 *x11, int output_index, const char* name) { int m; XRRModeInfo *mode; @@ -204,7 +207,6 @@ static void delete_mode(struct vdagent_x11 *x11, int output_index) XRRCrtcInfo *crtc_info; RRCrtc crtc; int current_mode = -1; - char name[20]; output_info = x11->randr.outputs[output_index]; if (output_info->ncrtc != 1) { @@ -215,23 +217,22 @@ static void delete_mode(struct vdagent_x11 *x11, int output_index) crtc_info = crtc_from_id(x11, output_info->crtcs[0]); current_mode = crtc_info->mode; crtc = output_info->crtc; - snprintf(name, sizeof(name), "vdagent-%d", output_index); for (m = 0 ; m < x11->randr.res->nmode; ++m) { mode = &x11->randr.res->modes[m]; if (strcmp(mode->name, name) == 0) break; } if (m < x11->randr.res->nmode) { - if (crtc && mode->id == current_mode) { - syslog(LOG_DEBUG, - "delete_mode of in use mode, setting crtc to NULL mode"); - XRRSetCrtcConfig(x11->display, x11->randr.res, crtc, - CurrentTime, 0, 0, None, RR_Rotate_0, NULL, 0); - } + arm_error_handler(x11); XRRDeleteOutputMode (x11->display, x11->randr.res->outputs[output_index], mode->id); XRRDestroyMode (x11->display, mode->id); + // ignore race error, if mode is created by others + check_error_handler(x11); } + + /* silly to update everytime for more then one monitor */ + update_randr_res(x11); } static void set_reduced_cvt_mode(XRRModeInfo *mode, int width, int height) @@ -315,24 +316,21 @@ static XRRModeInfo *create_new_mode(struct vdagent_x11 *x11, int output_index, char modename[20]; XRRModeInfo mode; - /* we may be using this mode from an old invocation - check first */ - snprintf(modename, sizeof(modename), "vdagent-%d", output_index); - arm_error_handler(x11); - delete_mode(x11, output_index); - check_error_handler(x11); - if (x11->set_crtc_config_not_functional) { - return NULL; - } + snprintf(modename, sizeof(modename), "%dx%d-%d", width, height, output_index); mode.hSkew = 0; <a href="http://mode.name" target="_blank">mode.name</a> = modename; mode.nameLength = strlen(<a href="http://mode.name" target="_blank">mode.name</a>); set_reduced_cvt_mode(&mode, width, height); mode.modeFlags = 0; <a href="http://mode.id" target="_blank">mode.id</a> = 0; - + arm_error_handler(x11); XRRCreateMode (x11->display, x11->root_window, &mode); + // ignore race error, if mode is created by others + check_error_handler(x11); + /* silly to update everytime for more then one monitor */ update_randr_res(x11); + return find_mode_by_name(x11, modename); } @@ -343,6 +341,7 @@ static int xrandr_add_and_set(struct vdagent_x11 *x11, int output, int x, int y, int xid; Status s; RROutput outputs[1]; + char modename[20]; if (!x11->randr.res || output >= x11->randr.res->noutput || output < 0) { syslog(LOG_ERR, "%s: program error: missing RANDR or bad output", @@ -367,12 +366,17 @@ static int xrandr_add_and_set(struct vdagent_x11 *x11, int output, int x, int y, s = XRRSetCrtcConfig(x11->display, x11->randr.res, x11->randr.res->crtcs[output], CurrentTime, x, y, mode->id, RR_Rotate_0, outputs, 1); + if (s != RRSetConfigSuccess) { syslog(LOG_ERR, "failed to XRRSetCrtcConfig"); - delete_mode(x11, output); x11->set_crtc_config_not_functional = 1; return 0; } + + /* clean the previous name, if any */ + snprintf(modename, sizeof(modename), "%dx%d-%d", x11->width, x11->height, output); + delete_mode(x11, output, modename); + return 1; } @@ -695,7 +699,8 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11, XRRSetScreenSize(x11->display, x11->root_window, primary_w, primary_h, DisplayWidthMM(x11->display, x11->screen), DisplayHeightMM(x11->display, x11->screen)); - check_error_handler(x11); + + x11->set_crtc_config_not_functional = check_error_handler(x11); } update: -- 1.7.11.7 </blockquote></div> -- Marc-André Lureau </div>