[Mesa-dev] [PATCH 02/10] radeonsi: make RW buffer descriptor array global, not per shader stage
Nicolai Hähnle
nhaehnle at gmail.com
Thu Apr 21 18:10:09 UTC 2016
On 21.04.2016 07:49, Bas Nieuwenhuizen wrote:
> On Wed, Apr 20, 2016 at 5:47 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> ---
>> src/gallium/drivers/radeonsi/si_descriptors.c | 50 +++++++++++++--------------
>> src/gallium/drivers/radeonsi/si_pipe.h | 2 +-
>> 2 files changed, 25 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
>> index 01cf79e..c802b1e 100644
>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>> @@ -900,7 +900,7 @@ void si_set_ring_buffer(struct pipe_context *ctx, uint shader, uint slot,
>> unsigned element_size, unsigned index_stride, uint64_t offset)
>> {
>> struct si_context *sctx = (struct si_context *)ctx;
>> - struct si_buffer_resources *buffers = &sctx->rw_buffers[shader];
>> + struct si_buffer_resources *buffers = &sctx->rw_buffers;
>>
>> if (shader >= SI_NUM_SHADERS)
>> return;
>
> I think it would be nice to remove this check and the shader argument
> to this function.
>
>> @@ -994,7 +994,7 @@ static void si_set_streamout_targets(struct pipe_context *ctx,
>> const unsigned *offsets)
>> {
>> struct si_context *sctx = (struct si_context *)ctx;
>> - struct si_buffer_resources *buffers = &sctx->rw_buffers[PIPE_SHADER_VERTEX];
>> + struct si_buffer_resources *buffers = &sctx->rw_buffers;
>> unsigned old_num_targets = sctx->b.streamout.num_targets;
>> unsigned i, bufidx;
>>
>> @@ -1198,7 +1198,7 @@ static void si_invalidate_buffer(struct pipe_context *ctx, struct pipe_resource
>>
>> /* Read/Write buffers. */
>> for (shader = 0; shader < SI_NUM_SHADERS; shader++) {
>> - struct si_buffer_resources *buffers = &sctx->rw_buffers[shader];
>> + struct si_buffer_resources *buffers = &sctx->rw_buffers;
>> uint64_t mask = buffers->desc.enabled_mask;
>
> Looping over the shaders here seems not necessary.
I think it's even incorrect, because applying
si_desc_reset_buffer_offset multiple times will cause va - old_va to be
added to the address on every iteration.
Nicolai
> Also the check in
> the loop is wrong when we add the PS entries to the rw_buffers set, as
> all of the entries get first processed at the VS.
>
> - Bas
>
>>
>> while (mask) {
>> @@ -1289,7 +1289,6 @@ static void si_mark_shader_pointers_dirty(struct si_context *sctx,
>> unsigned shader)
>> {
>> sctx->const_buffers[shader].desc.pointer_dirty = true;
>> - sctx->rw_buffers[shader].desc.pointer_dirty = true;
>> sctx->shader_buffers[shader].desc.pointer_dirty = true;
>> sctx->samplers[shader].views.desc.pointer_dirty = true;
>> sctx->images[shader].desc.pointer_dirty = true;
>> @@ -1307,6 +1306,7 @@ static void si_shader_userdata_begin_new_cs(struct si_context *sctx)
>> for (i = 0; i < SI_NUM_SHADERS; i++) {
>> si_mark_shader_pointers_dirty(sctx, i);
>> }
>> + sctx->rw_buffers.desc.pointer_dirty = true;
>> }
>>
>> /* Set a base register address for user data constants in the given shader.
>> @@ -1385,22 +1385,23 @@ void si_emit_graphics_shader_userdata(struct si_context *sctx,
>> uint32_t *sh_base = sctx->shader_userdata.sh_base;
>>
>> if (sctx->gs_shader.cso) {
>> - /* The VS copy shader needs these for clipping, streamout, and rings. */
>> + /* The VS copy shader needs this for clipping. */
>> unsigned vs_base = R_00B130_SPI_SHADER_USER_DATA_VS_0;
>> unsigned i = PIPE_SHADER_VERTEX;
>>
>> si_emit_shader_pointer(sctx, &sctx->const_buffers[i].desc, vs_base, true);
>> - si_emit_shader_pointer(sctx, &sctx->rw_buffers[i].desc, vs_base, true);
>> + }
>>
>> - if (sctx->tes_shader.cso) {
>> - /* The TESSEVAL shader needs this for the ESGS ring buffer. */
>> - si_emit_shader_pointer(sctx, &sctx->rw_buffers[i].desc,
>> - R_00B330_SPI_SHADER_USER_DATA_ES_0, true);
>> - }
>> - } else if (sctx->tes_shader.cso) {
>> - /* The TESSEVAL shader needs this for streamout. */
>> - si_emit_shader_pointer(sctx, &sctx->rw_buffers[PIPE_SHADER_VERTEX].desc,
>> + if (sctx->rw_buffers.desc.pointer_dirty) {
>> + si_emit_shader_pointer(sctx, &sctx->rw_buffers.desc,
>> R_00B130_SPI_SHADER_USER_DATA_VS_0, true);
>> + si_emit_shader_pointer(sctx, &sctx->rw_buffers.desc,
>> + R_00B230_SPI_SHADER_USER_DATA_GS_0, true);
>> + si_emit_shader_pointer(sctx, &sctx->rw_buffers.desc,
>> + R_00B330_SPI_SHADER_USER_DATA_ES_0, true);
>> + si_emit_shader_pointer(sctx, &sctx->rw_buffers.desc,
>> + R_00B430_SPI_SHADER_USER_DATA_HS_0, true);
>> + sctx->rw_buffers.desc.pointer_dirty = false;
>> }
>>
>> for (i = 0; i < SI_NUM_GRAPHICS_SHADERS; i++) {
>> @@ -1409,9 +1410,6 @@ void si_emit_graphics_shader_userdata(struct si_context *sctx,
>> if (!base)
>> continue;
>>
>> - if (i != PIPE_SHADER_TESS_EVAL)
>> - si_emit_shader_pointer(sctx, &sctx->rw_buffers[i].desc, base, false);
>> -
>> si_emit_shader_pointer(sctx, &sctx->const_buffers[i].desc, base, false);
>> si_emit_shader_pointer(sctx, &sctx->shader_buffers[i].desc, base, false);
>> si_emit_shader_pointer(sctx, &sctx->samplers[i].views.desc, base, false);
>> @@ -1446,10 +1444,6 @@ void si_init_all_descriptors(struct si_context *sctx)
>> SI_NUM_CONST_BUFFERS, SI_SGPR_CONST_BUFFERS,
>> RADEON_USAGE_READ, RADEON_PRIO_CONST_BUFFER,
>> &ce_offset);
>> - si_init_buffer_resources(&sctx->rw_buffers[i],
>> - SI_NUM_RW_BUFFERS, SI_SGPR_RW_BUFFERS,
>> - RADEON_USAGE_READWRITE, RADEON_PRIO_RINGS_STREAMOUT,
>> - &ce_offset);
>> si_init_buffer_resources(&sctx->shader_buffers[i],
>> SI_NUM_SHADER_BUFFERS, SI_SGPR_SHADER_BUFFERS,
>> RADEON_USAGE_READWRITE, RADEON_PRIO_SHADER_RW_BUFFER,
>> @@ -1464,6 +1458,10 @@ void si_init_all_descriptors(struct si_context *sctx)
>> null_image_descriptor, &ce_offset);
>> }
>>
>> + si_init_buffer_resources(&sctx->rw_buffers,
>> + SI_NUM_RW_BUFFERS, SI_SGPR_RW_BUFFERS,
>> + RADEON_USAGE_READWRITE, RADEON_PRIO_RINGS_STREAMOUT,
>> + &ce_offset);
>> si_init_descriptors(&sctx->vertex_buffers, SI_SGPR_VERTEX_BUFFERS,
>> 4, SI_NUM_VERTEX_BUFFERS, NULL, NULL);
>>
>> @@ -1496,8 +1494,6 @@ bool si_upload_graphics_shader_descriptors(struct si_context *sctx)
>> for (i = 0; i < SI_NUM_SHADERS; i++) {
>> if (!si_upload_descriptors(sctx, &sctx->const_buffers[i].desc,
>> &sctx->shader_userdata.atom) ||
>> - !si_upload_descriptors(sctx, &sctx->rw_buffers[i].desc,
>> - &sctx->shader_userdata.atom) ||
>> !si_upload_descriptors(sctx, &sctx->shader_buffers[i].desc,
>> &sctx->shader_userdata.atom) ||
>> !si_upload_descriptors(sctx, &sctx->samplers[i].views.desc,
>> @@ -1506,7 +1502,9 @@ bool si_upload_graphics_shader_descriptors(struct si_context *sctx)
>> &sctx->shader_userdata.atom))
>> return false;
>> }
>> - return si_upload_vertex_buffer_descriptors(sctx);
>> + return si_upload_descriptors(sctx, &sctx->rw_buffers.desc,
>> + &sctx->shader_userdata.atom) &&
>> + si_upload_vertex_buffer_descriptors(sctx);
>> }
>>
>> bool si_upload_compute_shader_descriptors(struct si_context *sctx)
>> @@ -1530,11 +1528,11 @@ void si_release_all_descriptors(struct si_context *sctx)
>>
>> for (i = 0; i < SI_NUM_SHADERS; i++) {
>> si_release_buffer_resources(&sctx->const_buffers[i]);
>> - si_release_buffer_resources(&sctx->rw_buffers[i]);
>> si_release_buffer_resources(&sctx->shader_buffers[i]);
>> si_release_sampler_views(&sctx->samplers[i].views);
>> si_release_image_views(&sctx->images[i]);
>> }
>> + si_release_buffer_resources(&sctx->rw_buffers);
>> si_release_descriptors(&sctx->vertex_buffers);
>> }
>>
>> @@ -1544,11 +1542,11 @@ void si_all_descriptors_begin_new_cs(struct si_context *sctx)
>>
>> for (i = 0; i < SI_NUM_SHADERS; i++) {
>> si_buffer_resources_begin_new_cs(sctx, &sctx->const_buffers[i]);
>> - si_buffer_resources_begin_new_cs(sctx, &sctx->rw_buffers[i]);
>> si_buffer_resources_begin_new_cs(sctx, &sctx->shader_buffers[i]);
>> si_sampler_views_begin_new_cs(sctx, &sctx->samplers[i].views);
>> si_image_views_begin_new_cs(sctx, &sctx->images[i]);
>> }
>> + si_buffer_resources_begin_new_cs(sctx, &sctx->rw_buffers);
>> si_vertex_buffers_begin_new_cs(sctx);
>> si_shader_userdata_begin_new_cs(sctx);
>> }
>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
>> index a28c7d70..48095b0 100644
>> --- a/src/gallium/drivers/radeonsi/si_pipe.h
>> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
>> @@ -246,8 +246,8 @@ struct si_context {
>>
>> /* shader descriptors */
>> struct si_descriptors vertex_buffers;
>> + struct si_buffer_resources rw_buffers;
>> struct si_buffer_resources const_buffers[SI_NUM_SHADERS];
>> - struct si_buffer_resources rw_buffers[SI_NUM_SHADERS];
>> struct si_buffer_resources shader_buffers[SI_NUM_SHADERS];
>> struct si_textures_info samplers[SI_NUM_SHADERS];
>> struct si_images_info images[SI_NUM_SHADERS];
>> --
>> 2.5.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list