<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Apr 2, 2021 at 12:56 AM Zhang, Tina <<a href="mailto:tina.zhang@intel.com">tina.zhang@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> -----Original Message-----<br>
> From: dri-devel <<a href="mailto:dri-devel-bounces@lists.freedesktop.org" target="_blank">dri-devel-bounces@lists.freedesktop.org</a>> On Behalf Of Gerd<br>
> Hoffmann<br>
> Sent: Wednesday, March 31, 2021 4:00 PM<br>
> To: Kasireddy, Vivek <<a href="mailto:vivek.kasireddy@intel.com" target="_blank">vivek.kasireddy@intel.com</a>><br>
> Cc: <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a>; <a href="mailto:gurchetansingh@chromium.org" target="_blank">gurchetansingh@chromium.org</a><br>
> Subject: Re: [PATCH 2/2] drm/virtio: Include modifier as part of<br>
> set_scanout_blob<br>
> <br>
> Hi,<br>
> <br>
> > -#define MAX_INLINE_CMD_SIZE 96<br>
> > +#define MAX_INLINE_CMD_SIZE 112<br>
> <br>
> Separate patch please.<br>
> <br>
> > --- a/include/uapi/linux/virtio_gpu.h<br>
> > +++ b/include/uapi/linux/virtio_gpu.h<br>
> > @@ -409,6 +409,7 @@ struct virtio_gpu_set_scanout_blob {<br>
> > __le32 width;<br>
> > __le32 height;<br>
> > __le32 format;<br>
> > + __le64 modifier;<br>
> > __le32 padding;<br>
> > __le32 strides[4];<br>
> > __le32 offsets[4];<br>
> <br>
> Nope. This breaks the virtio protocol.<br>
> <br>
> We'll need a virtio feature flag to negotiate modifier support between guest and<br>
> host. When supported by both sides it can be used. The new field should be<br>
> appended, not inserted in the middle. Or we create a new<br>
> virtio_gpu_set_scanout_blob2 struct with new command for this.<br>
> <br>
> Also: I guess the device should provide a list of supported modifiers, maybe as<br>
> capset?<br>
<br>
Hi,<br>
<br>
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.<br>
So, from the guest mesa iris driver's point of view, the working flow may like this:<br>
1) 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.<br>
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.<br>
3) Then, iris uses the kms_fd to allocate the scan-out buffer with the desired format.<br>
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.<br></blockquote><div><br></div><div>Thank you Tina and Vivek for looking into this! Added some commentary on your Mesa side MR.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
BR,<br>
Tina<br>
<br>
> <br>
> Note: I think it is already possible to create resources with modifiers when using<br>
> virgl commands for that. Allowing modifiers with virgl=off too makes sense<br>
> given your use case, but we should not use diverging approaches for virgl=on vs.<br>
> virgl=off. Specifically I'm not sure virtio_gpu_set_scanout_blob is the right place,<br>
> I think with virgl=on the modifier is linked to the resource not the scanout ...<br>
> <br>
> Cc'ing Gurchetan Singh for comments.<br>
> <br>
> take care,<br>
> Gerd<br>
> <br>
> _______________________________________________<br>
> dri-devel mailing list<br>
> <a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/dri-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</blockquote></div></div>