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 <[email protected]>
---
  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;
  };


_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to