[Mesa-dev] [PATCH v2] radeonsi: fix query buffer allocation

[Marek Olšák]

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Sun, Feb 24, 2019 at 6:56 PM Timothy Arceri <tarceri at itsqueeze.com>
wrote:

> Fix the logic for buffer full check on alloc.
>
> This patch just takes the fix Nicolai attached to the bug report
> and updates it to work on master.
>
> Fixes: e0f0d3675d4 ("radeonsi: factor si_query_buffer logic out of
> si_query_hw")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109561
> ---
>  src/gallium/drivers/radeonsi/si_query.c | 52 ++++++++++++++-----------
>  src/gallium/drivers/radeonsi/si_query.h |  5 ++-
>  2 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_query.c
> b/src/gallium/drivers/radeonsi/si_query.c
> index 266b9d3ce84..280eee3a280 100644
> --- a/src/gallium/drivers/radeonsi/si_query.c
> +++ b/src/gallium/drivers/radeonsi/si_query.c
> @@ -549,11 +549,15 @@ void si_query_buffer_reset(struct si_context *sctx,
> struct si_query_buffer *buff
>         }
>         buffer->results_end = 0;
>
> +       if (!buffer->buf)
> +               return;
> +
>         /* Discard even the oldest buffer if it can't be mapped without a
> stall. */
> -       if (buffer->buf &&
> -           (si_rings_is_buffer_referenced(sctx, buffer->buf->buf,
> RADEON_USAGE_READWRITE) ||
> -            !sctx->ws->buffer_wait(buffer->buf->buf, 0,
> RADEON_USAGE_READWRITE))) {
> +       if (si_rings_is_buffer_referenced(sctx, buffer->buf->buf,
> RADEON_USAGE_READWRITE) ||
> +           !sctx->ws->buffer_wait(buffer->buf->buf, 0,
> RADEON_USAGE_READWRITE)) {
>                 si_resource_reference(&buffer->buf, NULL);
> +       } else {
> +               buffer->unprepared = true;
>         }
>  }
>
> @@ -561,29 +565,31 @@ bool si_query_buffer_alloc(struct si_context *sctx,
> struct si_query_buffer *buff
>                            bool (*prepare_buffer)(struct si_context *,
> struct si_query_buffer*),
>                            unsigned size)
>  {
> -       if (buffer->buf && buffer->results_end + size >=
> buffer->buf->b.b.width0)
> -               return true;
> +       bool unprepared = buffer->unprepared;
> +       buffer->unprepared = false;
> +
> +       if (!buffer->buf || buffer->results_end + size >
> buffer->buf->b.b.width0) {
> +               if (buffer->buf) {
> +                       struct si_query_buffer *qbuf =
> MALLOC_STRUCT(si_query_buffer);
> +                       memcpy(qbuf, buffer, sizeof(*qbuf));
> +                       buffer->previous = qbuf;
> +               }
> +               buffer->results_end = 0;
>
> -       if (buffer->buf) {
> -               struct si_query_buffer *qbuf =
> MALLOC_STRUCT(si_query_buffer);
> -               memcpy(qbuf, buffer, sizeof(*qbuf));
> -               buffer->previous = qbuf;
> +               /* Queries are normally read by the CPU after
> +                * being written by the gpu, hence staging is probably a
> good
> +                * usage pattern.
> +                */
> +               struct si_screen *screen = sctx->screen;
> +               unsigned buf_size = MAX2(size,
> screen->info.min_alloc_size);
> +               buffer->buf = si_resource(
> +                       pipe_buffer_create(&screen->b, 0,
> PIPE_USAGE_STAGING, buf_size));
> +               if (unlikely(!buffer->buf))
> +                       return false;
> +               unprepared = true;
>         }
>
> -       buffer->results_end = 0;
> -
> -       /* Queries are normally read by the CPU after
> -        * being written by the gpu, hence staging is probably a good
> -        * usage pattern.
> -        */
> -       struct si_screen *screen = sctx->screen;
> -       unsigned buf_size = MAX2(size, screen->info.min_alloc_size);
> -       buffer->buf = si_resource(
> -               pipe_buffer_create(&screen->b, 0, PIPE_USAGE_STAGING,
> buf_size));
> -       if (unlikely(!buffer->buf))
> -               return false;
> -
> -       if (prepare_buffer) {
> +       if (unprepared && prepare_buffer) {
>                 if (unlikely(!prepare_buffer(sctx, buffer))) {
>                         si_resource_reference(&buffer->buf, NULL);
>                         return false;
> diff --git a/src/gallium/drivers/radeonsi/si_query.h
> b/src/gallium/drivers/radeonsi/si_query.h
> index aaf0bd03aca..c61af51d57c 100644
> --- a/src/gallium/drivers/radeonsi/si_query.h
> +++ b/src/gallium/drivers/radeonsi/si_query.h
> @@ -177,12 +177,13 @@ struct si_query_hw_ops {
>  struct si_query_buffer {
>         /* The buffer where query results are stored. */
>         struct si_resource              *buf;
> -       /* Offset of the next free result after current query data */
> -       unsigned                        results_end;
>         /* If a query buffer is full, a new buffer is created and the old
> one
>          * is put in here. When we calculate the result, we sum up the
> samples
>          * from all buffers. */
>         struct si_query_buffer  *previous;
> +       /* Offset of the next free result after current query data */
> +       unsigned                        results_end;
> +       bool unprepared;
>  };
>
>  void si_query_buffer_destroy(struct si_screen *sctx, struct
> si_query_buffer *buffer);
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190225/fdd5742f/attachment.html>