| Message ID | 20181107104921.20536-2-lhrazky@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | QXL interface to set monitor ID | expand |
Hi, > + * The device_display_id_{start,count} denotes the sequence of device display > + * IDs that map to the zero-based sequence of monitor IDs provided by monitors > + * config on this interface. For example with device_display_id_start = 2 and > + * device_display_id_count = 3 you get the following mapping: > + * monitor_id -> device_display_id > + * 0 -> 2 > + * 1 -> 3 > + * 2 -> 4 > + * > + * Note this example is unsupported in practice. The only supported cases are > + * either a single device display ID (count = 1) or multiple device display IDs > + * in a sequence starting from 0. This is confusing. The usage of this api in the qemu counterpart looks sane though. cheers, Gerd
Hello, On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote: > Hi, > > > + * The device_display_id_{start,count} denotes the sequence of device display > > + * IDs that map to the zero-based sequence of monitor IDs provided by monitors > > + * config on this interface. For example with device_display_id_start = 2 and > > + * device_display_id_count = 3 you get the following mapping: > > + * monitor_id -> device_display_id > > + * 0 -> 2 > > + * 1 -> 3 > > + * 2 -> 4 > > + * > > + * Note this example is unsupported in practice. The only supported cases are > > + * either a single device display ID (count = 1) or multiple device display IDs > > + * in a sequence starting from 0. > > This is confusing. The usage of this api in the qemu counterpart looks > sane though. Not sure what you find confusing in particular... The example? Using an example and then saying it's not supported? (I wated to use a nontrivial example that will show the concept...) Suggestions for improvement? Thanks, Lukas > cheers, > Gerd >
On Thu, 2018-11-08 at 11:05 +0100, Lukáš Hrázký wrote: > Hello, > > On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote: > > Hi, > > > > > + * The device_display_id_{start,count} denotes the sequence of > > > device display > > > + * IDs that map to the zero-based sequence of monitor IDs > > > provided by monitors > > > + * config on this interface. For example with > > > device_display_id_start = 2 and > > > + * device_display_id_count = 3 you get the following mapping: > > > + * monitor_id -> device_display_id > > > + * 0 -> 2 > > > + * 1 -> 3 > > > + * 2 -> 4 > > > + * > > > + * Note this example is unsupported in practice. The only > > > supported cases are > > > + * either a single device display ID (count = 1) or multiple > > > device display IDs > > > + * in a sequence starting from 0. > > > > This is confusing. The usage of this api in the qemu counterpart > > looks > > sane though. > > Not sure what you find confusing in particular... The example? Using > an > example and then saying it's not supported? (I wated to use a > nontrivial example that will show the concept...) Suggestions for > improvement? It is funny that you chose an unsupported example for demonstration, but I can't think of a better option. The supported scenarios don't demonstrate the full behavior very well... > > Thanks, > Lukas > > > cheers, > > Gerd > > > > _______________________________________________ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel
On Thu, Nov 08, 2018 at 11:05:10AM +0100, Lukáš Hrázký wrote: > Hello, > > On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote: > > Hi, > > > > > + * The device_display_id_{start,count} denotes the sequence of device display > > > + * IDs that map to the zero-based sequence of monitor IDs provided by monitors > > > + * config on this interface. For example with device_display_id_start = 2 and > > > + * device_display_id_count = 3 you get the following mapping: > > > + * monitor_id -> device_display_id > > > + * 0 -> 2 > > > + * 1 -> 3 > > > + * 2 -> 4 > > > + * > > > + * Note this example is unsupported in practice. The only supported cases are > > > + * either a single device display ID (count = 1) or multiple device display IDs > > > + * in a sequence starting from 0. > > > > This is confusing. The usage of this api in the qemu counterpart looks > > sane though. > > Not sure what you find confusing in particular... The example? Using an > example and then saying it's not supported? Yes, that for example. Also wondering why device_display_id doesn't start at zero. cheers, Gerd
> On Thu, Nov 08, 2018 at 11:05:10AM +0100, Lukáš Hrázký wrote: > > Hello, > > > > On Thu, 2018-11-08 at 07:49 +0100, Gerd Hoffmann wrote: > > > Hi, > > > > > > > + * The device_display_id_{start,count} denotes the sequence of device > > > > display > > > > + * IDs that map to the zero-based sequence of monitor IDs provided by > > > > monitors > > > > + * config on this interface. For example with device_display_id_start > > > > = 2 and > > > > + * device_display_id_count = 3 you get the following mapping: > > > > + * monitor_id -> device_display_id > > > > + * 0 -> 2 > > > > + * 1 -> 3 > > > > + * 2 -> 4 > > > > + * > > > > + * Note this example is unsupported in practice. The only supported > > > > cases are > > > > + * either a single device display ID (count = 1) or multiple device > > > > display IDs > > > > + * in a sequence starting from 0. > > > > > > This is confusing. The usage of this api in the qemu counterpart looks > > > sane though. > > > > Not sure what you find confusing in particular... The example? Using an > > example and then saying it's not supported? > > Yes, that for example. Also wondering why device_display_id doesn't > start at zero. > You introduced this ID so you should remember. It starts from 0, just this is not the first registration, the other displays were registered to other QXL instances. This was introduced for virtio-gpu to be able to register the different outputs to different QXL instances so to have first output/display: spice_qxl_set_device_info(instance1, path1, 0, 1); second: spice_qxl_set_device_info(instance2, path1, 1, 1); third: spice_qxl_set_device_info(instance3, path1, 2, 1); So as you can see device_display_id start from 0 but does not mean is always 0 for every call. > cheers, > Gerd > > > Frediano
Hi, > first output/display: > spice_qxl_set_device_info(instance1, path1, 0, 1); > second: > spice_qxl_set_device_info(instance2, path1, 1, 1); > third: > spice_qxl_set_device_info(instance3, path1, 2, 1); That is a much better example IMHO. cheers, Gerd
diff --git a/server/red-qxl.c b/server/red-qxl.c index 97940611..0ea424cd 100644 --- a/server/red-qxl.c +++ b/server/red-qxl.c @@ -41,6 +41,9 @@ #include "red-qxl.h" +#define MAX_DEVICE_ADDRESS_LEN 256 +#define MAX_MONITORS_COUNT 16 + struct QXLState { QXLWorker qxl_worker; QXLInstance *qxl; @@ -53,6 +56,9 @@ struct QXLState { unsigned int max_monitors; RedsState *reds; RedWorker *worker; + char device_address[MAX_DEVICE_ADDRESS_LEN]; + uint32_t device_display_ids[MAX_MONITORS_COUNT]; + size_t monitors_count; // length of ^^^ pthread_mutex_t scanout_mutex; SpiceMsgDisplayGlScanoutUnix scanout; @@ -846,6 +852,44 @@ void red_qxl_gl_draw_async_complete(QXLInstance *qxl) red_qxl_async_complete(qxl, cookie); } +SPICE_GNUC_VISIBLE +void spice_qxl_set_device_info(QXLInstance *instance, + const char *device_address, + uint32_t device_display_id_start, + uint32_t device_display_id_count) +{ + g_return_if_fail(device_address != NULL); + + size_t da_len = strnlen(device_address, MAX_DEVICE_ADDRESS_LEN); + if (da_len >= MAX_DEVICE_ADDRESS_LEN) { + spice_error("Device address too long: %lu > %u", da_len, MAX_DEVICE_ADDRESS_LEN); + return; + } + + if (device_display_id_count > MAX_MONITORS_COUNT) { + spice_error("Device display ID count (%u) is greater than limit %u", + device_display_id_count, + MAX_MONITORS_COUNT); + return; + } + + strncpy(instance->st->device_address, device_address, MAX_DEVICE_ADDRESS_LEN); + + g_debug("QXL Instance %d setting device address \"%s\" and monitor -> device display mapping:", + instance->id, + device_address); + + // store the mapping monitor_id -> device_display_id + for (uint32_t monitor_id = 0; monitor_id < device_display_id_count; ++monitor_id) { + uint32_t device_display_id = device_display_id_start + monitor_id; + instance->st->device_display_ids[monitor_id] = device_display_id; + g_debug(" monitor ID %u -> device display ID %u", + monitor_id, device_display_id); + } + + instance->st->monitors_count = device_display_id_count; +} + void red_qxl_init(RedsState *reds, QXLInstance *qxl) { QXLState *qxl_state; diff --git a/server/spice-qxl.h b/server/spice-qxl.h index 0c4e75fc..547d3d93 100644 --- a/server/spice-qxl.h +++ b/server/spice-qxl.h @@ -115,6 +115,52 @@ void spice_qxl_gl_draw_async(QXLInstance *instance, uint32_t w, uint32_t h, uint64_t cookie); +/* since spice 0.14.2 */ + +/** + * spice_qxl_set_device_info: + * @instance the QXL instance to set the path to + * @device_address the path of the device this QXL instance belongs to + * @device_display_id_start the starting device display ID of this interface, + * i.e. the one monitor ID 0 maps to + * @device_display_id_count the total number of device display IDs on this + * interface + * + * Sets the device information for this QXL interface, i.e. the hardware + * address (e.g. PCI) of the graphics device and the IDs of the displays of the + * graphics device that are exposed by this interface (device display IDs). + * + * The supported device address format is: + * "pci/<DOMAIN>/<SLOT>.<FUNCTION>/.../<SLOT>.<FUNCTION>" + * + * The "pci" identifies the rest of the string as a PCI address. It is the only + * supported address at the moment, other identifiers can be introduced later. + * <DOMAIN> is the PCI domain, followed by <SLOT>.<FUNCTION> of any PCI bridges + * in the chain leading to the device. The last <SLOT>.<FUNCTION> is the + * graphics device. All of <DOMAIN>, <SLOT>, <FUNCTION> are hexadecimal numbers + * with the following number of digits: + * <DOMAIN>: 4 + * <SLOT>: 2 + * <FUNCTION>: 1 + * + * The device_display_id_{start,count} denotes the sequence of device display + * IDs that map to the zero-based sequence of monitor IDs provided by monitors + * config on this interface. For example with device_display_id_start = 2 and + * device_display_id_count = 3 you get the following mapping: + * monitor_id -> device_display_id + * 0 -> 2 + * 1 -> 3 + * 2 -> 4 + * + * Note this example is unsupported in practice. The only supported cases are + * either a single device display ID (count = 1) or multiple device display IDs + * in a sequence starting from 0. + */ +void spice_qxl_set_device_info(QXLInstance *instance, + const char *device_address, + uint32_t device_display_id_start, + uint32_t device_display_id_count); + typedef struct QXLDevInitInfo { uint32_t num_memslots_groups; uint32_t num_memslots; diff --git a/server/spice-server.syms b/server/spice-server.syms index edf04a42..ac4d9b14 100644 --- a/server/spice-server.syms +++ b/server/spice-server.syms @@ -173,3 +173,8 @@ SPICE_SERVER_0.13.2 { global: spice_server_set_video_codecs; } SPICE_SERVER_0.13.1; + +SPICE_SERVER_0.14.2 { +global: + spice_qxl_set_device_info; +} SPICE_SERVER_0.13.2;
Adds a function to let QEMU provide information to identify graphics devices and their monitors in the guest. The function (spice_qxl_set_device_info) sets the device address (e.g. a PCI path) and monitor ID -> device display ID mapping of displays exposed by given QXL interface. Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com> --- server/red-qxl.c | 44 ++++++++++++++++++++++++++++++++++++++ server/spice-qxl.h | 46 ++++++++++++++++++++++++++++++++++++++++ server/spice-server.syms | 5 +++++ 3 files changed, 95 insertions(+)