[Mesa-dev] [PATCH 13/20] radeonsi: only emit compute shader state when switching shaders

[Bas Nieuwenhuizen]

On Mon, Apr 4, 2016 at 7:53 PM, Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
> On Mon, Apr 4, 2016 at 7:29 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Sat, Apr 2, 2016 at 3:10 PM, Bas Nieuwenhuizen
>> <bas at basnieuwenhuizen.nl> 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;
>>
>> Can this conditional, by any chance, be moved to the beginning of the function?
>
> si_setup_compute_sratch_buffer may need to reapply the relocations and
> then shader->bo changes.

And I just realized that can't happen when we use the same shader,
because it is then necessarily update from the last dispatch and we
don't create a new scratch buffer.

So, yes it can be moved to the beginning.

Thanks,
Bas

>>
>>> +
>>> +       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));
>>
>> Please use radeon_set_sh_reg_seq and radeon_set_sh_reg above.
>>
>> Marek