Re: [QEMU PATCH v4 12/13] virtio-gpu: Initialize Venus

[Antonio Caggiano <quic_acaggian@xxxxxxxxxxx>]

Hi Huang,

Thank you for pushing this forward!

On 31/08/2023 11:32, Huang Rui wrote:
> From: Antonio Caggiano <antonio.caggiano@xxxxxxxxxxxxx>
>
> Request Venus when initializing VirGL.
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@xxxxxxxxxxxxx>
> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> ---
>
> 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


>       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