[PATCH 3/8] ACRN:DM:VGPU: Add virtio_gpu_scanout structure to handle virtio-gpu-cmds correctly


Zhao, Yakui
 

Now it only supports one scanout panel for virtio-gpu. So the scanout_id is ignored in
course of handling virtio-gpu-cmd. In order to handle the virtio-gpu-cmds correctly, it
adds the virtio_gpu_scanout structure so that it can record the scanout info.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
devicemodel/hw/pci/virtio/virtio_gpu.c | 63 ++++++++++++++++++++++++++
1 file changed, 63 insertions(+)

diff --git a/devicemodel/hw/pci/virtio/virtio_gpu.c b/devicemodel/hw/pci/virtio/virtio_gpu.c
index cf3d20823..768e188d2 100644
--- a/devicemodel/hw/pci/virtio/virtio_gpu.c
+++ b/devicemodel/hw/pci/virtio/virtio_gpu.c
@@ -348,6 +348,16 @@ enum vga_thread_status {
VGA_THREAD_EOL = 0,
VGA_THREAD_RUNNING
};
+
+struct virtio_gpu_scanout {
+ int scanout_id;
+ uint32_t resource_id;
+ struct virtio_gpu_rect scanout_rect;
+ pixman_image_t *cur_img;
+ struct dma_buf_info *dma_buf;
+ bool active;
+};
+
/*
* Per-device struct
*/
@@ -366,6 +376,8 @@ struct virtio_gpu {
int32_t vga_thread_status;
uint8_t edid[VIRTIO_GPU_EDID_SIZE];
bool is_blob_supported;
+ int scanout_num;
+ struct virtio_gpu_scanout *gpu_scanouts;
};

