Re: [RFC PATCH 07/13] gfxstream + rutabaga prep: define callbacks in realize function

[Philippe Mathieu-Daudé]

On 21/4/23 03:12, Gurchetan Singh wrote:
> This reduces the amount of renderer backend specific needed to
> be exposed to the GL device.  We only need one realize function
> per renderer backend.
>
> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> ---
>   hw/display/virtio-gpu-gl.c     | 17 +++++++++++------
>   hw/display/virtio-gpu-virgl.c  | 35 ++++++++++++++++++++++++----------
>   include/hw/virtio/virtio-gpu.h |  7 -------
>   3 files changed, 36 insertions(+), 23 deletions(-)


> @@ -34,13 +39,13 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
> void *data)
>       VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_CLASS(klass);
>       VirtIOGPUClass *vgc = VIRTIO_GPU_CLASS(klass);
>   
> -    vbc->gl_flushed = virtio_gpu_virgl_flushed;
> -    vgc->handle_ctrl = virtio_gpu_virgl_handle_ctrl;
> -    vgc->process_cmd = virtio_gpu_virgl_process_cmd;
> -    vgc->update_cursor_data = virtio_gpu_virgl_update_cursor;
> +    vbc->gl_flushed = NULL;
> +    vgc->handle_ctrl = NULL;
> +    vgc->process_cmd = NULL;
> +    vgc->update_cursor_data = NULL;

If we don't check pointers are unset before assigning it in
virtio_gpu_virgl_device_realize(), then NULL-initializing here
is not really helpful.

> -    vdc->realize = virtio_gpu_virgl_device_realize;
> -    vdc->reset = virtio_gpu_virgl_reset;
> +    vdc->realize = virtio_gpu_gl_device_realize;
> +    vdc->reset = NULL;
>       device_class_set_props(dc, virtio_gpu_gl_properties);
>   }


> @@ -737,9 +752,9 @@ void virtio_gpu_virgl_device_realize(DeviceState *qdev, 
> Error **errp)
>           return;
>       }
>   
> -    g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
> -    VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
> -        virtio_gpu_virgl_get_num_capsets(g);
> +    gpudev->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);

Or:

       VIRTIO_DEVICE(gpudev)->conf.flags |= (1 << 
VIRTIO_GPU_FLAG_VIRGL_ENABLED);

> +    VIRTIO_GPU_BASE(gpudev)->virtio_config.num_capsets =
> +        virtio_gpu_virgl_get_num_capsets(gpudev);
>   
>       virtio_gpu_device_realize(qdev, errp);
>   }

Removing the NULL-inits:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

PD: I'd use "hw/display/virtio-gpu-virgl" patch prefix instead
of "gfxstream + rutabaga prep". You only start mentioning rutabaga
in the next patch. And the patches up to here could be applied
as virtio-gpu cleanup patches.