Switch to the virtio_gpu_array_* helper workflow.
Signed-off-by: Gerd Hoffmann <[email protected]>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 ++-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 35 +++++++++++---------------
drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++++--
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 78dc5a19a358..4df760ba018e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -302,9 +302,10 @@ void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev,
- uint32_t resource_id, uint32_t ctx_id,
+ uint32_t ctx_id,
uint64_t offset, uint32_t level,
struct virtio_gpu_box *box,
+ struct virtio_gpu_object_array *objs,
struct virtio_gpu_fence *fence);
void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo,
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 0d0acf0b85ed..56182abdbf36 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -298,8 +298,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
struct virtio_gpu_device *vgdev = dev->dev_private;
struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
struct drm_virtgpu_3d_transfer_from_host *args = data;
- struct drm_gem_object *gobj = NULL;
- struct virtio_gpu_object *qobj = NULL;
+ struct virtio_gpu_object_array *objs;
struct virtio_gpu_fence *fence;
int ret;
u32 offset = args->offset;
@@ -308,35 +307,31 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
if (vgdev->has_virgl_3d == false)
return -ENOSYS;
- gobj = drm_gem_object_lookup(file, args->bo_handle);
- if (gobj == NULL)
+ objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
+ if (objs == NULL)
return -ENOENT;
- qobj = gem_to_virtio_gpu_obj(gobj);
-
- ret = virtio_gpu_object_reserve(qobj);
- if (ret)
- goto out;
+ ret = virtio_gpu_array_lock_resv(objs);
+ if (ret != 0)
+ goto err_put_free;
convert_to_hw_box(&box, &args->box);
fence = virtio_gpu_fence_alloc(vgdev);
if (!fence) {
ret = -ENOMEM;
- goto out_unres;
+ goto err_unlock;
}
virtio_gpu_cmd_transfer_from_host_3d
- (vgdev, qobj->hw_res_handle,
- vfpriv->ctx_id, offset, args->level,
- &box, fence);
- reservation_object_add_excl_fence(qobj->base.base.resv,
- &fence->f);
-
+ (vgdev, vfpriv->ctx_id, offset, args->level,
+ &box, objs, fence);
dma_fence_put(&fence->f);
-out_unres:
- virtio_gpu_object_unreserve(qobj);
-out:
- drm_gem_object_put_unlocked(gobj);
+ return 0;
+
+err_unlock:
+ virtio_gpu_array_unlock_resv(objs);
+err_put_free:
+ virtio_gpu_array_put_free(objs);
return ret;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index fc908d5cb217..bef7036f4508 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -928,20 +928,24 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
}
void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev,
- uint32_t resource_id, uint32_t ctx_id,
+ uint32_t ctx_id,
uint64_t offset, uint32_t level,
struct virtio_gpu_box *box,
+ struct virtio_gpu_object_array *objs,
struct virtio_gpu_fence *fence)
{
+ struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
struct virtio_gpu_transfer_host_3d *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
+ vbuf->objs = objs;
+
cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D);
cmd_p->hdr.ctx_id = cpu_to_le32(ctx_id);
- cmd_p->resource_id = cpu_to_le32(resource_id);
+ cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
cmd_p->box = *box;
cmd_p->offset = cpu_to_le64(offset);
cmd_p->level = cpu_to_le32(level);
--
2.18.1
On Tue, Jul 2, 2019 at 7:19 AM Gerd Hoffmann <[email protected]> wrote:
>
> Switch to the virtio_gpu_array_* helper workflow.
(just repeating my question on patch 6)
Does this fix the obj refcount issue? When was the issue introduced?
>
> Signed-off-by: Gerd Hoffmann <[email protected]>
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 ++-
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 35 +++++++++++---------------
> drivers/gpu/drm/virtio/virtgpu_vq.c | 8 ++++--
> 3 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 78dc5a19a358..4df760ba018e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -302,9 +302,10 @@ void virtio_gpu_cmd_submit(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object_array *objs,
> struct virtio_gpu_fence *fence);
> void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev,
> - uint32_t resource_id, uint32_t ctx_id,
> + uint32_t ctx_id,
> uint64_t offset, uint32_t level,
> struct virtio_gpu_box *box,
> + struct virtio_gpu_object_array *objs,
> struct virtio_gpu_fence *fence);
> void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object *bo,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 0d0acf0b85ed..56182abdbf36 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -298,8 +298,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
> struct drm_virtgpu_3d_transfer_from_host *args = data;
> - struct drm_gem_object *gobj = NULL;
> - struct virtio_gpu_object *qobj = NULL;
> + struct virtio_gpu_object_array *objs;
> struct virtio_gpu_fence *fence;
> int ret;
> u32 offset = args->offset;
> @@ -308,35 +307,31 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev,
> if (vgdev->has_virgl_3d == false)
> return -ENOSYS;
>
> - gobj = drm_gem_object_lookup(file, args->bo_handle);
> - if (gobj == NULL)
> + objs = virtio_gpu_array_from_handles(file, &args->bo_handle, 1);
> + if (objs == NULL)
> return -ENOENT;
>
> - qobj = gem_to_virtio_gpu_obj(gobj);
> -
> - ret = virtio_gpu_object_reserve(qobj);
> - if (ret)
> - goto out;
> + ret = virtio_gpu_array_lock_resv(objs);
> + if (ret != 0)
> + goto err_put_free;
>
> convert_to_hw_box(&box, &args->box);
>
> fence = virtio_gpu_fence_alloc(vgdev);
> if (!fence) {
> ret = -ENOMEM;
> - goto out_unres;
> + goto err_unlock;
> }
> virtio_gpu_cmd_transfer_from_host_3d
> - (vgdev, qobj->hw_res_handle,
> - vfpriv->ctx_id, offset, args->level,
> - &box, fence);
> - reservation_object_add_excl_fence(qobj->base.base.resv,
> - &fence->f);
> -
> + (vgdev, vfpriv->ctx_id, offset, args->level,
> + &box, objs, fence);
> dma_fence_put(&fence->f);
> -out_unres:
> - virtio_gpu_object_unreserve(qobj);
> -out:
> - drm_gem_object_put_unlocked(gobj);
> + return 0;
> +
> +err_unlock:
> + virtio_gpu_array_unlock_resv(objs);
> +err_put_free:
> + virtio_gpu_array_put_free(objs);
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index fc908d5cb217..bef7036f4508 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -928,20 +928,24 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
> }
>
> void virtio_gpu_cmd_transfer_from_host_3d(struct virtio_gpu_device *vgdev,
> - uint32_t resource_id, uint32_t ctx_id,
> + uint32_t ctx_id,
> uint64_t offset, uint32_t level,
> struct virtio_gpu_box *box,
> + struct virtio_gpu_object_array *objs,
> struct virtio_gpu_fence *fence)
> {
> + struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
> struct virtio_gpu_transfer_host_3d *cmd_p;
> struct virtio_gpu_vbuffer *vbuf;
>
> cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> memset(cmd_p, 0, sizeof(*cmd_p));
>
> + vbuf->objs = objs;
> +
> cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D);
> cmd_p->hdr.ctx_id = cpu_to_le32(ctx_id);
> - cmd_p->resource_id = cpu_to_le32(resource_id);
> + cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
> cmd_p->box = *box;
> cmd_p->offset = cpu_to_le64(offset);
> cmd_p->level = cpu_to_le32(level);
> --
> 2.18.1
>
On Wed, Jul 03, 2019 at 01:05:12PM -0700, Chia-I Wu wrote:
> On Tue, Jul 2, 2019 at 7:19 AM Gerd Hoffmann <[email protected]> wrote:
> >
> > Switch to the virtio_gpu_array_* helper workflow.
> (just repeating my question on patch 6)
>
> Does this fix the obj refcount issue? When was the issue introduced?
obj refcount should be fine in both old and new code.
old code:
drm_gem_object_lookup
drm_gem_object_put_unlocked
new code:
virtio_gpu_array_from_handles
virtio_gpu_array_put_free (in virtio_gpu_dequeue_ctrl_func).
Or did I miss something?
cheers,
Gerd
On Thu, Jul 4, 2019 at 4:48 AM Gerd Hoffmann <[email protected]> wrote:
>
> On Wed, Jul 03, 2019 at 01:05:12PM -0700, Chia-I Wu wrote:
> > On Tue, Jul 2, 2019 at 7:19 AM Gerd Hoffmann <[email protected]> wrote:
> > >
> > > Switch to the virtio_gpu_array_* helper workflow.
> > (just repeating my question on patch 6)
> >
> > Does this fix the obj refcount issue? When was the issue introduced?
>
> obj refcount should be fine in both old and new code.
>
> old code:
> drm_gem_object_lookup
> drm_gem_object_put_unlocked
>
> new code:
> virtio_gpu_array_from_handles
> virtio_gpu_array_put_free (in virtio_gpu_dequeue_ctrl_func).
>
> Or did I miss something?
In the old code, drm_gem_object_put_unlocked is called before the vbuf
using the object is retired. Isn't that what object array wants to
fix?
We get away with that because the host only sees hw_res_handles, and
executes the commands in order.
Maybe it was me who missed something..?
>
> cheers,
> Gerd
>
On Thu, Jul 04, 2019 at 11:55:59AM -0700, Chia-I Wu wrote:
> On Thu, Jul 4, 2019 at 4:48 AM Gerd Hoffmann <[email protected]> wrote:
> >
> > On Wed, Jul 03, 2019 at 01:05:12PM -0700, Chia-I Wu wrote:
> > > On Tue, Jul 2, 2019 at 7:19 AM Gerd Hoffmann <[email protected]> wrote:
> > > >
> > > > Switch to the virtio_gpu_array_* helper workflow.
> > > (just repeating my question on patch 6)
> > >
> > > Does this fix the obj refcount issue? When was the issue introduced?
> >
> > obj refcount should be fine in both old and new code.
> >
> > old code:
> > drm_gem_object_lookup
> > drm_gem_object_put_unlocked
> >
> > new code:
> > virtio_gpu_array_from_handles
> > virtio_gpu_array_put_free (in virtio_gpu_dequeue_ctrl_func).
> >
> > Or did I miss something?
> In the old code, drm_gem_object_put_unlocked is called before the vbuf
> using the object is retired. Isn't that what object array wants to
> fix?
I think the fence keeps the bo alive.
cheers,
Gerd