On Mon, May 19, 2025 at 6:12 AM Dmitry Osipenko
<dmitry.osipe...@collabora.com> wrote:
>
> On 5/18/25 18:26, Yiwei Zhang wrote:
> > Venus and later native contexts have their own fence context along with
> > multiple timelines within. Fences wtih VIRTIO_GPU_FLAG_INFO_RING_IDX in
> > the flags must be dispatched to be created on the target context. Fence
> > signaling also has to be handled on the specific timeline within that
> > target context.
> >
> > Before this change, venus fencing is completely broken if the host
> > driver doesn't support implicit fencing with external memory objects.
> > Frames can go backwards along with random artifacts on screen if the
> > host driver doesn't attach an implicit fence to the render target. The
> > symptom could be hidden by certain guest wsi backend that waits on a
> > venus native VkFence object for the actual payload with limited present
> > modes or under special configs. e.g. x11 mailbox or xwayland.
> >
> > After this change, everything related to venus fencing starts making
> > sense. Confirmed this via guest and host side perfetto tracing.
> >
> > Changes since v1:
> > - Minor commit msg updates based on feedbacks from BALATON
> >
> > Cc: qemu-sta...@nongnu.org
> > Fixes: 94d0ea1c1928 ("virtio-gpu: Support Venus context")
> > Signed-off-by: Yiwei Zhang <zzyi...@gmail.com>
> > ---
> > hw/display/virtio-gpu-virgl.c | 44 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> > index 145a0b3879..94ddc01f91 100644
> > --- a/hw/display/virtio-gpu-virgl.c
> > +++ b/hw/display/virtio-gpu-virgl.c
> > @@ -970,6 +970,15 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> > }
> >
> > trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> > +#if VIRGL_VERSION_MAJOR >= 1
> > + if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> > + virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id,
> > +
> > VIRGL_RENDERER_FENCE_FLAG_MERGEABLE,
> > + cmd->cmd_hdr.ring_idx,
> > + cmd->cmd_hdr.fence_id);
> > + return;
> > + }
> > +#endif
> > virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
> > }
> >
> > @@ -983,6 +992,11 @@ static void virgl_write_fence(void *opaque, uint32_t
> > fence)
> > * the guest can end up emitting fences out of order
> > * so we should check all fenced cmds not just the first one.
> > */
> > +#if VIRGL_VERSION_MAJOR >= 1
> > + if (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX) {
> > + continue;
> > + }
> > +#endif
>
> Is it possible that virglrenderer will ever write a context fence in
> virgl_renderer_create_fence()? Do we really need this check?
I assume you were referring to whether a context fence can be written
in virgl_write_fence(). Yes, the fenceq contains both the global
fences and context specific fences. Without the check,
virgl_write_fence() will signal any context fence with a fence id
smaller than the latest signaled virgl global fence.
>
> --
> Best regards,
> Dmitry
