On 10/22/24 07:44, Kasireddy, Vivek wrote: >> virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, >> x, y, >> - width, height, objs, vgfb->fence); >> + width, height, objs, >> + vgplane_st->fence); >> virtio_gpu_notify(vgdev); >> - >> - dma_fence_wait_timeout(&vgfb->fence->f, true, >> + dma_fence_wait_timeout(&vgplane_st->fence->f, true, >> msecs_to_jiffies(50)); >> - dma_fence_put(&vgfb->fence->f); >> - vgfb->fence = NULL; > Not sure if it makes any difference but would there be a problem if you unref > the fence here (existing behavior) instead of deferring it until cleanup?
Previously fence was a part of virtio-gpu framebuffer, which was kind of a hack. Maybe there was no better option back then, when this code was written initially. Now fence is a part of plane's atomic state, like it should be. We shouldn't change atomic state at the commit time. ... >> @@ -326,11 +348,9 @@ static void virtio_gpu_cursor_plane_update(struct >> drm_plane *plane, >> (vgdev, 0, >> plane->state->crtc_w, >> plane->state->crtc_h, >> - 0, 0, objs, vgfb->fence); >> + 0, 0, objs, vgplane_st->fence); >> virtio_gpu_notify(vgdev); >> - dma_fence_wait(&vgfb->fence->f, true); >> - dma_fence_put(&vgfb->fence->f); >> - vgfb->fence = NULL; > Same comment as above. > Regardless, the patch LGTM. > > Acked-by: Vivek Kasireddy <vivek.kasire...@intel.com> Thanks for the review :) -- Best regards, Dmitry
