<div dir="ltr"><div dir="ltr"><div dir="ltr"> </div> <div class="gmail_quote"><div dir="ltr">On Tue, Dec 11, 2018 at 2:49 PM Elie Tournier <<a href="mailto:tournier.elie@gmail.com">tournier.elie@gmail.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Dec 10, 2018 at 10:20:36AM -0800, Gurchetan Singh wrote: > Previously, we ignored the the glUnmap(..) operation and > flushed before we flush the cbuf.Â Now, let's just flush > the data when we unmap. > > Neither method is optimal, for example: > > glMapBufferRange(.., 0, 100, GL_MAP_FLUSH_EXPLICIT_BIT) > glFlushMappedBufferRange(.., 25, 30) > glFlushMappedBufferRange(.., 65, 70) > > We'll end up flushing 25 --> 70.Â Maybe we can fix this later. > > v2: Add fixme comment in the code (Elie) Thanks. I still have to run some regressions tests. They are a bit slow on my system. So for now, the series is: Acked-by: Elie Tournier <<a href="mailto:elie.tournier@collabora.com" target="_blank">elie.tournier@collabora.com</a>> </blockquote><div> </div><div>Thanks!Â When your tests are complete, I posted a slightly updated series on Gitlab:</div><div> </div><div><a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests/25">https://gitlab.freedesktop.org/mesa/mesa/merge_requests/25</a></div><div>Â </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > --- >Â src/gallium/drivers/virgl/virgl_buffer.cÂ Â | 46 +++++++++++----------- >Â src/gallium/drivers/virgl/virgl_context.cÂ | 34 +--------------- >Â src/gallium/drivers/virgl/virgl_context.hÂ |Â 1 - >Â src/gallium/drivers/virgl/virgl_resource.h | 13 ------ >Â 4 files changed, 25 insertions(+), 69 deletions(-) > > diff --git a/src/gallium/drivers/virgl/virgl_buffer.c b/src/gallium/drivers/virgl/virgl_buffer.c > index d5d728735e..a20deab549 100644 > --- a/src/gallium/drivers/virgl/virgl_buffer.c > +++ b/src/gallium/drivers/virgl/virgl_buffer.c > @@ -33,7 +33,6 @@ static void virgl_buffer_destroy(struct pipe_screen *screen, >Â Â Â struct virgl_screen *vs = virgl_screen(screen); >Â Â Â struct virgl_buffer *vbuf = virgl_buffer(buf); >Â > -Â Â util_range_destroy(&vbuf->valid_buffer_range); >Â Â Â vs->vws->resource_unref(vs->vws, vbuf->base.hw_res); >Â Â Â FREE(vbuf); >Â } > @@ -53,7 +52,7 @@ static void *virgl_buffer_transfer_map(struct pipe_context *ctx, >Â Â Â bool readback; >Â Â Â bool doflushwait = false; >Â > -Â Â if ((usage & PIPE_TRANSFER_READ) && (vbuf->on_list == TRUE)) > +Â Â if (usage & PIPE_TRANSFER_READ) >Â Â Â Â doflushwait = true; >Â Â Â else >Â Â Â Â doflushwait = virgl_res_needs_flush_wait(vctx, &vbuf->base, usage); > @@ -92,13 +91,19 @@ static void virgl_buffer_transfer_unmap(struct pipe_context *ctx, >Â Â Â struct virgl_buffer *vbuf = virgl_buffer(transfer->resource); >Â >Â Â Â if (trans->base.usage & PIPE_TRANSFER_WRITE) { > -Â Â Â if (!(transfer->usage & PIPE_TRANSFER_FLUSH_EXPLICIT)) { > -Â Â Â Â Â struct virgl_screen *vs = virgl_screen(ctx->screen); > -Â Â Â Â Â vctx->num_transfers++; > -Â Â Â Â Â vs->vws->transfer_put(vs->vws, vbuf->base.hw_res, > -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &transfer->box, trans->base.stride, trans->base.layer_stride, trans->offset, transfer->level); > - > +Â Â Â struct virgl_screen *vs = virgl_screen(ctx->screen); > +Â Â Â if (transfer->usage & PIPE_TRANSFER_FLUSH_EXPLICIT) { > +Â Â Â Â Â transfer->box.x += trans->range.start; > +Â Â Â Â Â transfer->box.width = trans->range.end - trans->range.start; > +Â Â Â Â Â trans->offset = transfer->box.x; >Â Â Â Â } > + > +Â Â Â vctx->num_transfers++; > +Â Â Â vs->vws->transfer_put(vs->vws, vbuf->base.hw_res, > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â &transfer->box, trans->base.stride, > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â trans->base.layer_stride, trans->offset, > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â transfer->level); > + >Â Â Â } >Â >Â Â Â virgl_resource_destroy_transfer(vctx, trans); > @@ -108,20 +113,19 @@ static void virgl_buffer_transfer_flush_region(struct pipe_context *ctx, >Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct pipe_transfer *transfer, >Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â const struct pipe_box *box) >Â { > -Â Â struct virgl_context *vctx = virgl_context(ctx); >Â Â Â struct virgl_buffer *vbuf = virgl_buffer(transfer->resource); > +Â Â struct virgl_transfer *trans = virgl_transfer(transfer); >Â > -Â Â if (!vbuf->on_list) { > -Â Â Â Â struct pipe_resource *res = NULL; > - > -Â Â Â Â list_addtail(&vbuf->flush_list, &vctx->to_flush_bufs); > -Â Â Â Â vbuf->on_list = TRUE; > -Â Â Â Â pipe_resource_reference(&res, &vbuf->base.u.b); > -Â Â } > - > -Â Â util_range_add(&vbuf->valid_buffer_range, transfer->box.x + box->x, > -Â Â Â Â Â Â Â Â Â transfer->box.x + box->x + box->width); > - > +Â Â /* > +Â Â * FIXME: This is not optimal.Â For example, > +Â Â * > +Â Â * glMapBufferRange(.., 0, 100, GL_MAP_FLUSH_EXPLICIT_BIT) > +Â Â * glFlushMappedBufferRange(.., 25, 30) > +Â Â * glFlushMappedBufferRange(.., 65, 70) > +Â Â * > +Â Â * We'll end up flushing 25 --> 70. > +Â Â */ > +Â Â util_range_add(&trans->range, box->x, box->x + box->width); >Â Â Â vbuf->base.clean = FALSE; >Â } >Â > @@ -145,7 +149,6 @@ struct pipe_resource *virgl_buffer_create(struct virgl_screen *vs, >Â Â Â buf->base.u.b.screen = &vs->base; >Â Â Â buf->base.u.vtbl = &virgl_buffer_vtbl; >Â Â Â pipe_reference_init(&buf->base.u.b.reference, 1); > -Â Â util_range_init(&buf->valid_buffer_range); >Â Â Â virgl_resource_layout(&buf->base.u.b, &buf->metadata); >Â >Â Â Â vbind = pipe_to_virgl_bind(template->bind); > @@ -155,6 +158,5 @@ struct pipe_resource *virgl_buffer_create(struct virgl_screen *vs, >Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â template->width0, 1, 1, 1, 0, 0, >Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â buf->metadata.total_size); >Â > -Â Â util_range_set_empty(&buf->valid_buffer_range); >Â Â Â return &buf->base.u.b; >Â } > diff --git a/src/gallium/drivers/virgl/virgl_context.c b/src/gallium/drivers/virgl/virgl_context.c > index 90a9429fa5..a92fd6115a 100644 > --- a/src/gallium/drivers/virgl/virgl_context.c > +++ b/src/gallium/drivers/virgl/virgl_context.c > @@ -54,29 +54,6 @@ uint32_t virgl_object_assign_handle(void) >Â Â Â return ++next_handle; >Â } >Â > -static void virgl_buffer_flush(struct virgl_context *vctx, > -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct virgl_buffer *vbuf) > -{ > -Â Â struct virgl_screen *rs = virgl_screen(vctx->base.screen); > -Â Â struct pipe_box box; > - > -Â Â assert(vbuf->on_list); > - > -Â Â box.height = 1; > -Â Â box.depth = 1; > -Â Â box.y = 0; > -Â Â box.z = 0; > - > -Â Â box.x = vbuf->valid_buffer_range.start; > -Â Â box.width = MIN2(vbuf->valid_buffer_range.end - vbuf->valid_buffer_range.start, vbuf->base.u.b.width0); > - > -Â Â vctx->num_transfers++; > -Â Â rs->vws->transfer_put(rs->vws, vbuf->base.hw_res, > -Â Â Â Â Â Â Â Â Â Â Â Â Â &box, 0, 0, box.x, 0); > - > -Â Â util_range_set_empty(&vbuf->valid_buffer_range); > -} > - >Â static void virgl_attach_res_framebuffer(struct virgl_context *vctx) >Â { >Â Â Â struct virgl_winsys *vws = virgl_screen(vctx->base.screen)->vws; > @@ -736,19 +713,11 @@ static void virgl_flush_from_st(struct pipe_context *ctx, >Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enum pipe_flush_flags flags) >Â { >Â Â Â struct virgl_context *vctx = virgl_context(ctx); > -Â Â struct virgl_buffer *buf, *tmp; > +Â Â struct virgl_screen *rs = virgl_screen(ctx->screen); >Â >Â Â Â if (flags & PIPE_FLUSH_FENCE_FD) >Â Â Â Â Â vctx->cbuf->needs_out_fence_fd = true; >Â > -Â Â LIST_FOR_EACH_ENTRY_SAFE(buf, tmp, &vctx->to_flush_bufs, flush_list) { > -Â Â Â struct pipe_resource *res = &buf->base.u.b; > -Â Â Â virgl_buffer_flush(vctx, buf); > -Â Â Â list_del(&buf->flush_list); > -Â Â Â buf->on_list = FALSE; > -Â Â Â pipe_resource_reference(&res, NULL); > - > -Â Â } >Â Â Â virgl_flush_eq(vctx, vctx, fence); >Â >Â Â Â if (vctx->cbuf->in_fence_fd != -1) { > @@ -1291,7 +1260,6 @@ struct pipe_context *virgl_context_create(struct pipe_screen *pscreen, >Â Â Â virgl_init_query_functions(vctx); >Â Â Â virgl_init_so_functions(vctx); >Â > -Â Â list_inithead(&vctx->to_flush_bufs); >Â Â Â slab_create_child(&vctx->transfer_pool, &rs->transfer_pool); >Â >Â Â Â vctx->primconvert = util_primconvert_create(&vctx->base, rs->caps.caps.v1.prim_mask); > diff --git a/src/gallium/drivers/virgl/virgl_context.h b/src/gallium/drivers/virgl/virgl_context.h > index 4a6cc09ee5..65166154e4 100644 > --- a/src/gallium/drivers/virgl/virgl_context.h > +++ b/src/gallium/drivers/virgl/virgl_context.h > @@ -73,7 +73,6 @@ struct virgl_context { >Â Â Â struct pipe_resource *images[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_BUFFERS]; >Â Â Â int num_transfers; >Â Â Â int num_draws; > -Â Â struct list_head to_flush_bufs; >Â >Â Â Â struct pipe_resource *atomic_buffers[PIPE_MAX_HW_ATOMIC_BUFFERS]; >Â > diff --git a/src/gallium/drivers/virgl/virgl_resource.h b/src/gallium/drivers/virgl/virgl_resource.h > index fa985494a7..2e2fa186d1 100644 > --- a/src/gallium/drivers/virgl/virgl_resource.h > +++ b/src/gallium/drivers/virgl/virgl_resource.h > @@ -52,19 +52,6 @@ struct virgl_resource { >Â >Â struct virgl_buffer { >Â Â Â struct virgl_resource base; > - > -Â Â struct list_head flush_list; > -Â Â boolean on_list; > - > -Â Â /* The buffer range which is initialized (with a write transfer, > -Â Â * streamout, DMA, or as a random access target). The rest of > -Â Â * the buffer is considered invalid and can be mapped unsynchronized. > -Â Â * > -Â Â * This allows unsychronized mapping of a buffer range which hasn't > -Â Â * been used yet. It's for applications which forget to use > -Â Â * the unsynchronized map flag and expect the driver to figure it out. > -Â Â */ > -Â Â struct util_range valid_buffer_range; >Â Â Â struct virgl_resource_metadata metadata; >Â }; >Â > -- > 2.18.1 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div></div>