[Spice-devel] [PATCH vdagent-linux] randr: do not set crtc mode to NULL when changing resolution
Hans de Goede
hdegoede at redhat.com
Thu Nov 22 08:17:23 PST 2012
Hi,
ACK, but please fix a small whitespace problem before pushing (see comment below)
On 11/22/2012 04:08 PM, Marc-André Lureau wrote:
> 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);
No tabs for indentation please, we use spaces exclusively. Also there seems to be
a copy and paste error in the comment.
> }
> +
> + /* 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;
> mode.name = modename;
> mode.nameLength = strlen(mode.name);
> set_reduced_cvt_mode(&mode, width, height);
> mode.modeFlags = 0;
> mode.id = 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:
>
Regards,
Hans
More information about the Spice-devel
mailing list