Vivek Kasireddy <vivek.kasire...@intel.com> writes: > The new parameter named "connector" can be used to assign physical > monitors/connectors to individual GFX VCs such that when the monitor > is connected or hotplugged, the associated GTK window would be > fullscreened on it. If the monitor is disconnected or unplugged, > the associated GTK window would be destroyed and a relevant > disconnect event would be sent to the Guest. > > Usage: -device virtio-gpu-pci,max_outputs=2,blob=true,xres=1920,yres=1080... > -display gtk,gl=on,connector.0=eDP-1,connector.1=DP-1..... > > Cc: Dongwon Kim <dongwon....@intel.com> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Daniel P. Berrangé <berra...@redhat.com> > Cc: Markus Armbruster <arm...@redhat.com> > Cc: Philippe Mathieu-Daudé <f4...@amsat.org> > Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > Cc: Thomas Huth <th...@redhat.com> > Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com> > --- > qapi/ui.json | 9 ++- > qemu-options.hx | 1 + > ui/gtk.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 177 insertions(+), 1 deletion(-) > > diff --git a/qapi/ui.json b/qapi/ui.json > index 286c5731d1..86787a4c95 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1199,13 +1199,20 @@ > # interfaces (e.g. VGA and virtual console character devices) > # by default. > # Since 7.1 > +# @connector: List of physical monitor/connector names where the GTK > windows > +# containing the respective graphics virtual consoles (VCs) are > +# to be placed. If a mapping exists for a VC, it will be > +# fullscreened on that specific monitor or else it would not be > +# displayed anywhere and would appear disconnected to the > guest.
Let's see whether I understand this... We have VCs numbered 0, 1, ... VC #i is mapped to the i-th element of @connector, counting from zero. Correct? Ignorant question: what's a "physical monitor/connector name"? Would you mind breaking the lines a bit earlier, between column 70 and 75? > +# Since 7.2 > # > # Since: 2.12 > ## > { 'struct' : 'DisplayGTK', > 'data' : { '*grab-on-hover' : 'bool', > '*zoom-to-fit' : 'bool', > - '*show-tabs' : 'bool' } } > + '*show-tabs' : 'bool', > + '*connector' : ['str'] } } > > ## > # @DisplayEGLHeadless: > diff --git a/qemu-options.hx b/qemu-options.hx > index 31c04f7eea..576b65ef9f 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1945,6 +1945,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, > #if defined(CONFIG_GTK) > "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n" > " > [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]\n" > + " [,connector.<id of VC>=<connector name>]\n" Is "<id of VC>" a VC number? > #endif > #if defined(CONFIG_VNC) > "-display vnc=<display>[,<optargs>]\n" Remainder of my review is on style only. Style suggestions are not demands :) > diff --git a/ui/gtk.c b/ui/gtk.c > index 945c550909..651aaaf174 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -37,6 +37,7 @@ > #include "qapi/qapi-commands-misc.h" > #include "qemu/cutils.h" > #include "qemu/main-loop.h" > +#include "qemu/option.h" > > #include "ui/console.h" > #include "ui/gtk.h" > @@ -115,6 +116,7 @@ > #endif > > #define HOTKEY_MODIFIERS (GDK_CONTROL_MASK | GDK_MOD1_MASK) > +#define MAX_NUM_ATTEMPTS 5 Could use a comment, and maybe a more telling name (this one doesn't tell me what is being attempted). > > static const guint16 *keycode_map; > static size_t keycode_maplen; > @@ -126,6 +128,15 @@ struct VCChardev { > }; > typedef struct VCChardev VCChardev; > > +struct gd_monitor_data { > + GtkDisplayState *s; > + GdkDisplay *dpy; > + GdkMonitor *monitor; > + QEMUTimer *hp_timer; > + int attempt; > +}; > +typedef struct gd_monitor_data gd_monitor_data; We usually contract these like typedef struct gd_monitor_data { ... } gd_monitor_data; > + > #define TYPE_CHARDEV_VC "chardev-vc" > DECLARE_INSTANCE_CHECKER(VCChardev, VC_CHARDEV, > TYPE_CHARDEV_VC) > @@ -1385,6 +1396,147 @@ static void gd_menu_untabify(GtkMenuItem *item, void > *opaque) > } > } > > +static void gd_monitor_fullscreen(GdkDisplay *dpy, VirtualConsole *vc, > + gint monitor_num) > +{ > + GtkDisplayState *s = vc->s; > + > + if (!vc->window) { > + gd_tab_window_create(vc); > + } > + gtk_window_fullscreen_on_monitor(GTK_WINDOW(vc->window), > + gdk_display_get_default_screen(dpy), > + monitor_num); > + s->full_screen = TRUE; > + gd_update_cursor(vc); > +} > + > +static int gd_monitor_lookup(GdkDisplay *dpy, char *label) > +{ > + GdkMonitor *monitor; > + const char *monitor_name; > + int i, total_monitors; > + > + total_monitors = gdk_display_get_n_monitors(dpy); > + for (i = 0; i < total_monitors; i++) { Suggest to format like this: int total_monitors = gdk_display_get_n_monitors(dpy); GdkMonitor *monitor; const char *monitor_name; int i; for (i = 0; i < total_monitors; i++) { > + monitor = gdk_display_get_monitor(dpy, i); > + if (monitor) { > + monitor_name = gdk_monitor_get_model(monitor); > + if (monitor_name && !strcmp(monitor_name, label)) { Would if (monitor && !g_strcmp0(gdk_monitor_get_model(monitor), label)) { do? > + return i; > + } > + } > + } > + return -1; > +} > + > +static void gd_monitor_check_vcs(GdkDisplay *dpy, GdkMonitor *monitor, > + GtkDisplayState *s) > +{ > + VirtualConsole *vc; > + const char *monitor_name = gdk_monitor_get_model(monitor); > + int i; > + > + for (i = 0; i < s->nb_vcs; i++) { > + vc = &s->vc[i]; > + if (!strcmp(vc->label, monitor_name)) { > + gd_monitor_fullscreen(dpy, vc, gd_monitor_lookup(dpy, > vc->label)); > + gd_set_ui_size(vc, surface_width(vc->gfx.ds), > + surface_height(vc->gfx.ds)); > + break; > + } > + } > +} > + > +static void gd_monitor_hotplug_timer(void *opaque) > +{ > + gd_monitor_data *data = opaque; > + const char *monitor_name = gdk_monitor_get_model(data->monitor); > + > + if (monitor_name) { > + gd_monitor_check_vcs(data->dpy, data->monitor, data->s); > + } > + if (monitor_name || data->attempt == MAX_NUM_ATTEMPTS) { > + timer_del(data->hp_timer); > + g_free(data); > + } else { > + data->attempt++; > + timer_mod(data->hp_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > 50); Suggest to break the line like timer_mod(data->hp_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 50); for readability. > + } > +} > + > +static void gd_monitor_add(GdkDisplay *dpy, GdkMonitor *monitor, > + void *opaque) > +{ > + GtkDisplayState *s = opaque; > + gd_monitor_data *data; > + const char *monitor_name = gdk_monitor_get_model(monitor); > + > + if (!monitor_name) { > + data = g_malloc0(sizeof(*data)); > + data->s = s; > + data->dpy = dpy; > + data->monitor = monitor; > + data->hp_timer = timer_new_ms(QEMU_CLOCK_REALTIME, > + gd_monitor_hotplug_timer, data); > + timer_mod(data->hp_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > 50); > + } else { > + gd_monitor_check_vcs(dpy, monitor, s); > + } Often if (cond) { do stuff when cond } else { do stuff when !cond } is easier to read than if (!cond) { do stuff when !cond } else { do stuff when !!cond } Give it a thought. > +} > + > +static void gd_monitor_remove(GdkDisplay *dpy, GdkMonitor *monitor, > + void *opaque) > +{ > + GtkDisplayState *s = opaque; > + VirtualConsole *vc; > + const char *monitor_name = gdk_monitor_get_model(monitor); > + int i; > + > + if (!monitor_name) { > + return; > + } > + for (i = 0; i < s->nb_vcs; i++) { > + vc = &s->vc[i]; > + if (!strcmp(vc->label, monitor_name)) { > + gd_tab_window_close(NULL, NULL, vc); > + gd_set_ui_size(vc, 0, 0); > + break; > + } > + } > +} > + > +static void gd_connectors_init(GdkDisplay *dpy, GtkDisplayState *s) > +{ > + VirtualConsole *vc; > + strList *connector = s->opts->u.gtk.connector; > + gint page_num = 0, monitor_num; > + > + gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE); > + gtk_widget_hide(s->menu_bar); > + for (; connector; connector = connector->next) { Please don't split off part of the loop control. What about for (conn = s->opts->u.gtk.connector; conn; conn = conn->next) { ? > + vc = gd_vc_find_by_page(s, page_num); > + if (!vc) { > + break; > + } > + if (page_num == 0) { > + vc->window = s->window; > + } > + > + g_free(vc->label); > + vc->label = g_strdup(connector->value); > + monitor_num = gd_monitor_lookup(dpy, vc->label); > + if (monitor_num >= 0) { > + gd_monitor_fullscreen(dpy, vc, monitor_num); > + gd_set_ui_size(vc, surface_width(vc->gfx.ds), > + surface_height(vc->gfx.ds)); > + } else { > + gd_set_ui_size(vc, 0, 0); > + } > + page_num++; > + } > +} > + > static void gd_menu_show_menubar(GtkMenuItem *item, void *opaque) > { > GtkDisplayState *s = opaque; > @@ -1705,7 +1857,14 @@ static gboolean gd_configure(GtkWidget *widget, > GdkEventConfigure *cfg, gpointer opaque) > { > VirtualConsole *vc = opaque; > + GtkDisplayState *s = vc->s; > + GtkWidget *parent = gtk_widget_get_parent(widget); > > + if (s->opts->u.gtk.has_connector) { > + if (!parent || !GTK_IS_WINDOW(parent)) { > + return FALSE; > + } > + } > gd_set_ui_size(vc, cfg->width, cfg->height); > return FALSE; > } > @@ -2038,6 +2197,12 @@ static void gd_connect_signals(GtkDisplayState *s) > G_CALLBACK(gd_menu_grab_input), s); > g_signal_connect(s->notebook, "switch-page", > G_CALLBACK(gd_change_page), s); > + if (s->opts->u.gtk.has_connector) { > + g_signal_connect(gtk_widget_get_display(s->window), "monitor-added", > + G_CALLBACK(gd_monitor_add), s); > + g_signal_connect(gtk_widget_get_display(s->window), > "monitor-removed", > + G_CALLBACK(gd_monitor_remove), s); > + } > } > > static GtkWidget *gd_create_menu_machine(GtkDisplayState *s) > @@ -2402,6 +2567,9 @@ static void gtk_display_init(DisplayState *ds, > DisplayOptions *opts) > opts->u.gtk.show_tabs) { > gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item)); > } > + if (s->opts->u.gtk.has_connector) { > + gd_connectors_init(window_display, s); > + } > gd_clipboard_init(s); > }