svga: Fix index buffer uploads
authorThomas Hellstrom <thellstrom@vmware.com>
Thu, 11 Apr 2019 06:56:20 +0000 (08:56 +0200)
committerThomas Hellstrom <thellstrom@vmware.com>
Thu, 20 Jun 2019 07:30:22 +0000 (09:30 +0200)
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 <thellstrom@vmware.com>
Reviewed-by: Brian Paul <brianp@vmware.com>
src/gallium/drivers/svga/svga_draw.h
src/gallium/drivers/svga/svga_draw_elements.c
src/gallium/drivers/svga/svga_pipe_draw.c
src/gallium/drivers/svga/svga_swtnl.h
src/gallium/drivers/svga/svga_swtnl_backend.c
src/gallium/drivers/svga/svga_swtnl_draw.c

index baefcd94ec86a59d3455ca486299978b6ebb4c99..9d79676d3f93b1237b63186380e785cfde1b2ca9 100644 (file)
@@ -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_ */
index b1db8710740ce1594de0756c58c88f6d92d766c7..b955b2f77e272ac53e8934887eb9dda9e545ed80 100644 (file)
  * \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) {
index ace1d2cb6f3c06928037dde1c0c8a627cf5da550..5ebd17cf0eabc5c50685467353883625beefdd9e 100644 (file)
@@ -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));
 }
 
index 0661b718fd3b138e1db3ee057cb56c873d0a8e1b..fc094e514282deb200f9d2d474fd35a938e94f6d 100644 (file)
@@ -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
index 26f8107129eedb75a41a9dfd19989e6a35a93a49..b6fd07fe3467e7d7b6b37d1901a8af6f439c5379 100644 (file)
@@ -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));
 }
 
index 4cfd8c65ff466869833d28f50d97eb4b2894edf6..a7db73e02ee109df741321e6cd67bed95e88e5ea 100644 (file)
@@ -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);