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

2019-02-25 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek

On Sun, Feb 24, 2019 at 6:56 PM Timothy Arceri 
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(>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(>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(>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(>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 */
> -   unsignedresults_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   

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

2019-02-24 Thread Timothy Arceri
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(>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(>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(>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(>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 */
-   unsignedresults_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 */
+   unsignedresults_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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev