[Spice-devel] [protocol RFC 1/2] qxl_dev: dynamic head support on single display channel

[Hans de Goede]

Hi,

On 05/07/2012 08:29 AM, Alon Levy wrote:
> Adds QXLHead and QXL_RAM_FLAG_QXL_HEAD.
>
> This is meant to support multiple heads on a single device. The scheme
> is especially easy to use with randr drivers:
>
> 1. Maintain a single Primary per device, hence also a single Display
> channel.
> 2. Guest sends a QXLHead command per head, in which it specifies the
> head location relative to the primary.
> 3. QXLHead contains an id. To delete a head you send a QXLHead command
> with width=height=0
>
> Guest is notified of the spice server and qemu supporting the command
> (qemu doesn't actually need to change at all, except to set this flag)
> via QEMU_RAM_FLAG_QXL_HEAD on the QXLRam on the ram bar.
>
> spice-server will be updated to support the new command. It doesn't
> actually need to do anything with it, just read it from the command ring
> and send a SpiceHead command to the client.
>
> If a client connects that supports SpiceHead command receives it, it
> shall create a window for each active head id (one that has a non zero
> width and non zero height). That window reflects a part of the primary
> surface of the display.
>
> If a client connects that doesn't support SpiceHead command, then it
> shall see a single window of the primary surface in the worst case. This
> can be fixed by adding a new interrupt to signal to the guest the
> change, either via directly letting it see the guest capabilities, or
> via a change to the QXLRam flags. (TODO)
> ---
>   spice/qxl_dev.h |   12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/spice/qxl_dev.h b/spice/qxl_dev.h
> index 741a1f6..97c2926 100644
> --- a/spice/qxl_dev.h
> +++ b/spice/qxl_dev.h
> @@ -229,6 +229,10 @@ SPICE_RING_DECLARE(QXLReleaseRing, uint64_t, QXL_RELEASE_RING_SIZE);
>   #define QXL_INTERRUPT_IO_CMD (1<<  2)
>   #define QXL_INTERRUPT_ERROR  (1<<  3)
>
> +enum {
> +    QXL_RAM_FLAG_QXL_HEAD=0x01,
> +};
> +
>   /* qxl-1 compat: append only */
>   typedef struct SPICE_ATTR_PACKED QXLRam {
>       uint32_t magic;
> @@ -653,6 +657,14 @@ typedef struct SPICE_ATTR_PACKED QXLImage {
>       };
>   } QXLImage;
>
> +typedef struct SPICE_ATTR_PACKED QXLHead {
> +    uint32_t id;
> +    uint32_t x;
> +    uint32_t y;
> +    uint32_t width;
> +    uint32_t height;
> +} QXLHead;
> +

Maybe throw in a flags field for future usage?

Other then that the entire patch set looks good to me,
but this is not my area of expertise.

Regards,

Hans