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