[Mesa-dev] [PATCH 2/2] virgl: don't mark buffers as unclean after a write

[Erik Faye-Lund]

On Wed, 2018-11-21 at 20:08 -0800, Gurchetan Singh wrote:
> We can mark the buffer unclean if it's ever bound as a TBO,
> SSBO, ABO, or image.
> 
> This improves
> 
> dEQP-
> GLES3.performance.buffer.data_upload.function_call.map_buffer_range.n
> ew_specified_buffer.flag_write_full.stream_draw
> 
> from 9.58 MB/s to 451.17 MB/s.
> 
> v2: Using buffer bindings to track cleanliness (Ilia).
> ---
>  src/gallium/drivers/virgl/virgl_buffer.c |  1 -
>  src/gallium/drivers/virgl/virgl_encode.c | 10 ++++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_buffer.c
> b/src/gallium/drivers/virgl/virgl_buffer.c
> index 88a22b56f9..f72c93f499 100644
> --- a/src/gallium/drivers/virgl/virgl_buffer.c
> +++ b/src/gallium/drivers/virgl/virgl_buffer.c
> @@ -106,7 +106,6 @@ static void virgl_buffer_transfer_unmap(struct
> pipe_context *ctx,
>     if (trans->base.usage & PIPE_TRANSFER_WRITE) {
>        if (!(transfer->usage & PIPE_TRANSFER_FLUSH_EXPLICIT)) {
>           struct virgl_screen *vs = virgl_screen(ctx->screen);
> -         vbuf->base.clean = FALSE;
>           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);
> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> index 400ba68474..6483f47031 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.c
> +++ b/src/gallium/drivers/virgl/virgl_encode.c
> @@ -61,6 +61,12 @@ static void virgl_encoder_write_res(struct
> virgl_context *ctx,
>     }
>  }
>  
> +static void virgl_modify_clean(struct virgl_resource *res, boolean
> value)
> +{
> +   if (res)
> +      res->clean = value;

It looks like not all call-sites can have res == NULL, so it would be
better to move the conditional out to the call-sites, I think. And then
I would argue the usefulness of the helper is diminished...

Anyway, if you decide to keep the helper: it seems like this is always
called with a FALSE-argument, perhaps this should be:

void virgl_dirty_res(struct virgl_resource *res)

... instead?

> +}
> +
>  int virgl_encode_bind_object(struct virgl_context *ctx,
>                              uint32_t handle, uint32_t object)
>  {
> @@ -615,6 +621,7 @@ int virgl_encode_sampler_view(struct
> virgl_context *ctx,
>     if (res->u.b.target == PIPE_BUFFER) {
>        virgl_encoder_write_dword(ctx->cbuf, state->u.buf.offset /
> elem_size);
>        virgl_encoder_write_dword(ctx->cbuf, (state->u.buf.offset +
> state->u.buf.size) / elem_size - 1);
> +      virgl_modify_clean(res, FALSE);
>     } else {
>        virgl_encoder_write_dword(ctx->cbuf, state->u.tex.first_layer
> | state->u.tex.last_layer << 16);
>        virgl_encoder_write_dword(ctx->cbuf, state->u.tex.first_level
> | state->u.tex.last_level << 8);
> @@ -949,6 +956,7 @@ int virgl_encode_set_shader_buffers(struct
> virgl_context *ctx,
>           virgl_encoder_write_dword(ctx->cbuf,
> buffers[i].buffer_offset);
>           virgl_encoder_write_dword(ctx->cbuf,
> buffers[i].buffer_size);
>           virgl_encoder_write_res(ctx, res);
> +         virgl_modify_clean(res, FALSE);
>        } else {
>           virgl_encoder_write_dword(ctx->cbuf, 0);
>           virgl_encoder_write_dword(ctx->cbuf, 0);
> @@ -972,6 +980,7 @@ int virgl_encode_set_hw_atomic_buffers(struct
> virgl_context *ctx,
>           virgl_encoder_write_dword(ctx->cbuf,
> buffers[i].buffer_offset);
>           virgl_encoder_write_dword(ctx->cbuf,
> buffers[i].buffer_size);
>           virgl_encoder_write_res(ctx, res);
> +         virgl_modify_clean(res, FALSE);
>        } else {
>           virgl_encoder_write_dword(ctx->cbuf, 0);
>           virgl_encoder_write_dword(ctx->cbuf, 0);
> @@ -999,6 +1008,7 @@ int virgl_encode_set_shader_images(struct
> virgl_context *ctx,
>           virgl_encoder_write_dword(ctx->cbuf,
> images[i].u.buf.offset);
>           virgl_encoder_write_dword(ctx->cbuf, images[i].u.buf.size);
>           virgl_encoder_write_res(ctx, res);
> +         virgl_modify_clean(res, FALSE);
>        } else {
>           virgl_encoder_write_dword(ctx->cbuf, 0);
>           virgl_encoder_write_dword(ctx->cbuf, 0);