minigbm: virtio-gpu: wait for transfers when necessary

There are two situations where guest access and host access to buffers
need to be synchronized to ensure consistency:

  - If the guest CPU can write to the mapping and something in the host
    besides the GPU might access the buffer, flush must be synchronous
    to ensure the host sees any guest changes. Waiting is not necessary
    if only the GPU can access the buffer because any subsequent
    commands will be properly ordered.
  - If the guest CPU can access the mapping and something in the host
    can write to the buffer, then invalidate must be synchronous to
    ensure the guest sees any host changes.

To perform this synchronization, have the virtio_gpu backend wait for
the transfer to/from the host to complete before returning from
flush/invalidate. In the future, support for fence-based synchronization
should also be added.

BUG=b:120456557 b:136733358
TEST=arc-codec-test, ArcVideoPlayer

Change-Id: If4ad293886a67d93d85b0d4a682b31c66597d4ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/1687736
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: David Stevens <stevensd@chromium.org>
Tested-by: David Stevens <stevensd@chromium.org>

diff --git a/drv_priv.h b/drv_priv.h
index 1e0e75b..76fa443 100644 (file)
--- a/drv_priv.h
+++ b/drv_priv.h
@@ -75,16 +75,19 @@ struct backend {
 };
 
 // clang-format off
-#define BO_USE_RENDER_MASK BO_USE_LINEAR | BO_USE_PROTECTED | BO_USE_RENDERING | \
+#define BO_USE_RENDER_MASK (BO_USE_LINEAR | BO_USE_PROTECTED | BO_USE_RENDERING | \
                           BO_USE_RENDERSCRIPT | BO_USE_SW_READ_OFTEN | BO_USE_SW_WRITE_OFTEN | \
-                           BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY | BO_USE_TEXTURE
+                           BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY | BO_USE_TEXTURE)
 
-#define BO_USE_TEXTURE_MASK BO_USE_LINEAR | BO_USE_PROTECTED | BO_USE_RENDERSCRIPT | \
+#define BO_USE_TEXTURE_MASK (BO_USE_LINEAR | BO_USE_PROTECTED | BO_USE_RENDERSCRIPT | \
                            BO_USE_SW_READ_OFTEN | BO_USE_SW_WRITE_OFTEN | \
-                            BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY | BO_USE_TEXTURE
+                            BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY | BO_USE_TEXTURE)
 
-#define BO_USE_SW_MASK BO_USE_SW_READ_OFTEN | BO_USE_SW_WRITE_OFTEN | \
-                      BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY
+#define BO_USE_SW_MASK (BO_USE_SW_READ_OFTEN | BO_USE_SW_WRITE_OFTEN | \
+                      BO_USE_SW_READ_RARELY | BO_USE_SW_WRITE_RARELY)
+
+#define BO_USE_NON_GPU_HW (BO_USE_SCANOUT | BO_USE_CAMERA_WRITE | BO_USE_CAMERA_READ | \
+                         BO_USE_HW_VIDEO_ENCODER | BO_USE_HW_VIDEO_DECODER)
 
 #ifndef DRM_FORMAT_MOD_LINEAR
 #define DRM_FORMAT_MOD_LINEAR DRM_FORMAT_MOD_NONE

diff --git a/virtio_gpu.c b/virtio_gpu.c
index edc8fc9..d6c9974 100644 (file)
--- a/virtio_gpu.c
+++ b/virtio_gpu.c
@@ -378,7 +378,8 @@ static int virtio_gpu_bo_invalidate(struct bo *bo, struct mapping *mapping)
                return 0;
 
        // Invalidate is only necessary if the host writes to the buffer.
-       if ((bo->meta.use_flags & BO_USE_RENDERING) == 0)
+       if ((bo->meta.use_flags & (BO_USE_RENDERING | BO_USE_CAMERA_WRITE |
+                                  BO_USE_HW_VIDEO_ENCODER | BO_USE_HW_VIDEO_DECODER)) == 0)
                return 0;
 
        memset(&xfer, 0, sizeof(xfer));
@@ -389,8 +390,16 @@ static int virtio_gpu_bo_invalidate(struct bo *bo, struct mapping *mapping)
        xfer.box.h = mapping->rect.height;
        xfer.box.d = 1;
 
-       // TODO(b/145993887): Send also stride when the patches are landed
-       // When BO_USE_RENDERING is set, the level=stride hack is not needed
+       if ((bo->meta.use_flags & BO_USE_RENDERING) == 0) {
+               // Unfortunately, the kernel doesn't actually pass the guest layer_stride and
+               // guest stride to the host (compare virtio_gpu.h and virtgpu_drm.h). For gbm
+               // based resources, we can work around this by using the level field to pass
+               // the stride to virglrenderer's gbm transfer code. However, we need to avoid
+               // doing this for resources which don't rely on that transfer code, which is
+               // resources with the BO_USE_RENDERING flag set.
+               // TODO(b/145993887): Send also stride when the patches are landed
+               xfer.level = bo->meta.strides[0];
+       }
 
        ret = drmIoctl(bo->drv->fd, DRM_IOCTL_VIRTGPU_TRANSFER_FROM_HOST, &xfer);
        if (ret) {
@@ -417,6 +426,7 @@ static int virtio_gpu_bo_flush(struct bo *bo, struct mapping *mapping)
        int ret;
        struct drm_virtgpu_3d_transfer_to_host xfer;
        struct virtio_gpu_priv *priv = (struct virtio_gpu_priv *)bo->drv->priv;
+       struct drm_virtgpu_3d_wait waitcmd;
 
        if (!priv->has_3d)
                return 0;
@@ -443,6 +453,21 @@ static int virtio_gpu_bo_flush(struct bo *bo, struct mapping *mapping)
                return -errno;
        }
 
+       // If the buffer is only accessed by the host GPU, then the flush is ordered
+       // with subsequent commands. However, if other host hardware can access the
+       // buffer, we need to wait for the transfer to complete for consistency.
+       // TODO(b/136733358): Support returning fences from transfers
+       if (bo->meta.use_flags & BO_USE_NON_GPU_HW) {
+               memset(&waitcmd, 0, sizeof(waitcmd));
+               waitcmd.handle = mapping->vma->handle;
+
+               ret = drmIoctl(bo->drv->fd, DRM_IOCTL_VIRTGPU_WAIT, &waitcmd);
+               if (ret) {
+                       drv_log("DRM_IOCTL_VIRTGPU_WAIT failed with %s\n", strerror(errno));
+                       return -errno;
+               }
+       }
+
        return 0;
 }