[Mesa-dev] [PATCH v4 2/2] virgl: Pass resource size and transfer offsets
Gert Wollny
gw.fossdev at gmail.com
Thu Oct 4 13:58:23 UTC 2018
Am Donnerstag, den 04.10.2018, 11:00 +0200 schrieb Gert Wollny:
>
> >
> > What does vtest_get_transfer_size return in most cases? It could
> > be one of two options:
> >
> > (a) (box->width * box->height * box->depth *
> > util_format_get_stride(res->format, box->width)
> > (b) (box->width * box->height * box->depth *
> > util_format_get_stride(res->format, res->width)
>
> (a) and (b) are the same here ;)
>
> (b) should be (box->width * box->height * box->depth * stride)
> The latter is the case when the resource is a screen framebuffer,
> because here the screen dictates the alignment and stride is non-
> zero.
>
> > It should be (a) IMO, but the fact we need to take not read past
> > the
> > buffer and virgl_texture_transfer_map / virgl_buffer_transfer_map
> > implies it's (b). We only want to transfer as much data as Gallium
> > requests.
>
> Certainly. I'll debug when this code path correcting the transfer
> size is actually triggered. I think it might have something to do
> with layer not being used in vtest_get_transfer_size.
Okay, I burned a few hours into fixing this and it seems to be a rather
complicated interplay of various factors (also with virglrenderer). I'd
prefer for not to add a comment and leave this code as is in order to
deal with more pressing issues. After all this code is just there for
the CI and not really relevant for production.
I'll send an updated series including the size calculation correction
shortly.
Best,
Gert
>
>
> >
> > Hopefully, fixing that can help remove the "extra_data" logic in
> > virglrenderer commit fed5d2 and this commit may become
> > simpler.
>
> I think these are two different issues. I'll have to check this, but
> to
> me it seems that this fix in virglrenderer was actually necessary
> because the size was not set correctly (data_size +3/4) instead of
> ((data_size +3) /4)).
>
>
> Thanks for the review,
> Gert
>
>
> >
> >
> > > + 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
> > >
More information about the mesa-dev
mailing list