On Mon, Nov 05, 2018 at 01:18:57PM +0100, Lukáš Hrázký wrote: > On Mon, 2018-11-05 at 07:52 +0100, Gerd Hoffmann wrote: > > > 2. Have a single function as follows: > > > > > > void spice_qxl_set_device_info(QXLInstance *instance, > > > const char *device_address, > > > uint32_t device_display_id_start, > > > uint32_t device_display_id_count); > > > > How about: > > > > void spice_qxl_set_device_info(QXLInstance *instance, > > const char *device_address, > > uint32_t device_display_id); > > > > I don't think we need start+count: > > > > * For single-head devices device_display_id will be zero. > > * For one-channel-per-head multihead devices (i.e. virtio-gpu) > > device_display_id will enumerate the heads (so everybody can figure > > which channel is which head). > > * For one-channel-per-device multihead devices (i.e. qxl/linux) > > device_display_id will be zero too. Number of heads is set via > > spice_qxl_set_max_monitors(). > > That requires nontrivial and unexpected logic for the one-channel-per- > device multihead devices case. The API should be doing what it says and > the dumber the better, this seems too smart to me... Well, the device_display_id_count argument is redundant with spice_qxl_set_max_monitors(). That isn't a great API either. I can see that it simplifies the logic in spice-server if we have a single function call instead of two. So we could deprecate spice_qxl_set_max_monitors() in favour of your spice_qxl_set_device_info() variant. spice_qxl_set_max_monitors() would then basically do this: spice_qxl_set_max_monitors(qxl, max) { spice_qxl_set_device_info(qxl, NULL, 0, max); } cheers, Gerd _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel