Re: [PATCH 2/2] drm/virtio: Modify RESOURCE_GET_LAYOUT ioctl

[Dmitry Osipenko]

On 11/10/23 10:16, Julia Zhang wrote:
> Modify RESOURCE_GET_LAYOUT ioctl to handle the use case that query
> correct stride for guest linear resource before it is created.
> 
> Signed-off-by: Julia Zhang 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 26 --
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 47 --
>  drivers/gpu/drm/virtio/virtgpu_vq.c| 35 +++
>  include/uapi/drm/virtgpu_drm.h |  6 ++--
>  include/uapi/linux/virtio_gpu.h|  8 ++---
>  5 files changed, 66 insertions(+), 56 deletions(-)

1. Please squash this all into a single patch. For upstream kernel it's
not acceptable to have subsequent commits modifying previous commits. To
commit message add your s-o-b, your co-developed-by tags and a brief
comment explaining changes you've done to the original patch.

Signed-off-by: Daniel Stone 
Co-developed-by: Julia Zhang  # query correct
stride for guest linear resource before it's created
Signed-off-by: Julia Zhang 

2. Make sure that patch passes `scripts/checkpatch.pl`

3. Add link to the commit message for the relevant Mesa MR that makes
use of the new ioctl. The MR should be already merged or ready to be merged.

Link: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/123456

-- 
Best regards,
Dmitry

[PATCH 2/2] drm/virtio: Modify RESOURCE_GET_LAYOUT ioctl

[Julia Zhang]

Modify RESOURCE_GET_LAYOUT ioctl to handle the use case that query
correct stride for guest linear resource before it is created.

Signed-off-by: Julia Zhang 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h   | 26 --
 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 47 --
 drivers/gpu/drm/virtio/virtgpu_vq.c| 35 +++
 include/uapi/drm/virtgpu_drm.h |  6 ++--
 include/uapi/linux/virtio_gpu.h|  8 ++---
 5 files changed, 66 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index d6fc0d4ecb7d..82dffb3e4c6b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -93,15 +93,6 @@ struct virtio_gpu_object {
bool host3d_blob, guest_blob;
uint32_t blob_mem, blob_flags;
 
-   atomic_t layout_state;
-   struct {
-   uint64_t offset;
-   uint64_t size;
-   uint32_t stride;
-   } planes[VIRTIO_GPU_RES_MAX_PLANES];
-   uint64_t modifier;
-   uint32_t num_planes;
-
int uuid_state;
uuid_t uuid;
 
@@ -225,6 +216,16 @@ struct virtio_gpu_drv_cap_cache {
atomic_t is_valid;
 };
 
+struct virtio_gpu_query_info {
+   uint32_t num_planes;
+   uint64_t modifier;
+   struct {
+   uint64_t offset;
+   uint32_t stride;
+   } planes [VIRTIO_GPU_MAX_RESOURCE_PLANES];
+   atomic_t is_valid;
+};
+
 struct virtio_gpu_device {
struct drm_device *ddev;
 
@@ -448,7 +449,12 @@ void virtio_gpu_cmd_host_wait(struct virtio_gpu_device 
*vgdev,
 
 int
 virtio_gpu_cmd_get_resource_layout(struct virtio_gpu_device *vgdev,
-  struct virtio_gpu_object *bo);
+  struct virtio_gpu_query_info *bo_info,
+  uint32_t width,
+  uint32_t height,
+  uint32_t format,
+  uint32_t bind,
+  uint32_t hw_res_handle);
 
 /* virtgpu_display.c */
 int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c 
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 51d04460d0d8..034a7c0927a5 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -685,9 +685,9 @@ static int virtio_gpu_resource_query_layout_ioctl(struct 
drm_device *dev,
 {
struct drm_virtgpu_resource_query_layout *args = data;
struct virtio_gpu_device *vgdev = dev->dev_private;
-   struct drm_gem_object *obj;
-   struct virtio_gpu_object *bo;
-   int layout_state;
+   struct drm_gem_object *obj = NULL;
+   struct virtio_gpu_object *bo = NULL;
+   struct virtio_gpu_query_info bo_info = {0};
int ret = 0;
int i;
 
@@ -696,50 +696,45 @@ static int virtio_gpu_resource_query_layout_ioctl(struct 
drm_device *dev,
return -EINVAL;
}
 
-   obj = drm_gem_object_lookup(file, args->handle);
-   if (obj == NULL) {
-   DRM_ERROR("invalid handle 0x%x\n", args->handle);
-   return -ENOENT;
-   }
-   bo = gem_to_virtio_gpu_obj(obj);
-
-   layout_state = atomic_read(>layout_state);
-   if (layout_state == STATE_ERR) {
-   ret = -EINVAL;
-   goto out;
-   } else if (layout_state == STATE_OK) {
-   goto valid;
+   if (args->handle > 0) {
+   obj = drm_gem_object_lookup(file, args->handle);
+   if (obj == NULL) {
+   DRM_ERROR("invalid handle 0x%x\n", args->handle);
+   return -ENOENT;
+   }
+   bo = gem_to_virtio_gpu_obj(obj);
}
 
-   ret = virtio_gpu_cmd_get_resource_layout(vgdev, bo);
+   ret = virtio_gpu_cmd_get_resource_layout(vgdev, _info, args->width,
+args->height, args->format,
+args->bind, bo ? 
bo->hw_res_handle : 0);
if (ret)
goto out;
 
ret = wait_event_timeout(vgdev->resp_wq,
-atomic_read(>layout_state) == STATE_OK,
+atomic_read(_info.is_valid),
 5 * HZ);
if (!ret)
goto out;
 
 valid:
smp_rmb();
-   WARN_ON(atomic_read(>layout_state) != STATE_OK);
-   args->num_planes = bo->num_planes;
-   args->modifier = bo->modifier;
+   WARN_ON(atomic_read(_info.is_valid));
+   args->num_planes = bo_info.num_planes;
+   args->modifier = bo_info.modifier;
for (i = 0; i < args->num_planes; i++) {
-   args->planes[i].offset = bo->planes[i].offset;
-   args->planes[i].size = bo->planes[i].size;
-   args->planes[i].stride =