From f51915ba62aebfda5fc3b15d90540bd3f6a981ea Mon Sep 17 00:00:00 2001 From: Thomas Hellstrom Date: Thu, 11 Apr 2019 08:56:20 +0200 Subject: [PATCH] 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 Reviewed-by: Brian Paul --- src/gallium/drivers/svga/svga_draw.h | 10 +- src/gallium/drivers/svga/svga_draw_elements.c | 168 ++++++++++++++++---------- src/gallium/drivers/svga/svga_pipe_draw.c | 58 ++------- src/gallium/drivers/svga/svga_swtnl.h | 4 +- src/gallium/drivers/svga/svga_swtnl_backend.c | 58 +++------ src/gallium/drivers/svga/svga_swtnl_draw.c | 16 +-- 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 baefcd9..9d79676 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 b1db871..b955b2f 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 ace1d2c..5ebd17c 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 0661b71..fc094e5 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 26f8107..b6fd07f 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 4cfd8c6..a7db73e 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); -- 2.7.4