[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [QEMU PATCH v4 12/13] virtio-gpu: Initialize Venus
From: |
Huang Rui |
Subject: |
Re: [QEMU PATCH v4 12/13] virtio-gpu: Initialize Venus |
Date: |
Sat, 9 Sep 2023 18:52:27 +0800 |
On Thu, Aug 31, 2023 at 06:40:11PM +0800, Antonio Caggiano wrote:
> Hi Huang,
>
> Thank you for pushing this forward!
>
My pleasure! :-)
> On 31/08/2023 11:32, Huang Rui wrote:
> > From: Antonio Caggiano <antonio.caggiano@collabora.com>
> >
> > Request Venus when initializing VirGL.
> >
> > Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >
> > v1->v2:
> > - Rebase to latest version
> >
> > hw/display/virtio-gpu-virgl.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> > index 83cd8c8fd0..c5a62665bd 100644
> > --- a/hw/display/virtio-gpu-virgl.c
> > +++ b/hw/display/virtio-gpu-virgl.c
> > @@ -887,6 +887,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> > }
> > #endif
> >
> > + flags |= VIRGL_RENDERER_VENUS;
> > +
>
> VIRGL_RENDERER_VENUS is a symbol only available from virglrenderer 0.9.1
> [0] and only if VIRGL_RENDERER_UNSTABLE_APIS is defined.
>
> Luckily for us, VIRGL_RENDERER_UNSTABLE_APIS is defined unconditionally
> from virglrenderer 0.9.0 [1], so we could check for that in qemu/meson.build
>
> e.g.
>
>
> if virgl.version().version_compare('>= 0.9.0')
> message('Enabling virglrenderer unstable APIs')
> virgl = declare_dependency(compile_args:
> '-DVIRGL_RENDERER_UNSTABLE_APIS',
> dependencies: virgl)
> endif
>
>
> Also, while testing this with various versions of virglrenderer, I
> realized there are no guarantees for Venus backend to be available in
> the linked library. Virglrenderer should be built with
> -Dvenus_experimental=true, and if that is not the case, the following
> virgl_renderer_init would fail for previous versions of virglrenderer or
> in case it has not been built with venus support.
>
> I would suggest another approach for that which tries initializing Venus
> only if VIRGL_RENDERER_VENUS is actually defined. Then, if it fails
> cause virglrenderer has not been built with venus support, try again
> falling back to virgl only.
>
> e.g.
>
> #ifdef VIRGL_RENDERER_VENUS
> ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
> if (ret != 0) {
> warn_report("Failed to initialize virglrenderer with venus:
> %d", ret);
> warn_report("Falling back to virgl only");
> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs);
> }
> #else
> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs);
> #endif
>
Thanks a lot to explain so clearly. Yes, it's reasonable for me. I will
modify it in the next version. And agree, we should take care of different
virglrenderer versions.
Thanks,
Ray
>
> > ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
> > if (ret != 0) {
> > error_report("virgl could not be initialized: %d", ret);
>
> [0]
> https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/6c31f85330bb4c5aba8b82eba606971e598c6e25
> [1]
> https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/491afdc42a49ec6a1b8d7cbc5c97360229002d41
>
> Best regards,
> Antonio Caggiano
- Re: [QEMU PATCH v4 12/13] virtio-gpu: Initialize Venus,
Huang Rui <=