[Mesa-dev] [PATCH 13/20] radeonsi: only emit compute shader state when switching shaders
Nicolai Hähnle
nhaehnle at gmail.com
Tue Apr 5 23:58:20 UTC 2016
There's a dangling-pointer error waiting to happen with emitted_bo when
the last launched shader is deleted.
It seems safer to use emitted_shader rather than emitted_bo, and then
NULL that in si_delete_compute_state if required.
Cheers,
Nicolai
On 02.04.2016 08:10, Bas Nieuwenhuizen wrote:
> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> ---
> src/gallium/drivers/radeonsi/si_compute.c | 142 +++++++++++++++++-------------
> src/gallium/drivers/radeonsi/si_pipe.h | 2 +
> 2 files changed, 85 insertions(+), 59 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_compute.c b/src/gallium/drivers/radeonsi/si_compute.c
> index e712b46..74db8d4 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.c
> +++ b/src/gallium/drivers/radeonsi/si_compute.c
> @@ -173,6 +173,7 @@ static void si_initialize_compute(struct si_context *sctx)
> radeon_emit(cs, 0x190 /* Default value */);
> }
>
> + sctx->cs_shader_state.emitted_bo = NULL;
> sctx->cs_shader_state.initialized = true;
> }
>
> @@ -213,6 +214,87 @@ static bool si_setup_compute_scratch_buffer(struct si_context *sctx,
> return true;
> }
>
> +static bool si_switch_compute_shader(struct si_context *sctx,
> + struct si_compute *program,
> + struct si_shader *shader, unsigned offset)
> +{
> + struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
> + struct si_shader_config inline_config = {0};
> + struct si_shader_config *config;
> + uint64_t shader_va;
> +
> +
> + if (program->ir_type == PIPE_SHADER_IR_TGSI) {
> + config = &shader->config;
> + } else {
> + unsigned lds_blocks;
> +
> + config = &inline_config;
> + si_shader_binary_read_config(&shader->binary, config, offset);
> +
> + lds_blocks = config->lds_size;
> + /* XXX: We are over allocating LDS. For SI, the shader reports
> + * LDS in blocks of 256 bytes, so if there are 4 bytes lds
> + * allocated in the shader and 4 bytes allocated by the state
> + * tracker, then we will set LDS_SIZE to 512 bytes rather than 256.
> + */
> + if (sctx->b.chip_class <= SI) {
> + lds_blocks += align(program->local_size, 256) >> 8;
> + } else {
> + lds_blocks += align(program->local_size, 512) >> 9;
> + }
> +
> + assert(lds_blocks <= 0xFF);
> +
> + config->rsrc2 &= C_00B84C_LDS_SIZE;
> + config->rsrc2 |= S_00B84C_LDS_SIZE(lds_blocks);
> + }
> +
> + if (!si_setup_compute_scratch_buffer(sctx, shader, config))
> + return false;
> +
> + if (sctx->cs_shader_state.emitted_bo == shader->bo &&
> + sctx->cs_shader_state.offset == offset)
> + return true;
> +
> + if (shader->scratch_bo) {
> + COMPUTE_DBG(sctx->screen, "Waves: %u; Scratch per wave: %u bytes; "
> + "Total Scratch: %u bytes\n", sctx->scratch_waves,
> + config->scratch_bytes_per_wave,
> + config->scratch_bytes_per_wave *
> + sctx->scratch_waves);
> +
> + radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx,
> + shader->scratch_bo, RADEON_USAGE_READWRITE,
> + RADEON_PRIO_SCRATCH_BUFFER);
> + }
> +
> + shader_va = shader->bo->gpu_address + offset;
> +
> + radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, shader->bo,
> + RADEON_USAGE_READ, RADEON_PRIO_USER_SHADER);
> +
> + radeon_emit(cs, PKT3(PKT3_SET_SH_REG, 2, 0));
> + radeon_emit(cs, (R_00B830_COMPUTE_PGM_LO - SI_SH_REG_OFFSET) >> 2);
> + radeon_emit(cs, shader_va >> 8);
> + radeon_emit(cs, shader_va >> 40);
> +
> + radeon_emit(cs, PKT3(PKT3_SET_SH_REG, 2, 0));
> + radeon_emit(cs, (R_00B848_COMPUTE_PGM_RSRC1 - SI_SH_REG_OFFSET) >> 2);
> + radeon_emit(cs, config->rsrc1);
> + radeon_emit(cs, config->rsrc2);
> +
> + radeon_emit(cs, PKT3(PKT3_SET_SH_REG, 1, 0));
> + radeon_emit(cs, (R_00B860_COMPUTE_TMPRING_SIZE - SI_SH_REG_OFFSET) >> 2);
> + radeon_emit(cs, S_00B860_WAVES(sctx->scratch_waves)
> + | S_00B860_WAVESIZE(config->scratch_bytes_per_wave >> 10));
> +
> + sctx->cs_shader_state.emitted_bo = shader->bo;
> + sctx->cs_shader_state.offset = offset;
> +
> + return true;
> +}
> +
> static void si_upload_compute_input(struct si_context *sctx,
> const struct pipe_grid_info *info)
> {
> @@ -270,10 +352,7 @@ static void si_launch_grid(
> struct si_context *sctx = (struct si_context*)ctx;
> struct si_compute *program = sctx->cs_shader_state.program;
> struct si_pm4_state *pm4 = CALLOC_STRUCT(si_pm4_state);
> - uint64_t shader_va;
> unsigned i;
> - struct si_shader *shader = &program->shader;
> - unsigned lds_blocks;
>
> si_need_cs_space(sctx);
>
> @@ -290,29 +369,12 @@ static void si_launch_grid(
>
> pm4->compute_pkt = true;
>
> - /* Read the config information */
> - si_shader_binary_read_config(&shader->binary, &shader->config, info->pc);
> -
> - if (!si_setup_compute_scratch_buffer(sctx, shader, &shader->config))
> + if (!si_switch_compute_shader(sctx, program, &program->shader, info->pc))
> return;
>
> if (program->input_size)
> si_upload_compute_input(sctx, info);
>
> - if (shader->config.scratch_bytes_per_wave > 0) {
> -
> - COMPUTE_DBG(sctx->screen, "Waves: %u; Scratch per wave: %u bytes; "
> - "Total Scratch: %u bytes\n", sctx->scratch_waves,
> - shader->config.scratch_bytes_per_wave,
> - shader->config.scratch_bytes_per_wave *
> - sctx->scratch_waves);
> -
> - radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx,
> - shader->scratch_bo,
> - RADEON_USAGE_READWRITE,
> - RADEON_PRIO_SCRATCH_BUFFER);
> - }
> -
> si_pm4_set_reg(pm4, R_00B81C_COMPUTE_NUM_THREAD_X,
> S_00B81C_NUM_THREAD_FULL(info->block[0]));
> si_pm4_set_reg(pm4, R_00B820_COMPUTE_NUM_THREAD_Y,
> @@ -332,44 +394,6 @@ static void si_launch_grid(
> RADEON_PRIO_COMPUTE_GLOBAL);
> }
>
> - shader_va = shader->bo->gpu_address;
> - shader_va += info->pc;
> -
> - radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, shader->bo,
> - RADEON_USAGE_READ, RADEON_PRIO_USER_SHADER);
> - si_pm4_set_reg(pm4, R_00B830_COMPUTE_PGM_LO, shader_va >> 8);
> - si_pm4_set_reg(pm4, R_00B834_COMPUTE_PGM_HI, shader_va >> 40);
> -
> - si_pm4_set_reg(pm4, R_00B848_COMPUTE_PGM_RSRC1, shader->config.rsrc1);
> -
> - lds_blocks = shader->config.lds_size;
> - /* XXX: We are over allocating LDS. For SI, the shader reports LDS in
> - * blocks of 256 bytes, so if there are 4 bytes lds allocated in
> - * the shader and 4 bytes allocated by the state tracker, then
> - * we will set LDS_SIZE to 512 bytes rather than 256.
> - */
> - if (sctx->b.chip_class <= SI) {
> - lds_blocks += align(program->local_size, 256) >> 8;
> - } else {
> - lds_blocks += align(program->local_size, 512) >> 9;
> - }
> -
> - assert(lds_blocks <= 0xFF);
> -
> - shader->config.rsrc2 &= C_00B84C_LDS_SIZE;
> - shader->config.rsrc2 |= S_00B84C_LDS_SIZE(lds_blocks);
> -
> - si_pm4_set_reg(pm4, R_00B84C_COMPUTE_PGM_RSRC2, shader->config.rsrc2);
> -
> - si_pm4_set_reg(pm4, R_00B860_COMPUTE_TMPRING_SIZE,
> - /* The maximum value for WAVES is 32 * num CU.
> - * If you program this value incorrectly, the GPU will hang if
> - * COMPUTE_PGM_RSRC2.SCRATCH_EN is enabled.
> - */
> - S_00B860_WAVES(sctx->scratch_waves)
> - | S_00B860_WAVESIZE(shader->config.scratch_bytes_per_wave >> 10))
> - ;
> -
> si_pm4_cmd_begin(pm4, PKT3_DISPATCH_DIRECT);
> si_pm4_cmd_add(pm4, info->grid[0]); /* Thread groups DIM_X */
> si_pm4_cmd_add(pm4, info->grid[1]); /* Thread groups DIM_Y */
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
> index d53eef4..20d5e4e 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -133,6 +133,8 @@ struct si_sampler_state {
>
> struct si_cs_shader_state {
> struct si_compute *program;
> + struct r600_resource *emitted_bo;
> + unsigned offset;
> bool initialized;
> };
>
>
More information about the mesa-dev
mailing list