[Mesa-dev] [PATCH] Revert "radeonsi: only set BC_OPTIMIZE_DISABLE when necessary"

[Marek Olšák]

This is weird. We only enable this optimization if the shader code
doesn't use CENTROID at all. This is the main flag:
sctx->ps_shader->info.uses_centroid.

You can try to simplify the condition to:

bc_optimize_disable = sctx->ps_shader->info.uses_centroid;

That should avoid using the optimization even for the non-MSAA case if
CENTROID is used.

Marek

On Tue, Jan 13, 2015 at 8:44 AM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> This reverts commit 0543630d0b0d9d9f6eefbc14fbd3385d4de37ba0.
>
> It caused flickering artifacts in Steam games such as Team Fortress 2 or
> Left 4 Dead 2.
>
> We could only enable this optimization by also making sure the shader code
> only uses either SI_PARAM_LINEAR_CENTROID or SI_PARAM_LINEAR_CENTER, not
> both. This would probably require a shader variant.
>
> Sorry I didn't remember this when reviewing the reverted change.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>  src/gallium/drivers/radeonsi/si_pipe.h          |  1 -
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 20 ++++++--------------
>  2 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
> index dfb1cd6..6144fb1 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -169,7 +169,6 @@ struct si_context {
>         /* shader information */
>         unsigned                        sprite_coord_enable;
>         bool                            flatshade;
> -       bool                            bc_optimize_disable;
>         struct si_descriptors           vertex_buffers;
>         struct si_buffer_resources      const_buffers[SI_NUM_SHADERS];
>         struct si_buffer_resources      rw_buffers[SI_NUM_SHADERS];
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 817a990..887680f 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -232,7 +232,7 @@ static void si_shader_ps(struct si_shader *shader)
>  {
>         struct tgsi_shader_info *info = &shader->selector->info;
>         struct si_pm4_state *pm4;
> -       unsigned i;
> +       unsigned i, spi_ps_in_control;
>         unsigned num_sgprs, num_user_sgprs;
>         unsigned spi_baryc_cntl = 0, spi_ps_input_ena;
>         uint64_t va;
> @@ -267,6 +267,9 @@ static void si_shader_ps(struct si_shader *shader)
>                 }
>         }
>
> +       spi_ps_in_control = S_0286D8_NUM_INTERP(shader->nparam) |
> +               S_0286D8_BC_OPTIMIZE_DISABLE(1);
> +
>         si_pm4_set_reg(pm4, R_0286E0_SPI_BARYC_CNTL, spi_baryc_cntl);
>         spi_ps_input_ena = shader->spi_ps_input_ena;
>         /* we need to enable at least one of them, otherwise we hang the GPU */
> @@ -281,6 +284,7 @@ static void si_shader_ps(struct si_shader *shader)
>
>         si_pm4_set_reg(pm4, R_0286CC_SPI_PS_INPUT_ENA, spi_ps_input_ena);
>         si_pm4_set_reg(pm4, R_0286D0_SPI_PS_INPUT_ADDR, spi_ps_input_ena);
> +       si_pm4_set_reg(pm4, R_0286D8_SPI_PS_IN_CONTROL, spi_ps_in_control);
>
>         si_pm4_set_reg(pm4, R_028710_SPI_SHADER_Z_FORMAT, shader->spi_shader_z_format);
>         si_pm4_set_reg(pm4, R_028714_SPI_SHADER_COL_FORMAT,
> @@ -661,10 +665,6 @@ bcolor:
>                 }
>         }
>
> -       si_pm4_set_reg(pm4, R_0286D8_SPI_PS_IN_CONTROL,
> -                      S_0286D8_NUM_INTERP(ps->nparam) |
> -                      S_0286D8_BC_OPTIMIZE_DISABLE(sctx->bc_optimize_disable));
> -
>         si_pm4_set_state(sctx, spi, pm4);
>  }
>
> @@ -710,7 +710,6 @@ void si_update_shaders(struct si_context *sctx)
>  {
>         struct pipe_context *ctx = (struct pipe_context*)sctx;
>         struct si_state_rasterizer *rs = sctx->queued.named.rasterizer;
> -       bool bc_optimize_disable;
>
>         if (sctx->gs_shader) {
>                 si_shader_select(ctx, sctx->gs_shader);
> @@ -775,18 +774,11 @@ void si_update_shaders(struct si_context *sctx)
>
>         si_pm4_bind_state(sctx, ps, sctx->ps_shader->current->pm4);
>
> -       /* Whether CENTER != CENTROID. */
> -       bc_optimize_disable = sctx->framebuffer.nr_samples > 1 &&
> -                             rs->multisample_enable &&
> -                             sctx->ps_shader->info.uses_centroid;
> -
>         if (si_pm4_state_changed(sctx, ps) || si_pm4_state_changed(sctx, vs) ||
>             sctx->sprite_coord_enable != rs->sprite_coord_enable ||
> -           sctx->flatshade != rs->flatshade ||
> -           sctx->bc_optimize_disable != bc_optimize_disable) {
> +           sctx->flatshade != rs->flatshade) {
>                 sctx->sprite_coord_enable = rs->sprite_coord_enable;
>                 sctx->flatshade = rs->flatshade;
> -               sctx->bc_optimize_disable = bc_optimize_disable;
>                 si_update_spi_map(sctx);
>         }
>
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev