Hi,
One more small think here:
> +int virgl_encode_set_hw_atomic_buffers(struct virgl_context *ctx,
> + unsigned start_slot, unsigned
count,
> + const struct pipe_shader_buffer
*buffers)
> +{
> + int i;
I believe that it will be better to use 'unsigned' type for 'i' here
because the 'count' variable has 'unsigned' type.
Please lat me know if I am incorrect.
Regars,
Andrii.
On Fri, Aug 31, 2018 at 8:35 PM Gurchetan Singh <gurchetansi...@chromium.org>
wrote:
> On Thu, Aug 30, 2018 at 6:41 AM Erik Faye-Lund
> <erik.faye-l...@collabora.com> wrote:
> >
> > From: Tomeu Vizoso <tomeu.viz...@collabora.com>
> >
> > Emulating atomics on top of ssbos can lead to too small max SSBO count,
> > so let's use the hw-atomics mechanism to expose atomic buffers instead.
> >
> > Signed-off-by: Erik Faye-Lund <erik.faye-l...@collabora.com>
> > ---
> > src/gallium/drivers/virgl/virgl_context.c | 37 ++++++++++++++++++++++
> > src/gallium/drivers/virgl/virgl_context.h | 2 ++
> > src/gallium/drivers/virgl/virgl_encode.c | 23 ++++++++++++++
> > src/gallium/drivers/virgl/virgl_encode.h | 3 ++
> > src/gallium/drivers/virgl/virgl_hw.h | 5 +++
> > src/gallium/drivers/virgl/virgl_protocol.h | 9 ++++++
> > src/gallium/drivers/virgl/virgl_screen.c | 14 +++++---
> > 7 files changed, 88 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/gallium/drivers/virgl/virgl_context.c
> b/src/gallium/drivers/virgl/virgl_context.c
> > index edc03f5dcf..30cd0e4331 100644
> > --- a/src/gallium/drivers/virgl/virgl_context.c
> > +++ b/src/gallium/drivers/virgl/virgl_context.c
> > @@ -196,6 +196,19 @@ static void virgl_attach_res_shader_images(struct
> virgl_context *vctx,
> > }
> > }
> >
> > +static void virgl_attach_res_atomic_buffers(struct virgl_context *vctx)
> > +{
> > + 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++) {
>
> Why not PIPE_MAX_HW_ATOMIC_BUFFERS?
>
> > + res = virgl_resource(vctx->atomic_buffers[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.
> > @@ -214,6 +227,7 @@ static void virgl_reemit_res(struct virgl_context
> *vctx)
> > virgl_attach_res_shader_buffers(vctx, shader_type);
> > virgl_attach_res_shader_images(vctx, shader_type);
> > }
> > + virgl_attach_res_atomic_buffers(vctx);
> > virgl_attach_res_vertex_buffers(vctx);
> > virgl_attach_res_so_targets(vctx);
> > }
> > @@ -952,6 +966,28 @@ static void virgl_blit(struct pipe_context *ctx,
> > blit);
> > }
> >
> > +static void virgl_set_hw_atomic_buffers(struct pipe_context *ctx,
> > + unsigned start_slot,
> > + unsigned count,
> > + const struct
> pipe_shader_buffer *buffers)
>
> nit: mixing tabs and spaces
>
> > +{
> > + struct virgl_context *vctx = virgl_context(ctx);
> > +
> > + for (unsigned i = 0; i < count; i++) {
> > + unsigned idx = start_slot + i;
> > +
> > + if (buffers) {
> > + if (buffers[i].buffer) {
> > + pipe_resource_reference(&vctx->atomic_buffers[idx],
> > + buffers[i].buffer);
> > + continue;
> > + }
> > + }
> > + pipe_resource_reference(&vctx->atomic_buffers[idx], NULL);
> > + }
> > + virgl_encode_set_hw_atomic_buffers(vctx, start_slot, count, buffers);
> > +}
> > +
> > static void virgl_set_shader_buffers(sunsignedtruct pipe_context *ctx,
> > enum pipe_shader_type shader,
> > unsigned start_slot, unsigned
> count,
> > @@ -1209,6 +1245,7 @@ struct pipe_context *virgl_context_create(struct
> pipe_screen *pscreen,
> > vctx->base.blit = virgl_blit;
> >
> > vctx->base.set_shader_buffers = virgl_set_shader_buffers;
> > + vctx->base.set_hw_atomic_buffers = virgl_set_hw_atomic_buffers;
> > vctx->base.set_shader_images = virgl_set_shader_images;
> > vctx->base.memory_barrier = virgl_memory_barrier;
> >
> > diff --git a/src/gallium/drivers/virgl/virgl_context.h
> b/src/gallium/drivers/virgl/virgl_context.h
> > index 38d1f450e1..20988baa3c 100644
> > --- a/src/gallium/drivers/virgl/virgl_context.h
> > +++ b/src/gallium/drivers/virgl/virgl_context.h
> > @@ -75,6 +75,8 @@ struct virgl_context {
> > int num_draws;
> > struct list_head to_flush_bufs;
> >
> > + struct pipe_resource *atomic_buffers[PIPE_MAX_HW_ATOMIC_BUFFERS];
> > +
> > struct primconvert_context *primconvert;
> > uint32_t hw_sub_ctx_id;
> > };
> > diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> > index bcb14d8939..c9cc099061 100644
> > --- a/src/gallium/drivers/virgl/virgl_encode.c
> > +++ b/src/gallium/drivers/virgl/virgl_encode.c
> > @@ -958,6 +958,29 @@ int virgl_encode_set_shader_buffers(struct
> virgl_context *ctx,
> > return 0;
> > }
> >
> > +int virgl_encode_set_hw_atomic_buffers(struct virgl_context *ctx,
> > + unsigned start_slot, unsigned
> count,
> > + const struct pipe_shader_buffer
> *buffers)
> > +{
> > + int i;
> > + virgl_encoder_write_cmd_dword(ctx,
> VIRGL_CMD0(VIRGL_CCMD_SET_ATOMIC_BUFFERS, 0,
> VIRGL_SET_ATOMIC_BUFFER_SIZE(count)));
> > +
> > + virgl_encoder_write_dword(ctx->cbuf, start_slot);
> > + for (i = 0; i < count; i++) {
> > + if (buffers) {
> > + 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);
>
> nit: mixing tabs and spaces
>
> > + } else {
> > + virgl_encoder_write_dword(ctx->cbuf, 0);
> > + virgl_encoder_write_dword(ctx->cbuf, 0);
> > + virgl_encoder_write_dword(ctx->cbuf, 0);
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > int virgl_encode_set_shader_images(struct virgl_context *ctx,
> > enum pipe_shader_type shader,
> > unsigned start_slot, unsigned count,
> > diff --git a/src/gallium/drivers/virgl/virgl_encode.h
> b/src/gallium/drivers/virgl/virgl_encode.h
> > index 999123f426..40e62d453b 100644
> > --- a/src/gallium/drivers/virgl/virgl_encode.h
> > +++ b/src/gallium/drivers/virgl/virgl_encode.h
> > @@ -267,6 +267,9 @@ int virgl_encode_set_shader_images(struct
> virgl_context *ctx,
> > enum pipe_shader_type shader,
> > unsigned start_slot, unsigned count,
> > const struct pipe_image_view
> *images);
> > +int virgl_encode_set_hw_atomic_buffers(struct virgl_context *ctx,
> > + unsigned start_slot, unsigned
> count,
> > + const struct pipe_shader_buffer
> *buffers);
> > int virgl_encode_memory_barrier(struct virgl_context *ctx,
> > unsigned flags);
> > int virgl_encode_launch_grid(struct virgl_context *ctx,
> > diff --git a/src/gallium/drivers/virgl/virgl_hw.h
> b/src/gallium/drivers/virgl/virgl_hw.h
> > index 9a358dd844..7736ceb935 100644
> > --- a/src/gallium/drivers/virgl/virgl_hw.h
> > +++ b/src/gallium/drivers/virgl/virgl_hw.h
> > @@ -351,6 +351,11 @@ struct virgl_caps_v2 {
> > uint32_t max_texture_2d_size;
> > uint32_t max_texture_3d_size;
> > uint32_t max_texture_cube_size;
> > + uint32_t max_combined_shader_buffers;
> > + uint32_t max_atomic_counters[6];
> > + uint32_t max_atomic_counter_buffers[6];
> > + uint32_t max_combined_atomic_counters;
> > + uint32_t max_combined_atomic_counter_buffers;
> > };
> >
> > union virgl_caps {
> > diff --git a/src/gallium/drivers/virgl/virgl_protocol.h
> b/src/gallium/drivers/virgl/virgl_protocol.h
> > index dd1a4ee54d..8d99c5ed47 100644
> > --- a/src/gallium/drivers/virgl/virgl_protocol.h
> > +++ b/src/gallium/drivers/virgl/virgl_protocol.h
> > @@ -91,6 +91,7 @@ enum virgl_context_cmd {
> > VIRGL_CCMD_LAUNCH_GRID,
> > VIRGL_CCMD_SET_FRAMEBUFFER_STATE_NO_ATTACH,
> > VIRGL_CCMD_TEXTURE_BARRIER,
> > + VIRGL_CCMD_SET_ATOMIC_BUFFERS,
> > };
> >
> > /*
> > @@ -544,4 +545,12 @@ enum virgl_context_cmd {
> > #define VIRGL_TEXTURE_BARRIER_SIZE 1
> > #define VIRGL_TEXTURE_BARRIER_FLAGS 1
> >
> > +/* hw atomics */
> > +#define VIRGL_SET_ATOMIC_BUFFER_ELEMENT_SIZE 3
> > +#define VIRGL_SET_ATOMIC_BUFFER_SIZE(x)
> (VIRGL_SET_ATOMIC_BUFFER_ELEMENT_SIZE * (x)) + 1
> > +#define VIRGL_SET_ATOMIC_BUFFER_START_SLOT 1
> > +#define VIRGL_SET_ATOMIC_BUFFER_OFFSET(x) ((x) *
> VIRGL_SET_ATOMIC_BUFFER_ELEMENT_SIZE + 2)
> > +#define VIRGL_SET_ATOMIC_BUFFER_LENGTH(x) ((x) *
> VIRGL_SET_ATOMIC_BUFFER_ELEMENT_SIZE + 3)
> > +#define VIRGL_SET_ATOMIC_BUFFER_RES_HANDLE(x) ((x) *
> VIRGL_SET_ATOMIC_BUFFER_ELEMENT_SIZE + 4)
> > +
> > #endif
> > diff --git a/src/gallium/drivers/virgl/virgl_screen.c
> b/src/gallium/drivers/virgl/virgl_screen.c
> > index 1a9e4bc383..4bef15045a 100644
> > --- a/src/gallium/drivers/virgl/virgl_screen.c
> > +++ b/src/gallium/drivers/virgl/virgl_screen.c
> > @@ -247,6 +247,10 @@ virgl_get_param(struct pipe_screen *screen, enum
> pipe_cap param)
> > return vscreen->caps.caps.v2.capability_bits &
> VIRGL_CAP_SHADER_CLOCK;
> > case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
> > return vscreen->caps.caps.v2.capability_bits &
> VIRGL_CAP_TGSI_COMPONENTS;
> > + case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
> > + return vscreen->caps.caps.v2.max_combined_atomic_counters;
> > + case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
> > + return
> MIN2(vscreen->caps.caps.v2.max_combined_atomic_counter_buffers, 15 * 6);
> > case PIPE_CAP_TEXTURE_GATHER_SM5:
> > case PIPE_CAP_BUFFER_MAP_PERSISTENT_COHERENT:
> > case PIPE_CAP_FAKE_SW_MSAA:
> > @@ -316,15 +320,13 @@ virgl_get_param(struct pipe_screen *screen, enum
> pipe_cap param)
> > case PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE:
> > case PIPE_CAP_MAX_CONSERVATIVE_RASTER_SUBPIXEL_PRECISION_BIAS:
> > case PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS:
> > - case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
> > - case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
> > return 0;
> > case PIPE_CAP_MAX_GS_INVOCATIONS:
> > return 32;
> > case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
> > return 1 << 27;
> > case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
> > - return 0;
> > + return MIN2(vscreen->caps.caps.v2.max_combined_shader_buffers, 16
> * 6);
>
> nit: lose the MIN2, maybe we want to hit the asserts in st_init_limits
> if there's incorrect reporting on the host.
>
> > case PIPE_CAP_VENDOR_ID:
> > return 0x1af4;
> > case PIPE_CAP_DEVICE_ID:
> > @@ -414,12 +416,14 @@ virgl_get_shader_param(struct pipe_screen *screen,
> > return vscreen->caps.caps.v2.max_shader_image_other_stages;
> > case PIPE_SHADER_CAP_SUPPORTED_IRS:
> > return (1 << PIPE_SHADER_IR_TGSI);
> > + case PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTERS:
> > + return vscreen->caps.caps.v2.max_atomic_counters[shader];
> > + case PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTER_BUFFERS:
> > + return
> vscreen->caps.caps.v2.max_atomic_counter_buffers[shader];
> > case PIPE_SHADER_CAP_LOWER_IF_THRESHOLD:
> > case PIPE_SHADER_CAP_TGSI_SKIP_MERGE_REGISTERS:
> > case PIPE_SHADER_CAP_INT64_ATOMICS:
> > case PIPE_SHADER_CAP_FP16:
> > - case PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTERS:
> > - case PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTER_BUFFERS:
> > return 0;
> > case PIPE_SHADER_CAP_SCALAR_ISA:
> > return 1;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
