Hello,
Please find my comment below:
Regards,
Andrii.
On Fri, Sep 14, 2018 at 12:23 PM Gert Wollny <gert.wol...@collabora.com>
wrote:
> From: Tomeu Vizoso <tomeu.viz...@collabora.com>
>
> Pass the size of a resource when creating it so a backing can be kept in
> the other side.
>
> Also pass the required offset to transfer commands.
>
> This moves vtest closer to how virtio-gpu works, making it more useful
> for testing.
>
> v2: - Use new messages for creation and transfers, as changing the
> behavior of the existing messages would be messy given that we don't
> want to break compatibility with older servers.
>
> v3: - Gert: Use correct strides: The resource corresponding to the output
> display might have a differnt line stride then the IOVs, so when
> reading back to this resource take the resource stride and the the
> IOV stride into account.
>
> Signed-off-by: Tomeu Vizoso <tomeu.viz...@collabora.com> (v2)
> Signed-off-by: Gert Wollny <gert.wol...@collabora.com>
> ---
> .../winsys/virgl/vtest/virgl_vtest_socket.c | 143
> +++++++++++++++++++--
> .../winsys/virgl/vtest/virgl_vtest_winsys.c | 38 ++++--
> .../winsys/virgl/vtest/virgl_vtest_winsys.h | 19 ++-
> src/gallium/winsys/virgl/vtest/vtest_protocol.h | 29 +++++
> 4 files changed, 201 insertions(+), 28 deletions(-)
>
> diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_socket.c
> b/src/gallium/winsys/virgl/vtest/virgl_vtest_socket.c
> index 4d20a63ad6..3aa01aabdf 100644
> --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_socket.c
> +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_socket.c
> @@ -221,6 +221,42 @@ int virgl_vtest_send_get_caps(struct
> virgl_vtest_winsys *vws,
> return 0;
> }
>
> +static int virgl_vtest_send_resource_create2(struct virgl_vtest_winsys
> *vws,
> + uint32_t handle,
> + enum pipe_texture_target
> target,
> + uint32_t format,
> + uint32_t bind,
> + uint32_t width,
> + uint32_t height,
> + uint32_t depth,
> + uint32_t array_size,
> + uint32_t last_level,
> + uint32_t nr_samples,
> + uint32_t size)
> +{
> + uint32_t res_create_buf[VCMD_RES_CREATE2_SIZE],
> vtest_hdr[VTEST_HDR_SIZE];
> +
> + vtest_hdr[VTEST_CMD_LEN] = VCMD_RES_CREATE2_SIZE;
> + vtest_hdr[VTEST_CMD_ID] = VCMD_RESOURCE_CREATE2;
> +
> + res_create_buf[VCMD_RES_CREATE2_RES_HANDLE] = handle;
> + res_create_buf[VCMD_RES_CREATE2_TARGET] = target;
> + res_create_buf[VCMD_RES_CREATE2_FORMAT] = format;
> + res_create_buf[VCMD_RES_CREATE2_BIND] = bind;
> + res_create_buf[VCMD_RES_CREATE2_WIDTH] = width;
> + res_create_buf[VCMD_RES_CREATE2_HEIGHT] = height;
> + res_create_buf[VCMD_RES_CREATE2_DEPTH] = depth;
> + res_create_buf[VCMD_RES_CREATE2_ARRAY_SIZE] = array_size;
> + res_create_buf[VCMD_RES_CREATE2_LAST_LEVEL] = last_level;
> + res_create_buf[VCMD_RES_CREATE2_NR_SAMPLES] = nr_samples;
> + res_create_buf[VCMD_RES_CREATE2_DATA_SIZE] = size;
> +
> + virgl_block_write(vws->sock_fd, &vtest_hdr, sizeof(vtest_hdr));
> + virgl_block_write(vws->sock_fd, &res_create_buf,
> sizeof(res_create_buf));
> +
> + return 0;
> +}
> +
> int virgl_vtest_send_resource_create(struct virgl_vtest_winsys *vws,
> uint32_t handle,
> enum pipe_texture_target target,
> @@ -231,10 +267,17 @@ int virgl_vtest_send_resource_create(struct
> virgl_vtest_winsys *vws,
> uint32_t depth,
> uint32_t array_size,
> uint32_t last_level,
> - uint32_t nr_samples)
> + uint32_t nr_samples,
> + uint32_t size)
> {
> uint32_t res_create_buf[VCMD_RES_CREATE_SIZE],
> vtest_hdr[VTEST_HDR_SIZE];
>
> + if (vws->protocol_version >= 1)
> + return virgl_vtest_send_resource_create2(vws, handle, target,
> format,
> + bind, width, height, depth,
> + array_size, last_level,
> + nr_samples, size);
> +
> vtest_hdr[VTEST_CMD_LEN] = VCMD_RES_CREATE_SIZE;
> vtest_hdr[VTEST_CMD_ID] = VCMD_RESOURCE_CREATE;
>
> @@ -282,7 +325,7 @@ int virgl_vtest_send_resource_unref(struct
> virgl_vtest_winsys *vws,
> return 0;
> }
>
> -int virgl_vtest_send_transfer_cmd(struct virgl_vtest_winsys *vws,
> +static int virgl_vtest_send_transfer_cmd(struct virgl_vtest_winsys *vws,
> uint32_t vcmd,
> uint32_t handle,
> uint32_t level, uint32_t stride,
> @@ -315,6 +358,73 @@ int virgl_vtest_send_transfer_cmd(struct
> virgl_vtest_winsys *vws,
> return 0;
> }
>
> +static int virgl_vtest_send_transfer_cmd2(struct virgl_vtest_winsys *vws,
> + uint32_t vcmd,
> + uint32_t handle,
> + uint32_t level,
> + const struct pipe_box *box,
> + uint32_t data_size,
> + uint32_t offset)
> +{
> + uint32_t vtest_hdr[VTEST_HDR_SIZE];
> + uint32_t cmd[VCMD_TRANSFER2_HDR_SIZE];
> + vtest_hdr[VTEST_CMD_LEN] = VCMD_TRANSFER2_HDR_SIZE;
> + vtest_hdr[VTEST_CMD_ID] = vcmd;
> +
> +
> + if (vcmd == VCMD_TRANSFER_PUT2)
> + vtest_hdr[VTEST_CMD_LEN] += data_size + 3 / 4;
>
Looks like a copy/paste mistake)
I suppose that it is should be like:
... = (data_size + 3) / 4;
or may be just:
... = data_size;
> +
> + cmd[VCMD_TRANSFER2_RES_HANDLE] = handle;
> + cmd[VCMD_TRANSFER2_LEVEL] = level;
> + cmd[VCMD_TRANSFER2_X] = box->x;
> + cmd[VCMD_TRANSFER2_Y] = box->y;
> + cmd[VCMD_TRANSFER2_Z] = box->z;
> + cmd[VCMD_TRANSFER2_WIDTH] = box->width;
> + cmd[VCMD_TRANSFER2_HEIGHT] = box->height;
> + cmd[VCMD_TRANSFER2_DEPTH] = box->depth;
> + cmd[VCMD_TRANSFER2_DATA_SIZE] = data_size;
> + cmd[VCMD_TRANSFER2_OFFSET] = offset;
> + virgl_block_write(vws->sock_fd, &vtest_hdr, sizeof(vtest_hdr));
> + virgl_block_write(vws->sock_fd, &cmd, sizeof(cmd));
> +
> + return 0;
> +}
> +
> +int virgl_vtest_send_transfer_get(struct virgl_vtest_winsys *vws,
> + uint32_t handle,
> + uint32_t level, uint32_t stride,
> + uint32_t layer_stride,
> + const struct pipe_box *box,
> + uint32_t data_size,
> + uint32_t offset)
> +{
> + if (vws->protocol_version < 1)
> + return virgl_vtest_send_transfer_cmd(vws, VCMD_TRANSFER_GET, handle,
> + level, stride, layer_stride,
> box,
> + data_size);
> +
> + return virgl_vtest_send_transfer_cmd2(vws, VCMD_TRANSFER_GET2, handle,
> + level, box, data_size, offset);
> +}
> +
> +int virgl_vtest_send_transfer_put(struct virgl_vtest_winsys *vws,
> + uint32_t handle,
> + uint32_t level, uint32_t stride,
> + uint32_t layer_stride,
> + const struct pipe_box *box,
> + uint32_t data_size,
> + uint32_t offset)
> +{
> + if (vws->protocol_version < 1)
> + return virgl_vtest_send_transfer_cmd(vws, VCMD_TRANSFER_PUT, handle,
> + level, stride, layer_stride,
> box,
> + data_size);
> +
> + return virgl_vtest_send_transfer_cmd2(vws, VCMD_TRANSFER_PUT2, handle,
> + level, box, data_size, offset);
> +}
> +
> int virgl_vtest_send_transfer_put_data(struct virgl_vtest_winsys *vws,
> void *data,
> uint32_t data_size)
> @@ -327,20 +437,27 @@ int virgl_vtest_recv_transfer_get_data(struct
> virgl_vtest_winsys *vws,
> uint32_t data_size,
> uint32_t stride,
> const struct pipe_box *box,
> - uint32_t format)
> + uint32_t format, uint32_t
> res_stride)
> {
> - void *line;
> - void *ptr = data;
> - int hblocks = util_format_get_nblocksy(format, box->height);
> -
> - line = malloc(stride);
> - while (hblocks) {
> - virgl_block_read(vws->sock_fd, line, stride);
> - memcpy(ptr, line, util_format_get_stride(format, box->width));
> + char *ptr = data;
> + uint32_t bytes_to_read = data_size;
> + char dump[1024];
> +
> + /* Copy the date from the IOV to the target resource respecting
> + * the different strides */
> + for (int y = 0 ; y < box->height && bytes_to_read > 0; ++y) {
> + uint32_t btr = MIN2(res_stride, bytes_to_read);
> + virgl_block_read(vws->sock_fd, ptr, btr);
> ptr += stride;
> - hblocks--;
> + bytes_to_read -= btr;
> + }
> +
> + /* It seems that there may be extra bytes that need to be read */
> + while (bytes_to_read > 0 && bytes_to_read < data_size) {
> + uint32_t btr = MIN2(sizeof(dump), bytes_to_read);
> + virgl_block_read(vws->sock_fd, dump, btr);
> + bytes_to_read -= btr;
> }
> - free(line);
> return 0;
> }
>
> diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
> b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
> index 6c03a6b359..52a5245b6a 100644
> --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
> +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
> @@ -79,9 +79,13 @@ virgl_vtest_transfer_put(struct virgl_winsys *vws,
> size = vtest_get_transfer_size(res, box, stride, layer_stride, level,
> &valid_stride);
>
> - virgl_vtest_send_transfer_cmd(vtws, VCMD_TRANSFER_PUT, res->res_handle,
> + /* Don't read past the end of the buffer */
> + if (size + buf_offset > res->size)
> + size = res->size - buf_offset;
> +
> + virgl_vtest_send_transfer_put(vtws, res->res_handle,
> level, stride, layer_stride,
> - box, size);
> + box, size, buf_offset);
> ptr = virgl_vtest_resource_map(vws, res);
> virgl_vtest_send_transfer_put_data(vtws, ptr + buf_offset, size);
> virgl_vtest_resource_unmap(vws, res);
> @@ -102,15 +106,22 @@ virgl_vtest_transfer_get(struct virgl_winsys *vws,
>
> size = vtest_get_transfer_size(res, box, stride, layer_stride, level,
> &valid_stride);
> + /* Don't ask for more pixels than available */
> + if (size + buf_offset > res->size)
> + size = res->size - buf_offset;
>
> - virgl_vtest_send_transfer_cmd(vtws, VCMD_TRANSFER_GET, res->res_handle,
> + virgl_vtest_send_transfer_get(vtws, res->res_handle,
> level, stride, layer_stride,
> - box, size);
> -
> + box, size, buf_offset);
>
> ptr = virgl_vtest_resource_map(vws, res);
> +
> + /* This functions seems to be using a specific transfer resource that
> + * has exactly the box size and hence its src stride is equal to the
> target
> + * stride */
> virgl_vtest_recv_transfer_get_data(vtws, ptr + buf_offset, size,
> - valid_stride, box, res->format);
> + valid_stride, box, res->format,
> valid_stride);
> +
> virgl_vtest_resource_unmap(vws, res);
> return 0;
> }
> @@ -240,9 +251,10 @@ virgl_vtest_winsys_resource_create(struct
> virgl_winsys *vws,
> res->format = format;
> res->height = height;
> res->width = width;
> + res->size = size;
> virgl_vtest_send_resource_create(vtws, handle, target, format, bind,
> width, height, depth, array_size,
> - last_level, nr_samples);
> + last_level, nr_samples, size);
>
> res->res_handle = handle++;
> pipe_reference_init(&res->reference, 1);
> @@ -613,10 +625,16 @@ static void virgl_vtest_flush_frontbuffer(struct
> virgl_winsys *vws,
> map = vtws->sws->displaytarget_map(vtws->sws, res->dt, 0);
>
> /* execute a transfer */
> - virgl_vtest_send_transfer_cmd(vtws, VCMD_TRANSFER_GET, res->res_handle,
> - level, res->stride, 0, &box, size);
> + virgl_vtest_send_transfer_get(vtws, res->res_handle,
> + level, res->stride, 0, &box, size,
> offset);
> +
> + /* This functions gets the resource from the hardware backend that may
> have
> + * a hardware imposed stride that is different from the IOV stride
> used to
> + * get the data. */
> virgl_vtest_recv_transfer_get_data(vtws, map + offset, size,
> valid_stride,
> - &box, res->format);
> + &box, res->format,
> + util_format_get_stride(res->format,
> res->width));
> +
> vtws->sws->displaytarget_unmap(vtws->sws, res->dt);
>
> vtws->sws->displaytarget_display(vtws->sws, res->dt,
> winsys_drawable_handle,
> diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.h
> b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.h
> index 3628c74644..e51582032a 100644
> --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.h
> +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.h
> @@ -121,20 +121,29 @@ int virgl_vtest_send_resource_create(struct
> virgl_vtest_winsys *vws,
> uint32_t depth,
> uint32_t array_size,
> uint32_t last_level,
> - uint32_t nr_samples);
> + uint32_t nr_samples,
> + uint32_t size);
>
> int virgl_vtest_send_resource_unref(struct virgl_vtest_winsys *vws,
> uint32_t handle);
> int virgl_vtest_submit_cmd(struct virgl_vtest_winsys *vtws,
> struct virgl_vtest_cmd_buf *cbuf);
>
> -int virgl_vtest_send_transfer_cmd(struct virgl_vtest_winsys *vws,
> - uint32_t vcmd,
> +int virgl_vtest_send_transfer_get(struct virgl_vtest_winsys *vws,
> uint32_t handle,
> uint32_t level, uint32_t stride,
> uint32_t layer_stride,
> const struct pipe_box *box,
> - uint32_t data_size);
> + uint32_t data_size,
> + uint32_t offset);
> +
> +int virgl_vtest_send_transfer_put(struct virgl_vtest_winsys *vws,
> + uint32_t handle,
> + uint32_t level, uint32_t stride,
> + uint32_t layer_stride,
> + const struct pipe_box *box,
> + uint32_t data_size,
> + uint32_t offset);
>
> int virgl_vtest_send_transfer_put_data(struct virgl_vtest_winsys *vws,
> void *data,
> @@ -144,7 +153,7 @@ int virgl_vtest_recv_transfer_get_data(struct
> virgl_vtest_winsys *vws,
> uint32_t data_size,
> uint32_t stride,
> const struct pipe_box *box,
> - uint32_t format);
> + uint32_t format, uint32_t
> res_width);
>
> int virgl_vtest_busy_wait(struct virgl_vtest_winsys *vws, int handle,
> int flags);
> diff --git a/src/gallium/winsys/virgl/vtest/vtest_protocol.h
> b/src/gallium/winsys/virgl/vtest/vtest_protocol.h
> index 8eb904e73f..c299c31418 100644
> --- a/src/gallium/winsys/virgl/vtest/vtest_protocol.h
> +++ b/src/gallium/winsys/virgl/vtest/vtest_protocol.h
> @@ -58,6 +58,10 @@
>
> #define VCMD_PROTOCOL_VERSION 11
>
> +#define VCMD_RESOURCE_CREATE2 12
> +#define VCMD_TRANSFER_GET2 13
> +#define VCMD_TRANSFER_PUT2 14
> +
> #define VCMD_RES_CREATE_SIZE 10
> #define VCMD_RES_CREATE_RES_HANDLE 0
> #define VCMD_RES_CREATE_TARGET 1
> @@ -70,6 +74,19 @@
> #define VCMD_RES_CREATE_LAST_LEVEL 8
> #define VCMD_RES_CREATE_NR_SAMPLES 9
>
> +#define VCMD_RES_CREATE2_SIZE 11
> +#define VCMD_RES_CREATE2_RES_HANDLE 0
> +#define VCMD_RES_CREATE2_TARGET 1
> +#define VCMD_RES_CREATE2_FORMAT 2
> +#define VCMD_RES_CREATE2_BIND 3
> +#define VCMD_RES_CREATE2_WIDTH 4
> +#define VCMD_RES_CREATE2_HEIGHT 5
> +#define VCMD_RES_CREATE2_DEPTH 6
> +#define VCMD_RES_CREATE2_ARRAY_SIZE 7
> +#define VCMD_RES_CREATE2_LAST_LEVEL 8
> +#define VCMD_RES_CREATE2_NR_SAMPLES 9
> +#define VCMD_RES_CREATE2_DATA_SIZE 10
> +
> #define VCMD_RES_UNREF_SIZE 1
> #define VCMD_RES_UNREF_RES_HANDLE 0
>
> @@ -86,6 +103,18 @@
> #define VCMD_TRANSFER_DEPTH 9
> #define VCMD_TRANSFER_DATA_SIZE 10
>
> +#define VCMD_TRANSFER2_HDR_SIZE 10
> +#define VCMD_TRANSFER2_RES_HANDLE 0
> +#define VCMD_TRANSFER2_LEVEL 1
> +#define VCMD_TRANSFER2_X 2
> +#define VCMD_TRANSFER2_Y 3
> +#define VCMD_TRANSFER2_Z 4
> +#define VCMD_TRANSFER2_WIDTH 5
> +#define VCMD_TRANSFER2_HEIGHT 6
> +#define VCMD_TRANSFER2_DEPTH 7
> +#define VCMD_TRANSFER2_DATA_SIZE 8
> +#define VCMD_TRANSFER2_OFFSET 9
> +
> #define VCMD_BUSY_WAIT_FLAG_WAIT 1
>
> #define VCMD_BUSY_WAIT_SIZE 2
> --
> 2.16.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
