> > When the QXL driver receives a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG > interrupt, > we currently always notify userspace that there was some hotplug event. > > However, gnome-shell/mutter is reacting to this event by attempting a > resolution change, which it does by issueing drmModeRmFB, drmModeAddFB, > and then drmModeSetCrtc. This has the side-effect of causing > qxl_crtc_mode_set() to tell the QXL virtual hardware that a primary > surface was destroyed and created. After going through QEMU and then the > remote SPICE client, a new identical monitors config message will be > sent, resulting in a QXL_INTERRUPT_CLIENT_MONITORS_CONFIG interrupt to > be emitted, and the same scenario occurring again. > > As destroying/creating the primary surface causes a visible screen > flicker, this makes the guest hard to use ( > https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ). > > This commit checks if the screen configuration we received is the same > one as the current one, and does not notify userspace about it if that's > the case. > > Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > --- > drivers/gpu/drm/qxl/qxl_display.c | 62 > ++++++++++++++++++++++++++++++++------- > 1 file changed, 52 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > index 8cf5177..518333c 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -57,11 +57,18 @@ static void qxl_alloc_client_monitors_config(struct > qxl_device *qdev, unsigned c > qdev->client_monitors_config->count = count; > } > > +enum { > + MONITORS_CONFIG_MODIFIED, > + MONITORS_CONFIG_UNCHANGED, > + MONITORS_CONFIG_BAD_CRC, > +}; > + > static int qxl_display_copy_rom_client_monitors_config(struct qxl_device > *qdev) > { > int i; > int num_monitors; > uint32_t crc; > + int status = MONITORS_CONFIG_UNCHANGED; > > num_monitors = qdev->rom->client_monitors_config.count; > crc = crc32(0, (const uint8_t *)&qdev->rom->client_monitors_config, > @@ -70,7 +77,7 @@ static int > qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev) > qxl_io_log(qdev, "crc mismatch: have %X (%zd) != %X\n", crc, > sizeof(qdev->rom->client_monitors_config), > qdev->rom->client_monitors_config_crc); > - return 1; > + return MONITORS_CONFIG_BAD_CRC; > } > if (num_monitors > qdev->monitors_config->max_allowed) { > DRM_DEBUG_KMS("client monitors list will be truncated: %d < %d\n", > @@ -79,6 +86,10 @@ static int > qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev) > } else { > num_monitors = qdev->rom->client_monitors_config.count; > } > + if (qdev->client_monitors_config > + && (num_monitors != qdev->client_monitors_config->count)) { > + status = MONITORS_CONFIG_MODIFIED; > + } > qxl_alloc_client_monitors_config(qdev, num_monitors); > /* we copy max from the client but it isn't used */ > qdev->client_monitors_config->max_allowed = > @@ -88,17 +99,39 @@ static int > qxl_display_copy_rom_client_monitors_config(struct qxl_device *qdev) > &qdev->rom->client_monitors_config.heads[i]; > struct qxl_head *client_head = > &qdev->client_monitors_config->heads[i]; > - client_head->x = c_rect->left; > - client_head->y = c_rect->top; > - client_head->width = c_rect->right - c_rect->left; > - client_head->height = c_rect->bottom - c_rect->top; > - client_head->surface_id = 0; > - client_head->id = i; > - client_head->flags = 0; > + if (client_head->x != c_rect->left) { > + client_head->x = c_rect->left; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->y != c_rect->top) { > + client_head->y = c_rect->top; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->width != c_rect->right - c_rect->left) { > + client_head->width = c_rect->right - c_rect->left; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->height != c_rect->bottom - c_rect->top) { > + client_head->height = c_rect->bottom - c_rect->top; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->surface_id != 0) { > + client_head->surface_id = 0; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->id != i) { > + client_head->id = i; > + status = MONITORS_CONFIG_MODIFIED; > + } > + if (client_head->flags != 0) { > + client_head->flags = 0; > + status = MONITORS_CONFIG_MODIFIED; > + } > DRM_DEBUG_KMS("read %dx%d+%d+%d\n", client_head->width, > client_head->height, > client_head->x, client_head->y); > } > - return 0; > + > + return status; > } > > static void qxl_update_offset_props(struct qxl_device *qdev) > @@ -124,9 +157,18 @@ void qxl_display_read_client_monitors_config(struct > qxl_device *qdev) > { > > struct drm_device *dev = qdev->ddev; > - while (qxl_display_copy_rom_client_monitors_config(qdev)) { > + int status; > + > + status = qxl_display_copy_rom_client_monitors_config(qdev); > + while (status == MONITORS_CONFIG_BAD_CRC) { > qxl_io_log(qdev, "failed crc check for client_monitors_config," > " retrying\n"); > + status = qxl_display_copy_rom_client_monitors_config(qdev); > + } > + if (status == MONITORS_CONFIG_UNCHANGED) { > + qxl_io_log(qdev, "config unchanged\n"); > + DRM_DEBUG("ignoring unchanged client monitors config"); Why log and debug? Looks like a missing debug cleanup. > + return; > } > > drm_modeset_lock_all(dev); Frediano _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
