On Fri, Apr 2, 2021 at 12:56 AM Zhang, Tina tina.zhang@intel.com wrote:
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of
Gerd
Hoffmann Sent: Wednesday, March 31, 2021 4:00 PM To: Kasireddy, Vivek vivek.kasireddy@intel.com Cc: dri-devel@lists.freedesktop.org; gurchetansingh@chromium.org Subject: Re: [PATCH 2/2] drm/virtio: Include modifier as part of set_scanout_blob
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?
Hi,
I agree that we need a way to get the supported modifiers' info to guest user space mesa, specifically to the iris driver working in kmsro mode. So, from the guest mesa iris driver's point of view, the working flow may like this:
- Get the modifier info from a display device through the kms_fd. In our
case, the kms_fd comes from the virtio-gpu driver. So the info should come from virtio-gpu device. 2) When guest wants to allocate a scan-out buffer, the iris driver needs to check which format and modifier is suitable to be used. 3) Then, iris uses the kms_fd to allocate the scan-out buffer with the desired format. Maybe this time we'd better consider using VIRTGPU_RESOURCE_CREATE to allocate buffer instead of using DRM_IOCTL_MODE_CREATE_DUMB. It seems VIRTGPU_RESUORECE_CREATE can give more fb info.
Thank you Tina and Vivek for looking into this! Added some commentary on your Mesa side MR.
BR, Tina
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
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel