Re: [Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-17 Thread Gurchetan Singh 
On Tue, Dec 11, 2018 at 2:49 PM Elie Tournier 
wrote:

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

Thanks!  When your tests are complete, I posted a slightly updated series
on Gitlab:

https://gitlab.freedesktop.org/mesa/mesa/merge_requests/25


> > ---
> >  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(>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, >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,
> > -   >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,
> > +>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(>flush_list, >to_flush_bufs);
> > -   vbuf->on_list = TRUE;
> > -   pipe_resource_reference(, >base.u.b);
> > -   }
> > -
> > -   util_range_add(>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(>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 = >base;
> > buf->base.u.vtbl = _buffer_vtbl;
> > pipe_reference_init(>base.u.b.reference, 1);
> > -   util_range_init(>valid_buffer_range);
> >

Re: [Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-11 Thread Elie Tournier 
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 
> ---
>  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(>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, >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,
> -   >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,
> +>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(>flush_list, >to_flush_bufs);
> -   vbuf->on_list = TRUE;
> -   pipe_resource_reference(, >base.u.b);
> -   }
> -
> -   util_range_add(>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(>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 = >base;
> buf->base.u.vtbl = _buffer_vtbl;
> pipe_reference_init(>base.u.b.reference, 1);
> -   util_range_init(>valid_buffer_range);
> virgl_resource_layout(>base.u.b, >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(>valid_buffer_range);
> return >base.u.b;
>

[Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-10 Thread Gurchetan Singh 
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)
---
 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(>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, >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,
-   >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,
+>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(>flush_list, >to_flush_bufs);
-   vbuf->on_list = TRUE;
-   pipe_resource_reference(, >base.u.b);
-   }
-
-   util_range_add(>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(>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 = >base;
buf->base.u.vtbl = _buffer_vtbl;
pipe_reference_init(>base.u.b.reference, 1);
-   util_range_init(>valid_buffer_range);
virgl_resource_layout(>base.u.b, >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(>valid_buffer_range);
return >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)
-{
-

Re: [Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-10 Thread Gurchetan Singh 
On Mon, Dec 10, 2018 at 7:35 AM Elie Tournier 
wrote:

> On Thu, Dec 06, 2018 at 05:20:42PM -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.
> I don't know how to feel about that.
> We clearly go against the spec.
>
> We currently know the behavor of this piece of code but in few
> months, someone will facing the issue and loose lots of time.
>

I agree, the problem is the kernel transfer API
(drm_virtgpu_3d_transfer_to_host) doesn't pass usage flags
(PIPE_TRANSFER_FLUSH_EXPLICIT) such that the host can use
GL_MAP_FLUSH_EXPLICIT_BIT.  The best method is to use the virgl protocol to
encode the transfer.  This series is a self-contained cleanup for that:

https://gitlab.freedesktop.org/gurchetansingh/mesa/tree/optimize-transfers

In the meantime, I'll add big comment.


> I would prefer that we fix it now.
> If we decide to still upstream that code, we should at least add
> a big warning.
>






> > ---
> >  src/gallium/drivers/virgl/virgl_buffer.c   | 37 +-
> >  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, 16 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/gallium/drivers/virgl/virgl_buffer.c
> b/src/gallium/drivers/virgl/virgl_buffer.c
> > index d5d728735e..ae828446ec 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(>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, >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,
> > -   >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,
> > +>box, trans->base.stride,
> > +trans->base.layer_stride, trans->offset,
> > +transfer->level);
> > +
> > }
> >
> > virgl_resource_destroy_transfer(vctx, trans);
> > @@ -108,20 +113,10 @@ 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(>flush_list, >to_flush_bufs);
> > -   vbuf->on_list = TRUE;
> > -   pipe_resource_reference(, >base.u.b);
> > -   }
> > -
> > -   util_range_add(>valid_buffer_range, transfer->box.x + box->x,
> > -  transfer->box.x + box->x + box->width);
> > -
> > +   util_range_add(>range, box->x, box->x + box->width);
> > vbuf->base.clean = FALSE;
> >  }
> >
> > @@ -145,7 +140,6 @@ struct pipe_resource *virgl_buffer_create(struct
> virgl_screen *vs,
> > buf->base.u.b.screen = >base;
> > buf->base.u.vtbl = _buffer_vtbl;
> >

Re: [Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-10 Thread Elie Tournier 
On Thu, Dec 06, 2018 at 05:20:42PM -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.
I don't know how to feel about that.
We clearly go against the spec.

We currently know the behavor of this piece of code but in few
months, someone will facing the issue and loose lots of time.
I would prefer that we fix it now.
If we decide to still upstream that code, we should at least add
a big warning.
> ---
>  src/gallium/drivers/virgl/virgl_buffer.c   | 37 +-
>  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, 16 insertions(+), 69 deletions(-)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_buffer.c 
> b/src/gallium/drivers/virgl/virgl_buffer.c
> index d5d728735e..ae828446ec 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(>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, >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,
> -   >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,
> +>box, trans->base.stride,
> +trans->base.layer_stride, trans->offset,
> +transfer->level);
> +
> }
>  
> virgl_resource_destroy_transfer(vctx, trans);
> @@ -108,20 +113,10 @@ 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(>flush_list, >to_flush_bufs);
> -   vbuf->on_list = TRUE;
> -   pipe_resource_reference(, >base.u.b);
> -   }
> -
> -   util_range_add(>valid_buffer_range, transfer->box.x + box->x,
> -  transfer->box.x + box->x + box->width);
> -
> +   util_range_add(>range, box->x, box->x + box->width);
> vbuf->base.clean = FALSE;
>  }
>  
> @@ -145,7 +140,6 @@ struct pipe_resource *virgl_buffer_create(struct 
> virgl_screen *vs,
> buf->base.u.b.screen = >base;
> buf->base.u.vtbl = _buffer_vtbl;
> pipe_reference_init(>base.u.b.reference, 1);
> -   util_range_init(>valid_buffer_range);
> virgl_resource_layout(>base.u.b, >metadata);
>  
> vbind = pipe_to_virgl_bind(template->bind);
> @@ -155,6 +149,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(>valid_buffer_range);
> return >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
> ---

[Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

2018-12-06 Thread Gurchetan Singh 
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.
---
 src/gallium/drivers/virgl/virgl_buffer.c   | 37 +-
 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, 16 insertions(+), 69 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_buffer.c 
b/src/gallium/drivers/virgl/virgl_buffer.c
index d5d728735e..ae828446ec 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(>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, >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,
-   >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,
+>box, trans->base.stride,
+trans->base.layer_stride, trans->offset,
+transfer->level);
+
}
 
virgl_resource_destroy_transfer(vctx, trans);
@@ -108,20 +113,10 @@ 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(>flush_list, >to_flush_bufs);
-   vbuf->on_list = TRUE;
-   pipe_resource_reference(, >base.u.b);
-   }
-
-   util_range_add(>valid_buffer_range, transfer->box.x + box->x,
-  transfer->box.x + box->x + box->width);
-
+   util_range_add(>range, box->x, box->x + box->width);
vbuf->base.clean = FALSE;
 }
 
@@ -145,7 +140,6 @@ struct pipe_resource *virgl_buffer_create(struct 
virgl_screen *vs,
buf->base.u.b.screen = >base;
buf->base.u.vtbl = _buffer_vtbl;
pipe_reference_init(>base.u.b.reference, 1);
-   util_range_init(>valid_buffer_range);
virgl_resource_layout(>base.u.b, >metadata);
 
vbind = pipe_to_virgl_bind(template->bind);
@@ -155,6 +149,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(>valid_buffer_range);
return >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 -