[PATCH 2/2] drm/virtio: Include modifier as part of set_scanout_blob

[Gerd Hoffmann]

  Hi,

> -#define MAX_INLINE_CMD_SIZE   96
> +#define MAX_INLINE_CMD_SIZE   112

Separate patch please.

> --- a/include/uapi/linux/virtio_gpu.h
> +++ b/include/uapi/linux/virtio_gpu.h
> @@ -409,6 +409,7 @@ struct virtio_gpu_set_scanout_blob {
>  	__le32 width;
>  	__le32 height;
>  	__le32 format;
> +	__le64 modifier;
>  	__le32 padding;
>  	__le32 strides[4];
>  	__le32 offsets[4];

Nope.  This breaks the virtio protocol.

We'll need a virtio feature flag to negotiate modifier support between
guest and host.  When supported by both sides it can be used.  The new
field should be appended, not inserted in the middle.  Or we create a
new virtio_gpu_set_scanout_blob2 struct with new command for this.

Also: I guess the device should provide a list of supported modifiers,
maybe as capset?

Note: I think it is already possible to create resources with modifiers
when using virgl commands for that.  Allowing modifiers with virgl=off
too makes sense given your use case, but we should not use diverging
approaches for virgl=on vs. virgl=off.  Specifically I'm not sure
virtio_gpu_set_scanout_blob is the right place, I think with virgl=on
the modifier is linked to the resource not the scanout ...

Cc'ing Gurchetan Singh for comments.

take care,
  Gerd