Re: [PATCH v5 4/8] virtio-gpu: Support asynchronous fencing

[Dmitry Osipenko]

On 1/20/25 16:56, Alex Bennée wrote:
...
>> @@ -972,15 +973,29 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>>  
>>      trace_virtio_gpu_fence_ctrl(cmd->cmd_hdr.fence_id, cmd->cmd_hdr.type);
>>  
>> -    /*
>> -     * Unlike other virglrenderer functions, this one returns a positive
>> -     * error code.
>> -     */
>> -    ret = virgl_renderer_create_fence(cmd->cmd_hdr.fence_id, 0);
>> -    if (ret) {
>> -        qemu_log_mask(LOG_GUEST_ERROR,
>> -                      "%s: virgl_renderer_create_fence error: %s",
>> -                      __func__, strerror(ret));
>> +    if (gl->context_fence_enabled &&
>> +        (cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_INFO_RING_IDX)) {
>> +        uint32_t flags = 0;
> 
> Is this is constant or do we expect to change this later?
There are no immediate plans for using this flags variable in QEMU
today. But in general context-specific flags could be specified here.
Crosv makes use of the flags.

>> +        ret = virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id, 
>> flags,
>> +                                                  cmd->cmd_hdr.ring_idx,
>> +                                                  cmd->cmd_hdr.fence_id);
>> +        if (ret) {
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                          "%s: virgl_renderer_context_create_fence error: 
>> %s",
>> +                          __func__, strerror(-ret));
> 
> This still fails with older virglrenderers:
> 
> ../../hw/display/virtio-gpu-virgl.c: In function 
> ‘virtio_gpu_virgl_process_cmd’:
> ../../hw/display/virtio-gpu-virgl.c:980:15: error: implicit declaration of 
> function ‘virgl_renderer_context_create_fence’; did you mean 
> ‘virgl_renderer_context_create’? [-Werror=implicit-function-declaration]
>   980 |         ret = 
> virgl_renderer_context_create_fence(cmd->cmd_hdr.ctx_id, flags,
>       |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       |               virgl_renderer_context_create
> ../../hw/display/virtio-gpu-virgl.c:980:15: error: nested extern declaration 
> of ‘virgl_renderer_context_create_fence’ [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> [1981/2819] Compiling C object libcommon.a.p/ui_sdl2-gl.c.o

Indeed! Good catch again, thanks! Will fix in v6

-- 
Best regards,
Dmitry