[Mesa-dev] [PATCH 4/8] radeonsi: add a fast path for sRGB enable and no-op framebuffer changes

[Marek Olšák]

On Wed, Jun 7, 2017 at 2:09 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 05.06.2017 18:50, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> ---
>>   src/gallium/drivers/radeon/r600_pipe_common.h |  1 +
>>   src/gallium/drivers/radeon/r600_texture.c     |  1 +
>>   src/gallium/drivers/radeonsi/si_state.c       | 65
>> ++++++++++++++++++++++-----
>>   3 files changed, 55 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h
>> b/src/gallium/drivers/radeon/r600_pipe_common.h
>> index b17b690..6b78a07 100644
>> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
>> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
>> @@ -270,20 +270,21 @@ struct r600_texture {
>>         unsigned                        num_slow_clears;
>>         /* Counter that should be non-zero if the texture is bound to a
>>          * framebuffer. Implemented in radeonsi only.
>>          */
>>         uint32_t                        framebuffers_bound;
>>   };
>>     struct r600_surface {
>>         struct pipe_surface             base;
>> +       enum pipe_format                linear_format;
>>         /* These can vary with block-compressed textures. */
>>         unsigned width0;
>>         unsigned height0;
>>         bool color_initialized;
>>         bool depth_initialized;
>>         /* Misc. color flags. */
>>         bool alphatest_bypass;
>> diff --git a/src/gallium/drivers/radeon/r600_texture.c
>> b/src/gallium/drivers/radeon/r600_texture.c
>> index 4d72b86..48ae788 100644
>> --- a/src/gallium/drivers/radeon/r600_texture.c
>> +++ b/src/gallium/drivers/radeon/r600_texture.c
>> @@ -1943,20 +1943,21 @@ struct pipe_surface
>> *r600_create_surface_custom(struct pipe_context *pipe,
>>         assert(templ->u.tex.last_layer <= util_max_layer(texture,
>> templ->u.tex.level));
>>         pipe_reference_init(&surface->base.reference, 1);
>>         pipe_resource_reference(&surface->base.texture, texture);
>>         surface->base.context = pipe;
>>         surface->base.format = templ->format;
>>         surface->base.width = width;
>>         surface->base.height = height;
>>         surface->base.u = templ->u;
>>   +     surface->linear_format = util_format_linear(templ->format);
>>         surface->width0 = width0;
>>         surface->height0 = height0;
>>         surface->dcc_incompatible =
>>                 texture->target != PIPE_BUFFER &&
>>                 vi_dcc_formats_are_incompatible(texture,
>> templ->u.tex.level,
>>                                                 templ->format);
>>         return &surface->base;
>>   }
>>   diff --git a/src/gallium/drivers/radeonsi/si_state.c
>> b/src/gallium/drivers/radeonsi/si_state.c
>> index 6cf3559..50a0bd1 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -2423,32 +2423,84 @@ static void si_dec_framebuffer_counters(const
>> struct pipe_framebuffer_state *sta
>>                 if (!state->cbufs[i])
>>                         continue;
>>                 surf = (struct r600_surface*)state->cbufs[i];
>>                 rtex = (struct r600_texture*)surf->base.texture;
>>                 p_atomic_dec(&rtex->framebuffers_bound);
>>         }
>>   }
>>   +static void si_framebuffer_cache_flush(struct si_context *sctx)
>> +{
>> +       /* Only flush vL1 and L2 when changing the framebuffer state,
>> because
>> +        * the only client not using TC that can change textures is
>> +        * the framebuffer.
>> +        *
>> +        * Flush all CB and DB caches here because all buffers can be used
>> +        * for write by both TC (with shader image stores) and CB/DB.
>> +        */
>> +       sctx->b.flags |= SI_CONTEXT_INV_VMEM_L1 |
>> +                        SI_CONTEXT_INV_GLOBAL_L2 |
>
>
> This one bit shouldn't be needed on gfx9, right? Maybe for a separate patch.

No, it's correct. GFX9 is more complicated here. We can't remove
INV_GLOBAL_L2 in some cases. I have a WIP patch for that.

>
>
>
>> +                        SI_CONTEXT_FLUSH_AND_INV_CB |
>> +                        SI_CONTEXT_FLUSH_AND_INV_DB |
>> +                        SI_CONTEXT_CS_PARTIAL_FLUSH;
>> +}
>> +
>>   static void si_set_framebuffer_state(struct pipe_context *ctx,
>>                                      const struct pipe_framebuffer_state
>> *state)
>>   {
>>         struct si_context *sctx = (struct si_context *)ctx;
>>         struct pipe_constant_buffer constbuf = {0};
>>         struct r600_surface *surf = NULL;
>>         struct r600_texture *rtex;
>>         bool old_any_dst_linear = sctx->framebuffer.any_dst_linear;
>>         unsigned old_nr_samples = sctx->framebuffer.nr_samples;
>>         bool unbound = false;
>>         int i;
>>   +     /* Fast path for linear <-> sRGB changes and no-op changes. */
>
>
> Are linear <-> sRGB changes really that common to deserve this special
> treatment?

You're right. They are rare. I'll drop this. Even though it makes
GL_FRAMEBUFFER_SRGB changes 13x faster, they are probably well hidden
by u_threaded_context as long as they are rare.

Marek