Re: [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Oct 10, 2016 at 08:37:04AM -0400, Marc-André Lureau wrote:
> Imho, it should be fine for the client to send the same monitor
> config, the server should however not notify of changes if none
> happened.

Yup, after quite a lot of digging, this seems to be what is happening,
nothing in the spice-server->QEMU->kernel qxl->mutter is checking
whether the configuration we get is the same as the one which we had.
(the kernel has some checks for that, except that the way the resolution
changes is done in mutter disable these checks)

The way resolution changes happen in mutter are going to trigger a
surface destroy/create. Which gets us into this flicker loop:
- the client sends a MonitorsConfig message
- spice-server gets it, calls a QEMU callback
- QEMU raises an interrupt to inform the guest
- the kernel catches it and sends a udev event to userspace
- mutter catches this, and this triggers a resolution change
- the kernel does the resolution change, which involves a
  surface destroy/create
- the client receives the surface destroy/create, which will cause
  a MonitorsConfig message to be sent, so we are back to step 1

Checking whether we are getting an unchanged MonitorsConfig message
anywehere in qemu/kernel/mutter would solve (avoid ?) this bug, still
not fully sure where the best place is. Maybe best to do it once
host-side, and once guest-side.

Kernel-side, the check would go in qxl_display.c in qxl_display_read_client_monitors_config()
mutter-side, the check would belong in the call chain starting at on_uevent() in
backends/native/meta-monitor-manager-kms.c

In QEMU, the patch below fixes the flickering for me:

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 0e2682d..3530aeb 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1049,6 +1049,7 @@ static int interface_client_monitors_config(QXLInstance *sin,
         rect->right = monitor->x + monitor->width;
         rect->bottom = monitor->y + monitor->height;
     }
+    uint32_t old_crc = rom->client_monitors_config_crc;
     rom->client_monitors_config_crc = qxl_crc32(
             (const uint8_t *)&rom->client_monitors_config,
             sizeof(rom->client_monitors_config));
@@ -1059,7 +1060,9 @@ static int interface_client_monitors_config(QXLInstance *sin,
     trace_qxl_interrupt_client_monitors_config(qxl->id,
                         rom->client_monitors_config.count,
                         rom->client_monitors_config.heads);
-    qxl_send_events(qxl, QXL_INTERRUPT_CLIENT_MONITORS_CONFIG);
+    if (rom->client_monitors_config_crc != old_crc) {
+        qxl_send_events(qxl, QXL_INTERRUPT_CLIENT_MONITORS_CONFIG);
+    }
     return 1;
 }

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]