[Mesa-dev] [PATCH v5 2/2] virgl: Pass resource size and transfer offsets
Tomeu Vizoso
tomeu.vizoso at collabora.com
Fri Oct 5 05:48:10 UTC 2018
On 10/4/18 7:48 PM, Gurchetan Singh wrote:
> On Thu, Oct 4, 2018 at 7:41 AM Gert Wollny <gw.fossdev at gmail.com> wrote:
>>
>> From: Tomeu Vizoso <tomeu.vizoso at 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: - 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.
>>
>> v4: Fix transfer size calculation (Andrey Simiklit)
>>
>> v5: Add comment about transfer size value in the PUT commend (Gurchetan).
>> Add a comment about the size correction for transfers for reading and
>> writing the resource. Fixing this by correctly evaluating the size
>> upfront will need some work also on the virglrenderer side.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com> (v2)
>> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
>> ---
>> .../winsys/virgl/vtest/virgl_vtest_socket.c | 144 +++++++++++++++++++--
>> .../winsys/virgl/vtest/virgl_vtest_winsys.c | 44 +++++--
>> .../winsys/virgl/vtest/virgl_vtest_winsys.h | 19 ++-
>> src/gallium/winsys/virgl/vtest/vtest_protocol.h | 29 +++++
>> 4 files changed, 208 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 ce565ee76c..ff69accf52 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,
>> @@ -317,6 +360,74 @@ 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;
>> +
>> + /* The host expects the size in dwords so calculate the rounded up
>> + * value here. */
>> + if (vcmd == VCMD_TRANSFER_PUT2)
>> + vtest_hdr[VTEST_CMD_LEN] += (data_size + 3) / 4;
>> +
>> + 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)
>> @@ -329,20 +440,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..a589f694bb 100644
>> --- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
>> +++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
>> @@ -79,9 +79,19 @@ 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,
>> + /* The size calculated above is the full box size, but if this box origin
>> + * is not zero we may have to correct the transfer size to not read past the
>> + * end of the resource. The correct adjustment depends on various factors
>> + * that are not documented, so instead of going though all the hops to get
>> + * the size right up-front, we just make sure we don't read past the end.
>> + * FIXME: figure out what it takes to actually get this right.
>> + */
>> + if (size + buf_offset > res->size)
>> + size = res->size - buf_offset;
>
> The idea is to get rid of any adjustments on both the Mesa /
> virglrenderer sides -- so transfer size is just what's needed to
> transfer the box size, and the offset is the just offset into the
> iovec from which the transfer will start. For v1, we can just specify
> an offset of zero on the virglrenderer side.
>
> vtest isn't used in production but it will be used in the CI. Also,
> it's a simpler model of virgl3d and is useful for experimenting with
> new protocol additions (hostmap, vulkan). We should fix this now --
> it could take a while to disentangle the workarounds should anyone
> look at this again ...
Precisely because the main use case is CI, I think vtest should be made
to work as similarly as possible as the virtio-gpu-backed
implementations. In which case this code will be completely removed along
with this concern.
Haven't thought of all the details, but I would expect that with FD
passing on the vtest socket we could hit the same code paths as with
virtio-gpu once we manage to map buffers across domains.
But in the meantime, would be nice to have a more efficient way of
dealing with regressions.
Cheers,
Tomeu
>> +
>> + 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 +112,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 (see above) */
>> + 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 +257,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 +631,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
>>
More information about the mesa-dev
mailing list