[Mesa-dev] [PATCH 2/2] gallium/u_index_modify: don't add PIPE_TRANSFER_UNSYNCHRONIZED unconditionally

Nicolai Hähnle nhaehnle at gmail.com
Sun Feb 19 09:07:25 UTC 2017


On 17.02.2017 13:10, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> It's OK for r300g (because r300g can't write to buffers via the GPU), but
> not later hardware. This issue was spotted randomly.
>
> Cc: mesa-stable at lists.freedesktop.org

Both patches:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

> ---
>  src/gallium/auxiliary/util/u_index_modify.c      | 9 ++++++---
>  src/gallium/auxiliary/util/u_index_modify.h      | 3 +++
>  src/gallium/drivers/r300/r300_render_translate.c | 4 +++-
>  src/gallium/drivers/r600/r600_state_common.c     | 2 +-
>  src/gallium/drivers/radeonsi/si_state_draw.c     | 2 +-
>  5 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_index_modify.c b/src/gallium/auxiliary/util/u_index_modify.c
> index 5c4fc3c..7b072b2 100644
> --- a/src/gallium/auxiliary/util/u_index_modify.c
> +++ b/src/gallium/auxiliary/util/u_index_modify.c
> @@ -21,102 +21,105 @@
>   * USE OR OTHER DEALINGS IN THE SOFTWARE. */
>
>  #include "pipe/p_context.h"
>  #include "util/u_index_modify.h"
>  #include "util/u_inlines.h"
>
>  /* Ubyte indices. */
>
>  void util_shorten_ubyte_elts_to_userptr(struct pipe_context *context,
>  					struct pipe_index_buffer *ib,
> +                                        unsigned add_transfer_flags,
>  					int index_bias,
>  					unsigned start,
>  					unsigned count,
>  					void *out)
>  {
>      struct pipe_transfer *src_transfer = NULL;
>      const unsigned char *in_map;
>      unsigned short *out_map = out;
>      unsigned i;
>
>      if (ib->user_buffer) {
>         in_map = ib->user_buffer;
>      } else {
>         in_map = pipe_buffer_map(context, ib->buffer,
>                                  PIPE_TRANSFER_READ |
> -                                PIPE_TRANSFER_UNSYNCHRONIZED,
> +                                add_transfer_flags,
>                                  &src_transfer);
>      }
>      in_map += start;
>
>      for (i = 0; i < count; i++) {
>          *out_map = (unsigned short)(*in_map + index_bias);
>          in_map++;
>          out_map++;
>      }
>
>      if (src_transfer)
>         pipe_buffer_unmap(context, src_transfer);
>  }
>
>  /* Ushort indices. */
>
>  void util_rebuild_ushort_elts_to_userptr(struct pipe_context *context,
>  					 struct pipe_index_buffer *ib,
> +                                         unsigned add_transfer_flags,
>  					 int index_bias,
>  					 unsigned start, unsigned count,
>  					 void *out)
>  {
>      struct pipe_transfer *in_transfer = NULL;
>      const unsigned short *in_map;
>      unsigned short *out_map = out;
>      unsigned i;
>
>      if (ib->user_buffer) {
>         in_map = ib->user_buffer;
>      } else {
>         in_map = pipe_buffer_map(context, ib->buffer,
>                                  PIPE_TRANSFER_READ |
> -                                PIPE_TRANSFER_UNSYNCHRONIZED,
> +                                add_transfer_flags,
>                                  &in_transfer);
>      }
>      in_map += start;
>
>      for (i = 0; i < count; i++) {
>          *out_map = (unsigned short)(*in_map + index_bias);
>          in_map++;
>          out_map++;
>      }
>
>      if (in_transfer)
>         pipe_buffer_unmap(context, in_transfer);
>  }
>
>  /* Uint indices. */
>
>  void util_rebuild_uint_elts_to_userptr(struct pipe_context *context,
>  				       struct pipe_index_buffer *ib,
> +                                       unsigned add_transfer_flags,
>  				       int index_bias,
>  				       unsigned start, unsigned count,
>  				       void *out)
>  {
>      struct pipe_transfer *in_transfer = NULL;
>      const unsigned int *in_map;
>      unsigned int *out_map = out;
>      unsigned i;
>
>      if (ib->user_buffer) {
>         in_map = ib->user_buffer;
>      } else {
>         in_map = pipe_buffer_map(context, ib->buffer,
>                                  PIPE_TRANSFER_READ |
> -                                PIPE_TRANSFER_UNSYNCHRONIZED,
> +                                add_transfer_flags,
>                                  &in_transfer);
>      }
>      in_map += start;
>
>      for (i = 0; i < count; i++) {
>          *out_map = (unsigned int)(*in_map + index_bias);
>          in_map++;
>          out_map++;
>      }
>
> diff --git a/src/gallium/auxiliary/util/u_index_modify.h b/src/gallium/auxiliary/util/u_index_modify.h
> index 1d34b12..0cfc189 100644
> --- a/src/gallium/auxiliary/util/u_index_modify.h
> +++ b/src/gallium/auxiliary/util/u_index_modify.h
> @@ -22,28 +22,31 @@
>
>  #ifndef UTIL_INDEX_MODIFY_H
>  #define UTIL_INDEX_MODIFY_H
>
>  struct pipe_context;
>  struct pipe_resource;
>  struct pipe_index_buffer;
>
>  void util_shorten_ubyte_elts_to_userptr(struct pipe_context *context,
>  					struct pipe_index_buffer *ib,
> +                                        unsigned add_transfer_flags,
>  					int index_bias,
>  					unsigned start,
>  					unsigned count,
>  					void *out);
>
>  void util_rebuild_ushort_elts_to_userptr(struct pipe_context *context,
>  					 struct pipe_index_buffer *ib,
> +                                         unsigned add_transfer_flags,
>  					 int index_bias,
>  					 unsigned start, unsigned count,
>  					 void *out);
>
>  void util_rebuild_uint_elts_to_userptr(struct pipe_context *context,
>  				       struct pipe_index_buffer *ib,
> +                                       unsigned add_transfer_flags,
>  				       int index_bias,
>  				       unsigned start, unsigned count,
>  				       void *out);
>
>  #endif
> diff --git a/src/gallium/drivers/r300/r300_render_translate.c b/src/gallium/drivers/r300/r300_render_translate.c
> index 7221211..7800f6e 100644
> --- a/src/gallium/drivers/r300/r300_render_translate.c
> +++ b/src/gallium/drivers/r300/r300_render_translate.c
> @@ -34,46 +34,48 @@ void r300_translate_index_buffer(struct r300_context *r300,
>      unsigned out_offset;
>      void *ptr;
>
>      switch (*index_size) {
>      case 1:
>          *out_buffer = NULL;
>          u_upload_alloc(r300->uploader, 0, count * 2, 4,
>                         &out_offset, out_buffer, &ptr);
>
>          util_shorten_ubyte_elts_to_userptr(
> -                &r300->context, ib, index_offset,
> +                &r300->context, ib, PIPE_TRANSFER_UNSYNCHRONIZED, index_offset,
>                  *start, count, ptr);
>
>          *index_size = 2;
>          *start = out_offset / 2;
>          break;
>
>      case 2:
>          if (index_offset) {
>              *out_buffer = NULL;
>              u_upload_alloc(r300->uploader, 0, count * 2, 4,
>                             &out_offset, out_buffer, &ptr);
>
>              util_rebuild_ushort_elts_to_userptr(&r300->context, ib,
> +                                                PIPE_TRANSFER_UNSYNCHRONIZED,
>                                                  index_offset, *start,
>                                                  count, ptr);
>
>              *start = out_offset / 2;
>          }
>          break;
>
>      case 4:
>          if (index_offset) {
>              *out_buffer = NULL;
>              u_upload_alloc(r300->uploader, 0, count * 4, 4,
>                             &out_offset, out_buffer, &ptr);
>
>              util_rebuild_uint_elts_to_userptr(&r300->context, ib,
> +                                              PIPE_TRANSFER_UNSYNCHRONIZED,
>                                                index_offset, *start,
>                                                count, ptr);
>
>              *start = out_offset / 4;
>          }
>          break;
>      }
>  }
> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> index 9ff2364..1fbe392 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -1736,21 +1736,21 @@ static void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info
>  				else {
>  					start = 0;
>  					count = 0;
>  				}
>  			}
>
>  			u_upload_alloc(ctx->stream_uploader, start, count * 2,
>                                         256, &out_offset, &out_buffer, &ptr);
>
>  			util_shorten_ubyte_elts_to_userptr(
> -						&rctx->b.b, &ib, 0, ib.offset + start, count, ptr);
> +						&rctx->b.b, &ib, 0, 0, ib.offset + start, count, ptr);
>
>  			pipe_resource_reference(&ib.buffer, NULL);
>  			ib.user_buffer = NULL;
>  			ib.buffer = out_buffer;
>  			ib.offset = out_offset;
>  			ib.index_size = 2;
>  		}
>
>  		/* Upload the index buffer.
>  		 * The upload is skipped for small index counts on little-endian machines
> diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c b/src/gallium/drivers/radeonsi/si_state_draw.c
> index b45ef87..6dae904 100644
> --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> @@ -1055,21 +1055,21 @@ void si_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *info)
>  			start_offset = start * 2;
>
>  			u_upload_alloc(ctx->stream_uploader, start_offset,
>                                         count * 2, 256,
>  				       &out_offset, &out_buffer, &ptr);
>  			if (!out_buffer) {
>  				pipe_resource_reference(&ib.buffer, NULL);
>  				return;
>  			}
>
> -			util_shorten_ubyte_elts_to_userptr(&sctx->b.b, &ib, 0,
> +			util_shorten_ubyte_elts_to_userptr(&sctx->b.b, &ib, 0, 0,
>  							   ib.offset + start,
>  							   count, ptr);
>
>  			pipe_resource_reference(&ib.buffer, NULL);
>  			ib.user_buffer = NULL;
>  			ib.buffer = out_buffer;
>  			/* info->start will be added by the drawing code */
>  			ib.offset = out_offset - start_offset;
>  			ib.index_size = 2;
>  		} else if (ib.user_buffer && !ib.buffer) {
>



More information about the mesa-dev mailing list