[Mesa-dev] [PATCH v4 2/2] virgl: Pass resource size and transfer offsets

Gert Wollny gw.fossdev at gmail.com
Thu Oct 4 09:00:34 UTC 2018


Am Mittwoch, den 03.10.2018, 11:06 -0700 schrieb Gurchetan Singh:
> +
> > +
> > +   if (vcmd == VCMD_TRANSFER_PUT2)
> > +      vtest_hdr[VTEST_CMD_LEN] += (data_size + 3) / 4;
> 
> Can you explain this calculation with a comment?  
Will do. 


> Also,
> virgl_vtest_send_transfer_cmd still uses "data_size + 3 / 4" ..
> should we fix that?
Since this is a bugfix of the old code path I'll add it as extra commit
so it can be bisected. 

> 
> > 
> 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. 


> 
> 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