[Mesa-dev] [PATCH] virgl: add initial shader_storage_buffer_object support. (v2)

Gert Wollny gert.wollny at collabora.com
Fri Jul 20 11:13:00 UTC 2018


There still seem to be a few quirks (see below)  

Best, 
Gert 

Am Freitag, den 20.07.2018, 06:36 +1000 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> This adds the guest side support for
> ARB_shader_storage_buffer_object.
> 
> Co-authors: Gurchetan Singh <gurchetansingh at chromium.org>
> 
> v2: move to using separate maximums
> ---
>  src/gallium/drivers/virgl/virgl_buffer.c   |  2 ++
>  src/gallium/drivers/virgl/virgl_context.c  | 44
> ++++++++++++++++++++++++++++++
>  src/gallium/drivers/virgl/virgl_context.h  |  2 ++
>  src/gallium/drivers/virgl/virgl_encode.c   | 25 +++++++++++++++++
>  src/gallium/drivers/virgl/virgl_encode.h   |  5 ++++
>  src/gallium/drivers/virgl/virgl_hw.h       |  3 ++
>  src/gallium/drivers/virgl/virgl_protocol.h | 10 +++++++
>  src/gallium/drivers/virgl/virgl_resource.h |  2 ++
>  src/gallium/drivers/virgl/virgl_screen.c   |  5 ++++
>  9 files changed, 98 insertions(+)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_buffer.c
> b/src/gallium/drivers/virgl/virgl_buffer.c
> index 2e63aebc72c..3288bb20bd1 100644
> --- a/src/gallium/drivers/virgl/virgl_buffer.c
> +++ b/src/gallium/drivers/virgl/virgl_buffer.c
> @@ -164,6 +164,8 @@ struct pipe_resource *virgl_buffer_create(struct
> virgl_screen *vs,
>     vbind = pipe_to_virgl_bind(template->bind);
>     size = template->width0;
>  
> +   if (vbind == VIRGL_BIND_SHADER_BUFFER)
> +      buf->base.clean = FALSE;
>     buf->base.hw_res = vs->vws->resource_create(vs->vws, template-
> >target, template->format, vbind, template->width0, 1, 1, 1, 0, 0,
> size);
>  
>     util_range_set_empty(&buf->valid_buffer_range);
> diff --git a/src/gallium/drivers/virgl/virgl_context.c
> b/src/gallium/drivers/virgl/virgl_context.c
> index ee28680b8fc..74b232fe6cf 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -168,6 +168,20 @@ static void
> virgl_attach_res_uniform_buffers(struct virgl_context *vctx,
>     }
>  }
>  
> +static void virgl_attach_res_shader_buffers(struct virgl_context
> *vctx,
> +                                            enum pipe_shader_type
> shader_type)
> +{
> +   struct virgl_winsys *vws = virgl_screen(vctx->base.screen)->vws;
> +   struct virgl_resource *res;
> +   unsigned i;
> +   for (i = 0; i < PIPE_MAX_SHADER_BUFFERS; i++) {
> +      res = virgl_resource(vctx->ssbos[shader_type][i]);
> +      if (res) {
> +         vws->emit_res(vws, vctx->cbuf, res->hw_res, FALSE);
> +      }
> +   }
> +}
> +
>  /*
>   * after flushing, the hw context still has a bunch of
>   * resources bound, so we need to rebind those here.
> @@ -183,6 +197,7 @@ static void virgl_reemit_res(struct virgl_context
> *vctx)
>     for (shader_type = 0; shader_type < PIPE_SHADER_TYPES;
> shader_type++) {
>        virgl_attach_res_sampler_views(vctx, shader_type);
>        virgl_attach_res_uniform_buffers(vctx, shader_type);
> +      virgl_attach_res_shader_buffers(vctx, shader_type);
>     }
>     virgl_attach_res_vertex_buffers(vctx);
>     virgl_attach_res_so_targets(vctx);
> @@ -911,6 +926,34 @@ static void virgl_blit(struct pipe_context *ctx,
>                      blit);
>  }
>  
> +static void virgl_set_shader_buffers(struct pipe_context *ctx,
> +                                     enum pipe_shader_type shader,
> +                                     unsigned start_slot, unsigned
> count,
> +                                     const struct pipe_shader_buffer
> *buffers)
> +{
> +   struct virgl_context *vctx = virgl_context(ctx);
> +   struct virgl_screen *rs = virgl_screen(ctx->screen);
> +
> +   for (unsigned i = 0; i < count; i++) {
> +      unsigned idx = start_slot + i;
> +
> +      if (buffers) {
> +         if (buffers[i].buffer) {
> +            pipe_resource_reference(&vctx->ssbos[shader][idx],
> buffers[i].buffer);
> +            continue;
> +         }
> +      }
> +      pipe_resource_reference(&vctx->ssbos[shader][idx], NULL);
> +   }
> +
> +   uint32_t max_shader_buffer = shader == PIPE_SHADER_FRAGMENT ?
> +      rs->caps.caps.v2.max_shader_buffer_frag_compute :
Shouldn't max_shader_buffer_frag_compute also be used for
PIPE_SHADER_COMPUTE? 

> +      rs->caps.caps.v2.max_shader_buffer_other_stages;
> +   if (!max_shader_buffer)
> +      return;
> +   virgl_encode_set_shader_buffers(vctx, shader, start_slot, count,
> buffers);
> +}
> +
>  static void
>  virgl_context_destroy( struct pipe_context *ctx )
>  {
> @@ -1048,6 +1091,7 @@ struct pipe_context
> *virgl_context_create(struct pipe_screen *pscreen,
>     vctx->base.flush_resource = virgl_flush_resource;
>     vctx->base.blit =  virgl_blit;
>  
> +   vctx->base.set_shader_buffers = virgl_set_shader_buffers;
>     virgl_init_context_resource_functions(&vctx->base);
>     virgl_init_query_functions(vctx);
>     virgl_init_so_functions(vctx);
> diff --git a/src/gallium/drivers/virgl/virgl_context.h
> b/src/gallium/drivers/virgl/virgl_context.h
> index 3492dcfa494..5747654ea82 100644
> --- a/src/gallium/drivers/virgl/virgl_context.h
> +++ b/src/gallium/drivers/virgl/virgl_context.h
> @@ -68,6 +68,8 @@ struct virgl_context {
>     unsigned num_so_targets;
>  
>     struct pipe_resource
> *ubos[PIPE_SHADER_TYPES][PIPE_MAX_CONSTANT_BUFFERS];
> +
> +   struct pipe_resource
> *ssbos[PIPE_SHADER_TYPES][PIPE_MAX_SHADER_BUFFERS];
>     int num_transfers;
>     int num_draws;
>     struct list_head to_flush_bufs;
> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> index c1af01b6fdf..b09366dcee6 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.c
> +++ b/src/gallium/drivers/virgl/virgl_encode.c
> @@ -918,3 +918,28 @@ int virgl_encode_set_tess_state(struct
> virgl_context *ctx,
>        virgl_encoder_write_dword(ctx->cbuf, fui(inner[i]));
>     return 0;
>  }
> +
> +int virgl_encode_set_shader_buffers(struct virgl_context *ctx,
> +                                    enum pipe_shader_type shader,
> +                                    unsigned start_slot, unsigned
> count,
> +                                    const struct pipe_shader_buffer
> *buffers)
> +{
> +   int i;
> +   virgl_encoder_write_cmd_dword(ctx,
> VIRGL_CMD0(VIRGL_CCMD_SET_SHADER_BUFFERS, 0,
> VIRGL_SET_SHADER_BUFFER_SIZE(count)));
> +
> +   virgl_encoder_write_dword(ctx->cbuf, shader);
> +   virgl_encoder_write_dword(ctx->cbuf, start_slot);
> +   for (i = 0; i < count; i++) {
> +      if (buffers) {
Is it legal to have (count > 0) but buffers= NULL? 

> +         struct virgl_resource *res =
> virgl_resource(buffers[i].buffer);
> +         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);
> +      } else {
> +         virgl_encoder_write_dword(ctx->cbuf, 0);
> +         virgl_encoder_write_dword(ctx->cbuf, 0);
> +         virgl_encoder_write_dword(ctx->cbuf, 0);
> +      }
> +   }
> +   return 0;
> +}
> diff --git a/src/gallium/drivers/virgl/virgl_encode.h
> b/src/gallium/drivers/virgl/virgl_encode.h
> index 21c506eb56f..3221fcbcd0c 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.h
> +++ b/src/gallium/drivers/virgl/virgl_encode.h
> @@ -258,4 +258,9 @@ int virgl_encode_bind_shader(struct virgl_context
> *ctx,
>  int virgl_encode_set_tess_state(struct virgl_context *ctx,
>                                  const float outer[4],
>                                  const float inner[2]);
> +
> +int virgl_encode_set_shader_buffers(struct virgl_context *ctx,
> +                                    enum pipe_shader_type shader,
> +                                    unsigned start_slot, unsigned
> count,
> +                                    const struct pipe_shader_buffer
> *buffers);
>  #endif
> diff --git a/src/gallium/drivers/virgl/virgl_hw.h
> b/src/gallium/drivers/virgl/virgl_hw.h
> index f501da7cc13..bc3e6a22472 100644
> --- a/src/gallium/drivers/virgl/virgl_hw.h
> +++ b/src/gallium/drivers/virgl/virgl_hw.h
> @@ -211,6 +211,7 @@ enum virgl_formats {
>  #define VIRGL_BIND_CONSTANT_BUFFER (1 << 6)
>  #define VIRGL_BIND_DISPLAY_TARGET (1 << 7)
>  #define VIRGL_BIND_STREAM_OUTPUT (1 << 11)
> +#define VIRGL_BIND_SHADER_BUFFER (1 << 14)
>  #define VIRGL_BIND_CURSOR        (1 << 16)
>  #define VIRGL_BIND_CUSTOM        (1 << 17)
>  #define VIRGL_BIND_SCANOUT       (1 << 18)
> @@ -302,6 +303,8 @@ struct virgl_caps_v2 {
>          uint32_t capability_bits;
>          uint32_t msaa_sample_positions[8];
>          uint32_t max_vertex_attrib_stride;
> +        uint32_t max_shader_buffer_frag_compute;
> +        uint32_t max_shader_buffer_other_stages;
On the host you use a combined value
  uint32_t shader_storage_buffer_object_maximums;
Maybe using two uint16_t on both sides would be better? 


>  };
>  
>  union virgl_caps {
> diff --git a/src/gallium/drivers/virgl/virgl_protocol.h
> b/src/gallium/drivers/virgl/virgl_protocol.h
> index 53493d5c156..e6249b9cc42 100644
> --- a/src/gallium/drivers/virgl/virgl_protocol.h
> +++ b/src/gallium/drivers/virgl/virgl_protocol.h
> @@ -86,6 +86,7 @@ enum virgl_context_cmd {
>  
>     VIRGL_CCMD_SET_TESS_STATE,
>     VIRGL_CCMD_SET_MIN_SAMPLES,
> +   VIRGL_CCMD_SET_SHADER_BUFFERS,
>  };
>  
>  /*
> @@ -491,4 +492,13 @@ enum virgl_context_cmd {
>  #define VIRGL_SET_MIN_SAMPLES_SIZE 1
>  #define VIRGL_SET_MIN_SAMPLES_MASK 1
>  
> +/* set shader buffers */
> +#define VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE 3
> +#define VIRGL_SET_SHADER_BUFFER_SIZE(x)
> (VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE * (x)) + 2
> +#define VIRGL_SET_SHADER_BUFFER_SHADER_TYPE 1
> +#define VIRGL_SET_SHADER_BUFFER_START_SLOT 2
> +#define VIRGL_SET_SHADER_BUFFER_OFFSET(x) ((x) *
> VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE + 2)
> +#define VIRGL_SET_SHADER_BUFFER_LENGTH(x) ((x) *
> VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE + 3)
> +#define VIRGL_SET_SHADER_BUFFER_RES_HANDLE(x) ((x) *
> VIRGL_SET_SHADER_BUFFER_ELEMENT_SIZE + 4)
> +
>  #endif
> diff --git a/src/gallium/drivers/virgl/virgl_resource.h
> b/src/gallium/drivers/virgl/virgl_resource.h
> index bab9bcb9b4e..297bc72e198 100644
> --- a/src/gallium/drivers/virgl/virgl_resource.h
> +++ b/src/gallium/drivers/virgl/virgl_resource.h
> @@ -134,6 +134,8 @@ static inline unsigned
> pipe_to_virgl_bind(unsigned pbind)
>        outbind |= VIRGL_BIND_CUSTOM;
>     if (pbind & PIPE_BIND_SCANOUT)
>        outbind |= VIRGL_BIND_SCANOUT;
> +   if (pbind & PIPE_BIND_SHADER_BUFFER)
> +      outbind |= VIRGL_BIND_SHADER_BUFFER;
>     return outbind;
>  }
>  
> diff --git a/src/gallium/drivers/virgl/virgl_screen.c
> b/src/gallium/drivers/virgl/virgl_screen.c
> index e32d454d19b..da1daf0c099 100644
> --- a/src/gallium/drivers/virgl/virgl_screen.c
> +++ b/src/gallium/drivers/virgl/virgl_screen.c
> @@ -369,6 +369,11 @@ virgl_get_shader_param(struct pipe_screen
> *screen,
>           return 32;
>        case PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE:
>           return 4096 * sizeof(float[4]);
> +      case PIPE_SHADER_CAP_MAX_SHADER_BUFFERS:
> +         if (shader == PIPE_SHADER_FRAGMENT)
> +            return vscreen-
> >caps.caps.v2.max_shader_buffer_frag_compute;
Same like above: PIPE_SHADER_COMPUTE?
> +         else
> +            return vscreen-
> >caps.caps.v2.max_shader_buffer_other_stages;
>        case PIPE_SHADER_CAP_LOWER_IF_THRESHOLD:
>        case PIPE_SHADER_CAP_TGSI_SKIP_MERGE_REGISTERS:
>        case PIPE_SHADER_CAP_INT64_ATOMICS:


More information about the mesa-dev mailing list