[Mesa-dev] [PATCH v3] radeonsi: Reinitialize all descriptors in CE preamble.

[Bas Nieuwenhuizen]

On Fri, Jun 10, 2016 at 11:35 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 09.06.2016 19:50, Bas Nieuwenhuizen wrote:
>>
>> This fixes a problem with the CE preamble and restoring only stuff in the
>> preamble when needed.
>>
>> To illustrate suppose we have two graphics IB's 1 and 2, which  are
>> submitted in
>> that order. Furthermore suppose IB 1 does not use CE ram, but IB 2 does,
>> and we
>> have a context switch at the start of IB 1, but not between IB 1 and IB 2.
>>
>> The old code put the CE RAM loads in the preamble of IB 2. As the preamble
>> of
>> IB 1 does not have the loads and the preamble of IB 2 does not get
>> executed, the
>> old values are not load into CE RAM.
>>
>> Fix this by always restoring the entire CE RAM.
>>
>> v2: - Just load all descriptor set buffers instead of load and store the
>> entire
>>        CE RAM.
>>      - Leave the ce_ram_dirty tracking in place for the non-preamble case.
>>
>> v3: - Fixed parameter alignment.
>>      - Rebased to master (Nicolai's descriptor series).
>
>>
>>
>> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
>
>
> Does this patch still apply to the 12.0 branch? If not, make sure to let
> Emil know about the correct version for backports.

This version does not apply to 12.0 anymore, please use v2 for the
stable branch.

- Bas

> Apart from that,
>
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
>
>> ---
>>
>> Sending a new version as the rebase resulted in essentially rewriting the
>> main
>> part of the patch.
>>
>>   src/gallium/drivers/radeonsi/si_descriptors.c | 14 +++++++++++---
>>   src/gallium/drivers/radeonsi/si_hw_context.c  |  3 +++
>>   src/gallium/drivers/radeonsi/si_state.h       |  1 +
>>   3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c
>> b/src/gallium/drivers/radeonsi/si_descriptors.c
>> index e80db39..2d780e6 100644
>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>> @@ -160,8 +160,8 @@ static bool si_ce_upload(struct si_context *sctx,
>> unsigned ce_offset, unsigned s
>>         return true;
>>   }
>>
>> -static void si_reinitialize_ce_ram(struct si_context *sctx,
>> -                            struct si_descriptors *desc)
>> +static void si_ce_reinitialize_descriptors(struct si_context *sctx,
>> +                                           struct si_descriptors *desc)
>>   {
>>         if (desc->buffer) {
>>                 struct r600_resource *buffer = (struct
>> r600_resource*)desc->buffer;
>> @@ -186,6 +186,14 @@ static void si_reinitialize_ce_ram(struct si_context
>> *sctx,
>>         desc->ce_ram_dirty = false;
>>   }
>>
>> +void si_ce_reinitialize_all_descriptors(struct si_context *sctx)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < SI_NUM_DESCS; ++i)
>> +               si_ce_reinitialize_descriptors(sctx,
>> &sctx->descriptors[i]);
>> +}
>> +
>>   void si_ce_enable_loads(struct radeon_winsys_cs *ib)
>>   {
>>         radeon_emit(ib, PKT3(PKT3_CONTEXT_CONTROL, 1, 0));
>> @@ -207,7 +215,7 @@ static bool si_upload_descriptors(struct si_context
>> *sctx,
>>                 uint32_t const* list = (uint32_t const*)desc->list;
>>
>>                 if (desc->ce_ram_dirty)
>> -                       si_reinitialize_ce_ram(sctx, desc);
>> +                       si_ce_reinitialize_descriptors(sctx, desc);
>>
>>                 while(desc->dirty_mask) {
>>                         int begin, count;
>> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c
>> b/src/gallium/drivers/radeonsi/si_hw_context.c
>> index fa6a2cb..d1b9851 100644
>> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
>> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
>> @@ -213,6 +213,9 @@ void si_begin_new_cs(struct si_context *ctx)
>>         else if (ctx->ce_ib)
>>                 si_ce_enable_loads(ctx->ce_ib);
>>
>> +       if (ctx->ce_preamble_ib)
>> +               si_ce_reinitialize_all_descriptors(ctx);
>> +
>>         ctx->framebuffer.dirty_cbufs = (1 << 8) - 1;
>>         ctx->framebuffer.dirty_zsbuf = true;
>>         si_mark_atom_dirty(ctx, &ctx->framebuffer.atom);
>> diff --git a/src/gallium/drivers/radeonsi/si_state.h
>> b/src/gallium/drivers/radeonsi/si_state.h
>> index a4a58bb..ab34fec 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.h
>> +++ b/src/gallium/drivers/radeonsi/si_state.h
>> @@ -276,6 +276,7 @@ struct si_buffer_resources {
>>         } while(0)
>>
>>   /* si_descriptors.c */
>> +void si_ce_reinitialize_all_descriptors(struct si_context *sctx);
>>   void si_ce_enable_loads(struct radeon_winsys_cs *ib);
>>   void si_set_mutable_tex_desc_fields(struct r600_texture *tex,
>>                                     const struct radeon_surf_level
>> *base_level_info,
>>
>