Created attachment 258654 [details] [review]
RFC xrandr: use "hotplug_mode_update" property

See also comment in https://bugzilla.gnome.org/show_bug.cgi?id=702804#c9

btw, the kernel patch for the property is now in drm-next:
 http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-next

Review of attachment 258654 [details] [review]:

::: src/core/monitor-xrandr.c
@@ +321,3 @@
+  gboolean result = FALSE;
+
+  atom = XInternAtom (manager_xrandr->xdisplay, "hotplug_mode_update", FALSE);

Looks worth caching - can you add it to meta/atomnames.h?

@@ +328,3 @@
+
+  if (info) {
+      result = TRUE;

It's unlikely that drivers start to set the property to anything but TRUE, but something like
  result = info->values[0] == 1
or so still looks more correct to me.

@@ +1039,3 @@
   if (manager_xrandr->resources->timestamp >= manager_xrandr->resources->configTimestamp ||
+      (!hotplug_mode_update &&
+       meta_monitor_config_match_current (manager->config, manager)))

The comment above needs update.

Maybe it is clearer to write this as:

if (meta_monitor_manager_has_hotplug_mode_update (manager))
  {
    meta_monitor_config_make_default (manager->config, manager);
  }
else if (manager_xrandr->resources->timestamp >= ...)

?

Review of attachment 258654 [details] [review]:

::: src/core/monitor-xrandr.c
@@ +321,3 @@
+  gboolean result = FALSE;
+
+  atom = XInternAtom (manager_xrandr->xdisplay, "hotplug_mode_update", FALSE);

done

@@ +328,3 @@
+
+  if (info) {
+      result = TRUE;

it's set to 0 in the patch, we could eventually ask Dave to change that quickly, but in general the idea is to use this property as a tag, its value shouldn't be meaningful.

http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=4695b03970df378dcb93fe3e7158381f1e980fa2

@@ +1039,3 @@
   if (manager_xrandr->resources->timestamp >= manager_xrandr->resources->configTimestamp ||
+      (!hotplug_mode_update &&
+       meta_monitor_config_match_current (manager->config, manager)))

well, we also want to allow re-configuration with manual xrandr usage or other tools, so the config timestamp check is still necessary. I updated the patch to make the 2 cases easier to read.

Created attachment 259742 [details] [review]
RFC xrandr: use "hotplug_mode_update" property

Use the "hotplug_mode_update" connector property indicating that the
screen settings should be updated: get a new preferred mode on hotplug
events to handle dynamic guest resizing (where you resize the host
window and the guest resizes with it).

Review of attachment 259742 [details] [review]:

OK.