Re: [PATCH v7 5/9] gfxstream + rutabaga prep: added need defintions, fields, and options

[Mark Cave-Ayland]

On 17/08/2023 03:23, Gurchetan Singh wrote:

From: Gurchetan Singh <gurchetansi...@chromium.org>
This modifies the common virtio-gpu.h file have the fields and
defintions needed by gfxstream/rutabaga, by VirtioGpuRutabaga.

Signed-off-by: Gurchetan Singh <gurchetansi...@chromium.org>
Tested-by: Alyssa Ross <h...@alyssa.is>
Tested-by: Emmanouil Pitsidianakis <manos.pitsidiana...@linaro.org>
Reviewed-by: Emmanouil Pitsidianakis <manos.pitsidiana...@linaro.org>
---
v1: void *rutabaga --> struct rutabaga *rutabaga (Akihiko)
     have a separate rutabaga device instead of using GL device (Bernard)

v2: VirtioGpuRutabaga --> VirtIOGPURutabaga (Akihiko)
     move MemoryRegionInfo into VirtIOGPURutabaga (Akihiko)
     remove 'ctx' field (Akihiko)
     remove 'rutabaga_active'

v6: remove command from commit message, refer to docs instead (Manos)

  include/hw/virtio/virtio-gpu.h | 28 ++++++++++++++++++++++++++++
  1 file changed, 28 insertions(+)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 55973e112f..e2a07e68d9 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -38,6 +38,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPUGL, VIRTIO_GPU_GL)
  #define TYPE_VHOST_USER_GPU "vhost-user-gpu"
  OBJECT_DECLARE_SIMPLE_TYPE(VhostUserGPU, VHOST_USER_GPU)
  
+#define TYPE_VIRTIO_GPU_RUTABAGA "virtio-gpu-rutabaga-device"
+OBJECT_DECLARE_SIMPLE_TYPE(VirtIOGPURutabaga, VIRTIO_GPU_RUTABAGA)
+
  struct virtio_gpu_simple_resource {
      uint32_t resource_id;
      uint32_t width;
@@ -94,6 +97,7 @@ enum virtio_gpu_base_conf_flags {
      VIRTIO_GPU_FLAG_DMABUF_ENABLED,
      VIRTIO_GPU_FLAG_BLOB_ENABLED,
      VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
+    VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
  };
  
  #define virtio_gpu_virgl_enabled(_cfg) \
@@ -108,6 +112,8 @@ enum virtio_gpu_base_conf_flags {
      (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
  #define virtio_gpu_context_init_enabled(_cfg) \
      (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
+#define virtio_gpu_rutabaga_enabled(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
  #define virtio_gpu_hostmem_enabled(_cfg) \
      (_cfg.hostmem > 0)
  
@@ -232,6 +238,28 @@ struct VhostUserGPU {
      bool backend_blocked;
  };
  
+#define MAX_SLOTS 4096
+
+struct MemoryRegionInfo {
+    int used;
+    MemoryRegion mr;
+    uint32_t resource_id;
+};
+
+struct rutabaga;
+
+struct VirtIOGPURutabaga {
+    struct VirtIOGPU parent_obj;

The QOM macro should define a typedef for you, so you can drop the "struct" 
here.

+
+    struct MemoryRegionInfo memory_regions[MAX_SLOTS];
+    char *capset_names;
+    char *wayland_socket_path;
+    char *wsi;
+    bool headless;
+    uint32_t num_capsets;
+    struct rutabaga *rutabaga;
+};
+

Shouldn't the VIRTIO_GPU_RUTABAGA QOM declaration and this structure be in a separate 
virtio-gpu-rutabaga header file which also includes the header defining struct 
rutabaga? The fact that you're having to pre-declare struct rutabaga in this header 
when rutabaga support is an optional dependency doesn't seem right.

  #define VIRTIO_GPU_FILL_CMD(out) do {                                   \
          size_t s;                                                       \
          s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0,          \


ATB,

Mark.