struct virtio_gpu_command {
@@ -782,12 +794,22 @@ virtio_gpu_cmd_set_scanout(struct virtio_gpu_command *cmd)
struct virtio_gpu_ctrl_hdr resp;
struct surface surf;
struct virtio_gpu *gpu;
+ struct virtio_gpu_scanout *gpu_scanout;

gpu = cmd->gpu;
memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
memset(&resp, 0, sizeof(resp));
virtio_gpu_update_resp_fence(&cmd->hdr, &resp);

+ if (req.scanout_id >= gpu->scanout_num) {
+ pr_err("%s: Incorrect scanout_id %d\n", req.scanout_id);
+ resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+ memcpy(cmd->iov[1].iov_base, &resp, sizeof(resp));
+ return;
+ }
+ gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+ gpu_scanout->scanout_id = req.scanout_id;
+
r2d = virtio_gpu_find_resource_2d(gpu, req.resource_id);
if ((req.resource_id == 0) || (r2d == NULL)) {
vdpy_surface_set(gpu->vdpy_handle, 0, NULL);
@@ -1130,6 +1152,7 @@ virtio_gpu_cmd_set_scanout_blob(struct virtio_gpu_command *cmd)
struct surface surf;
uint32_t drm_fourcc;
struct virtio_gpu *gpu;
+ struct virtio_gpu_scanout *gpu_scanout;

gpu = cmd->gpu;
memset(&surf, 0, sizeof(surf));
@@ -1140,6 +1163,14 @@ virtio_gpu_cmd_set_scanout_blob(struct virtio_gpu_command *cmd)
if (cmd->gpu->vga.enable) {
cmd->gpu->vga.enable = false;
}
+ if (req.scanout_id >= gpu->scanout_num) {
+ pr_err("%s: Incorrect scanout_id %d\n", req.scanout_id);
+ resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+ memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp, sizeof(resp));
+ return;
+ }
+ gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+ gpu_scanout->scanout_id = req.scanout_id;
if (req.resource_id == 0) {
resp.type = VIRTIO_GPU_RESP_OK_NODATA;
memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp, sizeof(resp));
@@ -1485,10 +1516,22 @@ virtio_gpu_init(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
gpu->vq,
BACKEND_VBSU);

+ gpu->scanout_num = 1;
gpu->vdpy_handle = vdpy_init(NULL);
gpu->base.mtx = &gpu->mtx;
gpu->base.device_caps = VIRTIO_GPU_S_HOSTCAPS;

+ if (gpu->scanout_num < 0) {
+ pr_err("%s: return incorrect scanout num %d\n", gpu->scanout_num);
+ return -1;
+ }
+ gpu->gpu_scanouts = calloc(gpu->scanout_num, sizeof(struct virtio_gpu_scanout));
+ if (gpu->gpu_scanouts == NULL) {
+ pr_err("%s: out of memory for gpu_scanouts\n", __func__);
+ free(gpu);
+ return -1;
+ }
+
if (vm_allow_dmabuf(gpu->base.dev->vmctx)) {
FILE *fp;
char buf[16];
@@ -1643,6 +1686,7 @@ virtio_gpu_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
{
struct virtio_gpu *gpu;
struct virtio_gpu_resource_2d *r2d;
+ int i;

gpu = (struct virtio_gpu *)dev->arg;
gpu->vga.enable = false;
@@ -1661,6 +1705,25 @@ virtio_gpu_deinit(struct vmctx *ctx, struct pci_vdev *dev, char *opts)
gpu->vga.gc = NULL;
}

+ for (i=0; i < gpu->scanout_num; i++) {
+ struct virtio_gpu_scanout *gpu_scanout;
+
+ gpu_scanout = gpu->gpu_scanouts + i;
+ if (gpu_scanout && gpu_scanout->active) {
+ if (gpu_scanout->cur_img) {
+ pixman_image_unref(gpu_scanout->cur_img);
+ gpu_scanout->cur_img = NULL;
+ }
+ if (gpu_scanout->dma_buf) {
+ virtio_gpu_dmabuf_unref(gpu_scanout->dma_buf);
+ gpu_scanout->dma_buf = NULL;
+ }
+ gpu_scanout->active = false;
+ }
+ }
+ free(gpu->gpu_scanouts);
+ gpu->gpu_scanouts = NULL;
+
pthread_mutex_destroy(&gpu->vga_thread_mtx);
while (LIST_FIRST(&gpu->r2d_list)) {
r2d = LIST_FIRST(&gpu->r2d_list);
--
2.25.1


Sun, Peng
 

On Mon, 2022-08-08 at 14:13 +0800, Zhao, Yakui wrote:
Now it only supports one scanout panel for virtio-gpu. So the
[Sun, Peng] Now it only supports one scanout for virtio-gpu.
scanout_id is ignored in
course of handling virtio-gpu-cmd. In order to handle the
[Sun, Peng] course of handling virtio-gpu commands.
virtio-gpu-cmds correctly, it
adds the virtio_gpu_scanout structure so that it can record the
scanout info.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
 devicemodel/hw/pci/virtio/virtio_gpu.c | 63
++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/devicemodel/hw/pci/virtio/virtio_gpu.c
b/devicemodel/hw/pci/virtio/virtio_gpu.c
index cf3d20823..768e188d2 100644
--- a/devicemodel/hw/pci/virtio/virtio_gpu.c
+++ b/devicemodel/hw/pci/virtio/virtio_gpu.c
@@ -348,6 +348,16 @@ enum vga_thread_status {
        VGA_THREAD_EOL = 0,
        VGA_THREAD_RUNNING
 };
+
+struct virtio_gpu_scanout {
+       int scanout_id;
+       uint32_t resource_id;
+       struct virtio_gpu_rect scanout_rect;
+       pixman_image_t *cur_img;
[Sun, Peng] What is the mean of cur_img? Cursor or Current?
+       struct dma_buf_info *dma_buf;
+       bool active;
[Sun, Peng] I think is_active is better.
+};
+
 /*
  * Per-device struct
  */
@@ -366,6 +376,8 @@ struct virtio_gpu {
        int32_t vga_thread_status;
        uint8_t edid[VIRTIO_GPU_EDID_SIZE];
        bool is_blob_supported;
+       int scanout_num;
+       struct virtio_gpu_scanout *gpu_scanouts;
 };
 
 struct virtio_gpu_command {
@@ -782,12 +794,22 @@ virtio_gpu_cmd_set_scanout(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        struct surface surf;
        struct virtio_gpu *gpu;
+       struct virtio_gpu_scanout *gpu_scanout;
 
        gpu = cmd->gpu;
        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        memset(&resp, 0, sizeof(resp));
        virtio_gpu_update_resp_fence(&cmd->hdr, &resp);
 
+       if (req.scanout_id >= gpu->scanout_num) {
+               pr_err("%s: Incorrect scanout_id %d\n",
req.scanout_id);
[Sun, Peng] pr_err("%s: Invalid scanout_id
+               resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+               memcpy(cmd->iov[1].iov_base, &resp, sizeof(resp));
+               return;
+       }
+       gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+       gpu_scanout->scanout_id = req.scanout_id;
+
        r2d = virtio_gpu_find_resource_2d(gpu, req.resource_id);
        if ((req.resource_id == 0) || (r2d == NULL)) {
                vdpy_surface_set(gpu->vdpy_handle, 0, NULL);
@@ -1130,6 +1152,7 @@ virtio_gpu_cmd_set_scanout_blob(struct
virtio_gpu_command *cmd)
        struct surface surf;
        uint32_t drm_fourcc;
        struct virtio_gpu *gpu;
+       struct virtio_gpu_scanout *gpu_scanout;
 
        gpu = cmd->gpu;
        memset(&surf, 0, sizeof(surf));
@@ -1140,6 +1163,14 @@ virtio_gpu_cmd_set_scanout_blob(struct
virtio_gpu_command *cmd)
        if (cmd->gpu->vga.enable) {
                cmd->gpu->vga.enable = false;
        }
+       if (req.scanout_id >= gpu->scanout_num) {
+               pr_err("%s: Incorrect scanout_id %d\n",
req.scanout_id);
[Sun, Peng] Also "Invalid scanout_id" here.
+               resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
+       gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+       gpu_scanout->scanout_id = req.scanout_id;
        if (req.resource_id == 0) {
                resp.type = VIRTIO_GPU_RESP_OK_NODATA;
                memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
@@ -1485,10 +1516,22 @@ virtio_gpu_init(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
                        gpu->vq,
                        BACKEND_VBSU);
 
+       gpu->scanout_num = 1;
        gpu->vdpy_handle = vdpy_init(NULL);
[Sun, Peng] I think this patch shouldn't be 3/8 patch. Because in 2/8
patch, vdpy_init() should return the supported number of vscreens.
        gpu->base.mtx = &gpu->mtx;
        gpu->base.device_caps = VIRTIO_GPU_S_HOSTCAPS;
 
+       if (gpu->scanout_num < 0) {
+               pr_err("%s: return incorrect scanout num %d\n", gpu-
scanout_num);
[Sun, Peng] pr_err("%s: Invalid scanout num %d, never should be
here!\n",
gpu->scanout_num);
+               return -1;
+       }
+       gpu->gpu_scanouts = calloc(gpu->scanout_num, sizeof(struct
virtio_gpu_scanout));
+       if (gpu->gpu_scanouts == NULL) {
+               pr_err("%s: out of memory for gpu_scanouts\n",
__func__);
+               free(gpu);
+               return -1;
+       }
+
        if (vm_allow_dmabuf(gpu->base.dev->vmctx)) {
                FILE *fp;
                char buf[16];
@@ -1643,6 +1686,7 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
 {
        struct virtio_gpu *gpu;
        struct virtio_gpu_resource_2d *r2d;
+       int i;
 
        gpu = (struct virtio_gpu *)dev->arg;
        gpu->vga.enable = false;
@@ -1661,6 +1705,25 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
                gpu->vga.gc = NULL;
        }
 
+       for (i=0; i < gpu->scanout_num; i++) {
+               struct virtio_gpu_scanout *gpu_scanout;
+
+               gpu_scanout = gpu->gpu_scanouts + i;
+               if (gpu_scanout && gpu_scanout->active) {
+                       if (gpu_scanout->cur_img) {
+                               pixman_image_unref(gpu_scanout-
cur_img);
+                               gpu_scanout->cur_img = NULL;
+                       }
+                       if (gpu_scanout->dma_buf) {
+                               virtio_gpu_dmabuf_unref(gpu_scanout-
dma_buf);
+                               gpu_scanout->dma_buf = NULL;
+                       }
+                       gpu_scanout->active = false;
+               }
+       }
+       free(gpu->gpu_scanouts);
+       gpu->gpu_scanouts = NULL;
+
        pthread_mutex_destroy(&gpu->vga_thread_mtx);
        while (LIST_FIRST(&gpu->r2d_list)) {
                r2d = LIST_FIRST(&gpu->r2d_list);
--
Sun Peng <peng.p.sun@...>
SSE/ACRN Upstream


Zhao, Yakui
 

On 2022/8/8 14:52, Sun Peng wrote:
On Mon, 2022-08-08 at 14:13 +0800, Zhao, Yakui wrote:
Now it only supports one scanout panel for virtio-gpu. So the
[Sun, Peng] Now it only supports one scanout for virtio-gpu.
scanout_id is ignored in
course of handling virtio-gpu-cmd. In order to handle the
[Sun, Peng] course of handling virtio-gpu commands.
virtio-gpu-cmds correctly, it
adds the virtio_gpu_scanout structure so that it can record the
scanout info.
This will be updated in next version.


Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
 devicemodel/hw/pci/virtio/virtio_gpu.c | 63
++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/devicemodel/hw/pci/virtio/virtio_gpu.c
b/devicemodel/hw/pci/virtio/virtio_gpu.c
index cf3d20823..768e188d2 100644
--- a/devicemodel/hw/pci/virtio/virtio_gpu.c
+++ b/devicemodel/hw/pci/virtio/virtio_gpu.c
@@ -348,6 +348,16 @@ enum vga_thread_status {
        VGA_THREAD_EOL = 0,
        VGA_THREAD_RUNNING
 };
+
+struct virtio_gpu_scanout {
+       int scanout_id;
+       uint32_t resource_id;
+       struct virtio_gpu_rect scanout_rect;
+       pixman_image_t *cur_img;
[Sun, Peng] What is the mean of cur_img? Cursor or Current?
It is current. This indicates the image used by the scanout when
it uses the VIRTIO_GPU_SET_SCANOUT cmd.

+       struct dma_buf_info *dma_buf;
+       bool active;
[Sun, Peng] I think is_active is better.
Sure. It will be changed in next version.

+};
+
 /*
  * Per-device struct
  */
@@ -366,6 +376,8 @@ struct virtio_gpu {
        int32_t vga_thread_status;
        uint8_t edid[VIRTIO_GPU_EDID_SIZE];
        bool is_blob_supported;
+       int scanout_num;
+       struct virtio_gpu_scanout *gpu_scanouts;
 };
 struct virtio_gpu_command {
@@ -782,12 +794,22 @@ virtio_gpu_cmd_set_scanout(struct
virtio_gpu_command *cmd)
        struct virtio_gpu_ctrl_hdr resp;
        struct surface surf;
        struct virtio_gpu *gpu;
+       struct virtio_gpu_scanout *gpu_scanout;
        gpu = cmd->gpu;
        memcpy(&req, cmd->iov[0].iov_base, sizeof(req));
        memset(&resp, 0, sizeof(resp));
        virtio_gpu_update_resp_fence(&cmd->hdr, &resp);
+       if (req.scanout_id >= gpu->scanout_num) {
+               pr_err("%s: Incorrect scanout_id %d\n",
req.scanout_id);
[Sun, Peng] pr_err("%s: Invalid scanout_id
OK. It will be updated in next version.

+               resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+               memcpy(cmd->iov[1].iov_base, &resp, sizeof(resp));
+               return;
+       }
+       gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+       gpu_scanout->scanout_id = req.scanout_id;
+
        r2d = virtio_gpu_find_resource_2d(gpu, req.resource_id);
        if ((req.resource_id == 0) || (r2d == NULL)) {
                vdpy_surface_set(gpu->vdpy_handle, 0, NULL);
@@ -1130,6 +1152,7 @@ virtio_gpu_cmd_set_scanout_blob(struct
virtio_gpu_command *cmd)
        struct surface surf;
        uint32_t drm_fourcc;
        struct virtio_gpu *gpu;
+       struct virtio_gpu_scanout *gpu_scanout;
        gpu = cmd->gpu;
        memset(&surf, 0, sizeof(surf));
@@ -1140,6 +1163,14 @@ virtio_gpu_cmd_set_scanout_blob(struct
virtio_gpu_command *cmd)
        if (cmd->gpu->vga.enable) {
                cmd->gpu->vga.enable = false;
        }
+       if (req.scanout_id >= gpu->scanout_num) {
+               pr_err("%s: Incorrect scanout_id %d\n",
req.scanout_id);
[Sun, Peng] Also "Invalid scanout_id" here.
+               resp.type = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+               memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
+               return;
+       }
+       gpu_scanout = gpu->gpu_scanouts + req.scanout_id;
+       gpu_scanout->scanout_id = req.scanout_id;
        if (req.resource_id == 0) {
                resp.type = VIRTIO_GPU_RESP_OK_NODATA;
                memcpy(cmd->iov[cmd->iovcnt - 1].iov_base, &resp,
sizeof(resp));
@@ -1485,10 +1516,22 @@ virtio_gpu_init(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
                        gpu->vq,
                        BACKEND_VBSU);
+       gpu->scanout_num = 1;
        gpu->vdpy_handle = vdpy_init(NULL);
[Sun, Peng] I think this patch shouldn't be 3/8 patch. Because in 2/8
patch, vdpy_init() should return the supported number of vscreens.
The patch 2/8 still returns the fixed number of vscreen.
Currently it is still 1.
Before the virtio-gpu BE doesn't handle the scanout_info correctly, it is still assumed that only one scanout is supported for guest.
Only after the virtio-gpu BE can handle the scanout_info correctly, it will return the supported num of vscreens.

        gpu->base.mtx = &gpu->mtx;
        gpu->base.device_caps = VIRTIO_GPU_S_HOSTCAPS;
+       if (gpu->scanout_num < 0) {
+               pr_err("%s: return incorrect scanout num %d\n", gpu-
scanout_num);
[Sun, Peng] pr_err("%s: Invalid scanout num %d, never should be
here!\n",
gpu->scanout_num);
+               return -1;
+       }
+       gpu->gpu_scanouts = calloc(gpu->scanout_num, sizeof(struct
virtio_gpu_scanout));
+       if (gpu->gpu_scanouts == NULL) {
+               pr_err("%s: out of memory for gpu_scanouts\n",
__func__);
+               free(gpu);
+               return -1;
+       }
+
        if (vm_allow_dmabuf(gpu->base.dev->vmctx)) {
                FILE *fp;
                char buf[16];
@@ -1643,6 +1686,7 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
 {
        struct virtio_gpu *gpu;
        struct virtio_gpu_resource_2d *r2d;
+       int i;
        gpu = (struct virtio_gpu *)dev->arg;
        gpu->vga.enable = false;
@@ -1661,6 +1705,25 @@ virtio_gpu_deinit(struct vmctx *ctx, struct
pci_vdev *dev, char *opts)
                gpu->vga.gc = NULL;
        }
+       for (i=0; i < gpu->scanout_num; i++) {
+               struct virtio_gpu_scanout *gpu_scanout;
+
+               gpu_scanout = gpu->gpu_scanouts + i;
+               if (gpu_scanout && gpu_scanout->active) {
+                       if (gpu_scanout->cur_img) {
+                               pixman_image_unref(gpu_scanout-
cur_img);
+                               gpu_scanout->cur_img = NULL;
+                       }
+                       if (gpu_scanout->dma_buf) {
+                               virtio_gpu_dmabuf_unref(gpu_scanout-
dma_buf);
+                               gpu_scanout->dma_buf = NULL;
+                       }
+                       gpu_scanout->active = false;
+               }
+       }
+       free(gpu->gpu_scanouts);
+       gpu->gpu_scanouts = NULL;
+
        pthread_mutex_destroy(&gpu->vga_thread_mtx);
        while (LIST_FIRST(&gpu->r2d_list)) {
                r2d = LIST_FIRST(&gpu->r2d_list);