svga: Fix index buffer uploads

In the case of SWTNL and index translation we were uploading index buffers
and then reading out from them using the CPU. Furthermore, when translating
indices we often cached the results with an upload_mgr buffer, causing the
cached indexes to be immediately discarded on the next write to that
upload_mgr buffer.

Fix this by only uploading when we know the index buffer is going to be
used by hardware. If translating, only cache translated indices if the
original buffer was not a user buffer. In the latter case when we're not
caching, use an upload_mgr buffer for the hardware indices.

This means we can also remove the SWTNL hand-crafted index buffer upload
mechanism in favour of the upload_mgr.

Finally avoid using util_upload_index_buffer(). It wastes index buffer
space by trying to make sure that the offset of the indices in the
upload_mgr buffer is larger or equal to the position of the indices in
the source buffer. From what I can tell, the SVGA device does not
require that.

Testing done: Piglit quick. No regressions.

Signed-off-by: Thomas Hellstrom <[email protected]>
Reviewed-by: Brian Paul <[email protected]>

6 files changed, 142 insertions, 172 deletions

diff --git a/src/gallium/drivers/svga/svga_draw.h b/src/gallium/drivers/svga/svga_draw.h
index baefcd94ec8..9d79676d3f9 100644
--- a/src/gallium/drivers/svga/svga_draw.h
+++ b/src/gallium/drivers/svga/svga_draw.h
@@ -64,13 +64,8 @@ svga_hwtnl_draw_arrays(struct svga_hwtnl *hwtnl,
 
 enum pipe_error
 svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
-                               struct pipe_resource *indexBuffer,
-                               unsigned index_size,
-                               int index_bias,
-                               unsigned min_index,
-                               unsigned max_index,
-                               enum pipe_prim_type prim, unsigned start, unsigned count,
-                               unsigned start_instance, unsigned instance_count);
+                               const struct pipe_draw_info *info,
+                               unsigned count);
 
 boolean
 svga_hwtnl_is_buffer_referred(struct svga_hwtnl *hwtnl,
@@ -80,5 +75,4 @@ enum pipe_error svga_hwtnl_flush(struct svga_hwtnl *hwtnl);
 
 void svga_hwtnl_set_index_bias(struct svga_hwtnl *hwtnl, int index_bias);
 
-
 #endif /* SVGA_DRAW_H_ */
diff --git a/src/gallium/drivers/svga/svga_draw_elements.c b/src/gallium/drivers/svga/svga_draw_elements.c
index b1db8710740..b955b2f77e2 100644
--- a/src/gallium/drivers/svga/svga_draw_elements.c
+++ b/src/gallium/drivers/svga/svga_draw_elements.c
@@ -59,33 +59,41 @@
  * \return error code to indicate success failure
  */
 static enum pipe_error
-translate_indices(struct svga_hwtnl *hwtnl, struct pipe_resource *src,
-                  unsigned offset,
-                  enum pipe_prim_type orig_prim, enum pipe_prim_type gen_prim,
+translate_indices(struct svga_hwtnl *hwtnl,
+                  const struct pipe_draw_info *info,
+                  enum pipe_prim_type gen_prim,
                   unsigned orig_nr, unsigned gen_nr,
-                  unsigned index_size,
-                  u_translate_func translate, struct pipe_resource **out_buf)
+                  unsigned gen_size,
+                  u_translate_func translate,
+                  struct pipe_resource **out_buf,
+                  unsigned *out_offset)
 {
    struct pipe_context *pipe = &hwtnl->svga->pipe;
    struct svga_screen *screen = svga_screen(pipe->screen);
-   struct svga_buffer *src_sbuf = svga_buffer(src);
+   struct svga_buffer *src_sbuf = NULL;
    struct pipe_transfer *src_transfer = NULL;
    struct pipe_transfer *dst_transfer = NULL;
-   unsigned size = index_size * gen_nr;
+   const unsigned size = gen_size * gen_nr;
+   const unsigned offset = info->start * info->index_size;
    const void *src_map = NULL;
    struct pipe_resource *dst = NULL;
    void *dst_map = NULL;
 
-   assert(index_size == 2 || index_size == 4);
+   assert(gen_size == 2 || gen_size == 4);
+   if (!info->has_user_indices)
+      src_sbuf = svga_buffer(info->index.resource);
 
-   if (!screen->debug.no_cache_index_buffers) {
+   /* If the draw_info provides us with a buffer rather than a
+    * user pointer, Check to see if we've already translated that buffer
+    */
+   if (src_sbuf && !screen->debug.no_cache_index_buffers) {
       /* Check if we already have a translated index buffer */
       if (src_sbuf->translated_indices.buffer &&
-          src_sbuf->translated_indices.orig_prim == orig_prim &&
+          src_sbuf->translated_indices.orig_prim == info->mode &&
           src_sbuf->translated_indices.new_prim == gen_prim &&
           src_sbuf->translated_indices.offset == offset &&
           src_sbuf->translated_indices.count == orig_nr &&
-          src_sbuf->translated_indices.index_size == index_size) {
+          src_sbuf->translated_indices.index_size == gen_size) {
          pipe_resource_reference(out_buf, src_sbuf->translated_indices.buffer);
          return PIPE_OK;
       }
@@ -96,51 +104,73 @@ translate_indices(struct svga_hwtnl *hwtnl, struct pipe_resource *src,
     */
    u_trim_pipe_prim(gen_prim, &gen_nr);
 
-   size = index_size * gen_nr;
-
-   dst = pipe_buffer_create(pipe->screen,
-                            PIPE_BIND_INDEX_BUFFER, PIPE_USAGE_DEFAULT, size);
-   if (!dst)
-      goto fail;
-
-   src_map = pipe_buffer_map(pipe, src, PIPE_TRANSFER_READ, &src_transfer);
-   if (!src_map)
-      goto fail;
-
-   dst_map = pipe_buffer_map(pipe, dst, PIPE_TRANSFER_WRITE, &dst_transfer);
-   if (!dst_map)
-      goto fail;
+   if (src_sbuf) {
+      /* If we have a source buffer, create a destination buffer in the
+       * hope that we can reuse the translated data later. If not,
+       * we'd probably be better off using the upload buffer.
+       */
+      dst = pipe_buffer_create(pipe->screen,
+                               PIPE_BIND_INDEX_BUFFER, PIPE_USAGE_IMMUTABLE,
+                               size);
+      if (!dst)
+         goto fail;
+
+      dst_map = pipe_buffer_map(pipe, dst, PIPE_TRANSFER_WRITE, &dst_transfer);
+      if (!dst_map)
+         goto fail;
+
+      *out_offset = 0;
+      src_map = pipe_buffer_map(pipe, info->index.resource, PIPE_TRANSFER_READ,
+                                &src_transfer);
+      if (!src_map)
+         goto fail;
+   } else {
+      /* Allocate upload buffer space. Align to the index size. */
+      u_upload_alloc(pipe->stream_uploader, 0, size, gen_size,
+                     out_offset, &dst, &dst_map);
+      if (!dst)
+         goto fail;
+
+      src_map = info->index.user;
+   }
 
    translate((const char *) src_map + offset, 0, 0, gen_nr, 0, dst_map);
 
-   pipe_buffer_unmap(pipe, src_transfer);
-   pipe_buffer_unmap(pipe, dst_transfer);
+   if (src_transfer)
+      pipe_buffer_unmap(pipe, src_transfer);
+
+   if (dst_transfer)
+      pipe_buffer_unmap(pipe, dst_transfer);
+   else
+      u_upload_unmap(pipe->stream_uploader);
 
    *out_buf = dst;
 
-   if (!screen->debug.no_cache_index_buffers) {
+   if (src_sbuf && !screen->debug.no_cache_index_buffers) {
       /* Save the new, translated index buffer in the hope we can use it
        * again in the future.
        */
       pipe_resource_reference(&src_sbuf->translated_indices.buffer, dst);
-      src_sbuf->translated_indices.orig_prim = orig_prim;
+      src_sbuf->translated_indices.orig_prim = info->mode;
       src_sbuf->translated_indices.new_prim = gen_prim;
       src_sbuf->translated_indices.offset = offset;
       src_sbuf->translated_indices.count = orig_nr;
-      src_sbuf->translated_indices.index_size = index_size;
+      src_sbuf->translated_indices.index_size = gen_size;
    }
 
    return PIPE_OK;
 
  fail:
-   if (src_map)
+   if (src_transfer)
       pipe_buffer_unmap(pipe, src_transfer);
 
-   if (dst_map)
+   if (dst_transfer)
       pipe_buffer_unmap(pipe, dst_transfer);
+   else if (dst_map)
+      u_upload_unmap(pipe->stream_uploader);
 
    if (dst)
-      pipe->screen->resource_destroy(pipe->screen, dst);
+      pipe_resource_reference(&dst, NULL);
 
    return PIPE_ERROR_OUT_OF_MEMORY;
 }
@@ -180,12 +210,10 @@ svga_hwtnl_simple_draw_range_elements(struct svga_hwtnl *hwtnl,
 
 enum pipe_error
 svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
-                               struct pipe_resource *index_buffer,
-                               unsigned index_size, int index_bias,
-                               unsigned min_index, unsigned max_index,
-                               enum pipe_prim_type prim, unsigned start, unsigned count,
-                               unsigned start_instance, unsigned instance_count)
+                               const struct pipe_draw_info *info,
+                               unsigned count)
 {
+   struct pipe_context *pipe = &hwtnl->svga->pipe;
    enum pipe_prim_type gen_prim;
    unsigned gen_size, gen_nr;
    enum indices_mode gen_type;
@@ -195,9 +223,9 @@ svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
    SVGA_STATS_TIME_PUSH(svga_sws(hwtnl->svga),
                         SVGA_STATS_TIME_HWTNLDRAWELEMENTS);
 
-   if (svga_need_unfilled_fallback(hwtnl, prim)) {
-      gen_type = u_unfilled_translator(prim,
-                                       index_size,
+   if (svga_need_unfilled_fallback(hwtnl, info->mode)) {
+      gen_type = u_unfilled_translator(info->mode,
+                                       info->index_size,
                                        count,
                                        hwtnl->api_fillmode,
                                        &gen_prim,
@@ -205,8 +233,8 @@ svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
    }
    else {
       gen_type = u_index_translator(svga_hw_prims,
-                                    prim,
-                                    index_size,
+                                    info->mode,
+                                    info->index_size,
                                     count,
                                     hwtnl->api_pv,
                                     hwtnl->hw_pv,
@@ -217,17 +245,36 @@ svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
    if (gen_type == U_TRANSLATE_MEMCPY) {
       /* No need for translation, just pass through to hardware:
        */
+      unsigned start_offset = info->start * info->index_size;
+      struct pipe_resource *index_buffer = NULL;
+      unsigned index_offset;
+
+      if (info->has_user_indices) {
+         u_upload_data(pipe->stream_uploader, 0, count * info->index_size,
+                       info->index_size, (char *) info->index.user + start_offset,
+                       &index_offset, &index_buffer);
+         u_upload_unmap(pipe->stream_uploader);
+         index_offset /= info->index_size;
+      } else {
+         pipe_resource_reference(&index_buffer, info->index.resource);
+         index_offset = info->start;
+      }
+
+      assert(index_buffer != NULL);
+
       ret = svga_hwtnl_simple_draw_range_elements(hwtnl, index_buffer,
-                                                   index_size,
-                                                   index_bias,
-                                                   min_index,
-                                                   max_index,
-                                                   gen_prim, start, count,
-                                                   start_instance,
-                                                   instance_count);
+                                                  info->index_size,
+                                                  info->index_bias,
+                                                  info->min_index,
+                                                  info->max_index,
+                                                  gen_prim, index_offset, count,
+                                                  info->start_instance,
+                                                  info->instance_count);
+      pipe_resource_reference(&index_buffer, NULL);
    }
    else {
       struct pipe_resource *gen_buf = NULL;
+      unsigned gen_offset = 0;
 
       /* Need to allocate a new index buffer and run the translate
        * func to populate it.  Could potentially cache this translated
@@ -236,22 +283,21 @@ svga_hwtnl_draw_range_elements(struct svga_hwtnl *hwtnl,
        * GL though, as index buffers are typically used only once
        * there.
        */
-      ret = translate_indices(hwtnl,
-                              index_buffer,
-                              start * index_size,
-                              prim, gen_prim,
+      ret = translate_indices(hwtnl, info, gen_prim,
                               count, gen_nr, gen_size,
-                              gen_func, &gen_buf);
+                              gen_func, &gen_buf, &gen_offset);
       if (ret == PIPE_OK) {
+         gen_offset /= gen_size;
          ret = svga_hwtnl_simple_draw_range_elements(hwtnl,
                                                      gen_buf,
                                                      gen_size,
-                                                     index_bias,
-                                                     min_index,
-                                                     max_index,
-                                                     gen_prim, 0, gen_nr,
-                                                     start_instance,
-                                                     instance_count);
+                                                     info->index_bias,
+                                                     info->min_index,
+                                                     info->max_index,
+                                                     gen_prim, gen_offset,
+                                                     gen_nr,
+                                                     info->start_instance,
+                                                     info->instance_count);
       }
 
       if (gen_buf) {
diff --git a/src/gallium/drivers/svga/svga_pipe_draw.c b/src/gallium/drivers/svga/svga_pipe_draw.c
index ace1d2cb6f3..5ebd17cf0ea 100644
--- a/src/gallium/drivers/svga/svga_pipe_draw.c
+++ b/src/gallium/drivers/svga/svga_pipe_draw.c
@@ -49,33 +49,20 @@ is_using_flat_shading(const struct svga_context *svga)
 
 static enum pipe_error
 retry_draw_range_elements(struct svga_context *svga,
-                          struct pipe_resource *index_buffer,
-                          unsigned index_size,
-                          int index_bias,
-                          unsigned min_index,
-                          unsigned max_index,
-                          enum pipe_prim_type prim,
-                          unsigned start,
-                          unsigned count,
-                          unsigned start_instance,
-                          unsigned instance_count)
+                          const struct pipe_draw_info *info,
+                          unsigned count)
 {
    enum pipe_error ret;
 
    SVGA_STATS_TIME_PUSH(svga_sws(svga), SVGA_STATS_TIME_DRAWELEMENTS);
 
-   for (unsigned try = 0; try < 2; try++) {
-      ret = svga_hwtnl_draw_range_elements(svga->hwtnl,
-                                           index_buffer, index_size,
-                                           index_bias,
-                                           min_index, max_index,
-                                           prim, start, count,
-                                           start_instance, instance_count);
-      if (ret == PIPE_OK)
-         break;
+   ret = svga_hwtnl_draw_range_elements(svga->hwtnl, info, count);
+   if (ret != PIPE_OK) {
       svga_context_flush(svga, NULL);
+      ret = svga_hwtnl_draw_range_elements(svga->hwtnl, info, count);
    }
 
+   assert (ret == PIPE_OK);
    SVGA_STATS_TIME_POP(svga_sws(svga));
    return ret;
 }
@@ -137,8 +124,6 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
    unsigned count = info->count;
    enum pipe_error ret = 0;
    boolean needed_swtnl;
-   struct pipe_resource *indexbuf =
-      info->has_user_indices ? NULL : info->index.resource;
 
    SVGA_STATS_TIME_PUSH(svga_sws(svga), SVGA_STATS_TIME_DRAWVBO);
 
@@ -148,13 +133,6 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
        svga->curr.rast->templ.cull_face == PIPE_FACE_FRONT_AND_BACK)
       goto done;
 
-   /* Upload a user index buffer. */
-   unsigned index_offset = 0;
-   if (info->index_size && info->has_user_indices &&
-       !util_upload_index_buffer(pipe, info, &indexbuf, &index_offset)) {
-      goto done;
-   }
-
    /*
     * Mark currently bound target surfaces as dirty
     * doesn't really matter if it is done before drawing.
@@ -200,7 +178,7 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
 
       /* Avoid leaking the previous hwtnl bias to swtnl */
       svga_hwtnl_set_index_bias(svga->hwtnl, 0);
-      ret = svga_swtnl_draw_vbo(svga, info, indexbuf, index_offset);
+      ret = svga_swtnl_draw_vbo(svga, info);
    }
    else {
       if (!svga_update_state_retry(svga, SVGA_STATE_HW_DRAW)) {
@@ -209,7 +187,6 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
          pipe_debug_message(&svga->debug.callback, INFO, "%s", msg);
          goto done;
       }
-
       svga_hwtnl_set_fillmode(svga->hwtnl, svga->curr.rast->hw_fillmode);
 
       /** determine if flatshade is to be used after svga_update_state()
@@ -220,23 +197,8 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
                                is_using_flat_shading(svga),
                                svga->curr.rast->templ.flatshade_first);
 
-      if (info->index_size && indexbuf) {
-         unsigned offset;
-
-         assert(index_offset % info->index_size == 0);
-         offset = index_offset / info->index_size;
-
-         ret = retry_draw_range_elements(svga,
-                                         indexbuf,
-                                         info->index_size,
-                                         info->index_bias,
-                                         info->min_index,
-                                         info->max_index,
-                                         info->mode,
-                                         info->start + offset,
-                                         count,
-                                         info->start_instance,
-                                         info->instance_count);
+      if (info->index_size) {
+         ret = retry_draw_range_elements(svga, info, count);
       }
       else {
          ret = retry_draw_arrays(svga, info->mode, info->start, count,
@@ -253,8 +215,6 @@ svga_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info)
    }
 
 done:
-   if (info->index_size && info->index.resource != indexbuf)
-      pipe_resource_reference(&indexbuf, NULL);
    SVGA_STATS_TIME_POP(svga_sws(svga));
 }
 
diff --git a/src/gallium/drivers/svga/svga_swtnl.h b/src/gallium/drivers/svga/svga_swtnl.h
index 0661b718fd3..fc094e51428 100644
--- a/src/gallium/drivers/svga/svga_swtnl.h
+++ b/src/gallium/drivers/svga/svga_swtnl.h
@@ -39,9 +39,7 @@ void svga_destroy_swtnl( struct svga_context *svga );
 
 enum pipe_error
 svga_swtnl_draw_vbo(struct svga_context *svga,
-                    const struct pipe_draw_info *info,
-                    struct pipe_resource *indexbuf,
-                    unsigned index_offset);
+                    const struct pipe_draw_info *info);
 
 
 #endif
diff --git a/src/gallium/drivers/svga/svga_swtnl_backend.c b/src/gallium/drivers/svga/svga_swtnl_backend.c
index 26f8107129e..b6fd07fe346 100644
--- a/src/gallium/drivers/svga/svga_swtnl_backend.c
+++ b/src/gallium/drivers/svga/svga_swtnl_backend.c
@@ -323,36 +323,29 @@ svga_vbuf_render_draw_elements(struct vbuf_render *render,
 {
    struct svga_vbuf_render *svga_render = svga_vbuf_render(render);
    struct svga_context *svga = svga_render->svga;
-   struct pipe_screen *screen = svga->pipe.screen;
    int bias = (svga_render->vbuf_offset - svga_render->vdecl_offset)
       / svga_render->vertex_size;
    boolean ret;
-   size_t size = 2 * nr_indices;
    /* instancing will already have been resolved at this point by 'draw' */
-   const unsigned start_instance = 0;
-   const unsigned instance_count = 1;
+   const struct pipe_draw_info info = {
+      .index_size = 2,
+      .mode = svga_render->prim,
+      .has_user_indices = 1,
+      .index.user = indices,
+      .start_instance = 0,
+      .instance_count = 1,
+      .index_bias = bias,
+      .min_index = svga_render->min_index,
+      .max_index = svga_render->max_index,
+      .start = 0,
+      .count = nr_indices
+   };
 
    assert((svga_render->vbuf_offset - svga_render->vdecl_offset)
           % svga_render->vertex_size == 0);
 
    SVGA_STATS_TIME_PUSH(svga_sws(svga), SVGA_STATS_TIME_VBUFDRAWELEMENTS);
 
-   if (svga_render->ibuf_size < svga_render->ibuf_offset + size)
-      pipe_resource_reference(&svga_render->ibuf, NULL);
-
-   if (!svga_render->ibuf) {
-      svga_render->ibuf_size = MAX2(size, svga_render->ibuf_alloc_size);
-      svga_render->ibuf = pipe_buffer_create(screen,
-                                             PIPE_BIND_INDEX_BUFFER,
-                                             PIPE_USAGE_STREAM,
-                                             svga_render->ibuf_size);
-      svga_render->ibuf_offset = 0;
-   }
-
-   pipe_buffer_write_nooverlap(&svga->pipe, svga_render->ibuf,
-                               svga_render->ibuf_offset, 2 * nr_indices,
-                               indices);
-
    /* off to hardware */
    svga_vbuf_submit_state(svga_render);
 
@@ -361,34 +354,13 @@ svga_vbuf_render_draw_elements(struct vbuf_render *render,
     * redbook/polys.c
     */
    svga_update_state_retry(svga, SVGA_STATE_HW_DRAW);
-
-   ret = svga_hwtnl_draw_range_elements(svga->hwtnl,
-                                        svga_render->ibuf,
-                                        2,
-                                        bias,
-                                        svga_render->min_index,
-                                        svga_render->max_index,
-                                        svga_render->prim,
-                                        svga_render->ibuf_offset / 2,
-                                        nr_indices,
-                                        start_instance, instance_count);
+   ret = svga_hwtnl_draw_range_elements(svga->hwtnl, &info, nr_indices);
    if (ret != PIPE_OK) {
       svga_context_flush(svga, NULL);
-      ret = svga_hwtnl_draw_range_elements(svga->hwtnl,
-                                           svga_render->ibuf,
-                                           2,
-                                           bias,
-                                           svga_render->min_index,
-                                           svga_render->max_index,
-                                           svga_render->prim,
-                                           svga_render->ibuf_offset / 2,
-                                           nr_indices,
-                                           start_instance, instance_count);
+      ret = svga_hwtnl_draw_range_elements(svga->hwtnl, &info, nr_indices);
       svga->swtnl.new_vbuf = TRUE;
       assert(ret == PIPE_OK);
    }
-   svga_render->ibuf_offset += size;
-
    SVGA_STATS_TIME_POP(svga_sws(svga));
 }
 
diff --git a/src/gallium/drivers/svga/svga_swtnl_draw.c b/src/gallium/drivers/svga/svga_swtnl_draw.c
index 4cfd8c65ff4..a7db73e02ee 100644
--- a/src/gallium/drivers/svga/svga_swtnl_draw.c
+++ b/src/gallium/drivers/svga/svga_swtnl_draw.c
@@ -38,9 +38,7 @@
 
 enum pipe_error
 svga_swtnl_draw_vbo(struct svga_context *svga,
-                    const struct pipe_draw_info *info,
-                    struct pipe_resource *indexbuf,
-                    unsigned index_offset)
+                    const struct pipe_draw_info *info)
 {
    struct pipe_transfer *vb_transfer[PIPE_MAX_ATTRIBS] = { 0 };
    struct pipe_transfer *ib_transfer = NULL;
@@ -85,11 +83,13 @@ svga_swtnl_draw_vbo(struct svga_context *svga,
 
    /* Map index buffer, if present */
    map = NULL;
-   if (info->index_size && indexbuf) {
-      map = pipe_buffer_map(&svga->pipe, indexbuf,
-                            PIPE_TRANSFER_READ,
-                            &ib_transfer);
-      map = (ubyte *) map + index_offset;
+   if (info->index_size) {
+      if (info->has_user_indices) {
+         map = (ubyte *) info->index.user;
+      } else {
+         map = pipe_buffer_map(&svga->pipe, info->index.resource,
+                               PIPE_TRANSFER_READ, &ib_transfer);
+      }
       draw_set_indexes(draw,
                        (const ubyte *) map,
                        info->index_size, ~0);