[Mesa-dev] [PATCH 07/17] radeonsi: skip DCC render feedback checking if color writes are disabled

[Timothy Arceri]

On 13/04/18 10:45, Timothy Arceri wrote:
> This change cause around 20+ piglit crashes on my Polaris.
> 
> e.g tests/spec/arb_compute_shader/execution/atomic-counter.shader_test
> 
> Thread 1 "shader_runner" received signal SIGSEGV, Segmentation fault.
> 0x00007ffff1009ccc in si_get_total_colormask (sctx=0x64b140) at 
> si_pipe.h:945
> 945        if (sctx->queued.named.rasterizer->rasterizer_discard)
> 
> 
> It also seems to cause hundreds of test failures e.g
> 
> ./bin/copyteximage CUBE -auto
> 
> 
> Unfortunately it doesn't revert cleanly either.

Actually ignore this second problem I'm seeing a lot of intermittent 
test failures. These are being caused by something else, however the
crash above is caused by this commit.


> 
> On 04/04/18 11:59, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> The previous patch is required for this.
>> ---
>>   src/gallium/drivers/radeonsi/si_blit.c          |  5 +++++
>>   src/gallium/drivers/radeonsi/si_pipe.h          | 17 +++++++++++++++++
>>   src/gallium/drivers/radeonsi/si_state_shaders.c |  6 +-----
>>   3 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
>> b/src/gallium/drivers/radeonsi/si_blit.c
>> index 45770b0d9bf..8dd8bc2a4dd 100644
>> --- a/src/gallium/drivers/radeonsi/si_blit.c
>> +++ b/src/gallium/drivers/radeonsi/si_blit.c
>> @@ -706,20 +706,25 @@ static void 
>> si_check_render_feedback_resident_images(struct si_context *sctx)
>>           si_check_render_feedback_texture(sctx, tex,
>>                            view->u.tex.level,
>>                            view->u.tex.level,
>>                            view->u.tex.first_layer,
>>                            view->u.tex.last_layer);
>>       }
>>   }
>>   static void si_check_render_feedback(struct si_context *sctx)
>>   {
>> +    /* There is no render feedback if color writes are disabled.
>> +     * (e.g. a pixel shader with image stores)
>> +     */
>> +    if (!si_get_total_colormask(sctx))
>> +        return;
>>       if (!sctx->need_check_render_feedback)
>>           return;
>>       for (int i = 0; i < SI_NUM_SHADERS; ++i) {
>>           si_check_render_feedback_images(sctx, &sctx->images[i]);
>>           si_check_render_feedback_textures(sctx, &sctx->samplers[i]);
>>       }
>>       si_check_render_feedback_resident_images(sctx);
>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
>> b/src/gallium/drivers/radeonsi/si_pipe.h
>> index e3d45ef6c3b..e65c946d186 100644
>> --- a/src/gallium/drivers/radeonsi/si_pipe.h
>> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
>> @@ -933,11 +933,28 @@ vi_tc_compat_htile_enabled(struct r600_texture 
>> *tex, unsigned level)
>>   }
>>   static inline unsigned si_get_ps_iter_samples(struct si_context *sctx)
>>   {
>>       if (sctx->ps_uses_fbfetch)
>>           return sctx->framebuffer.nr_samples;
>>       return sctx->ps_iter_samples;
>>   }
>> +static inline unsigned si_get_total_colormask(struct si_context *sctx)
>> +{
>> +    if (sctx->queued.named.rasterizer->rasterizer_discard)
>> +        return 0;
>> +
>> +    struct si_shader_selector *ps = sctx->ps_shader.cso;
>> +    unsigned colormask = sctx->framebuffer.colorbuf_enabled_4bit &
>> +                 sctx->queued.named.blend->cb_target_mask;
>> +
>> +    if (!ps->info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS])
>> +        colormask &= ps->colors_written_4bit;
>> +    else if (!ps->colors_written_4bit)
>> +        colormask = 0; /* color0 writes all cbufs, but it's not 
>> written */
>> +
>> +    return colormask;
>> +}
>> +
>>   #endif
>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
>> b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> index d7742eafb04..f2d29e40744 100644
>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> @@ -1208,25 +1208,21 @@ static void 
>> si_shader_selector_key_hw_vs(struct si_context *sctx,
>>       bool ps_disabled = true;
>>       if (ps) {
>>           const struct si_state_blend *blend = sctx->queued.named.blend;
>>           bool alpha_to_coverage = blend && blend->alpha_to_coverage;
>>           bool ps_modifies_zs = ps->info.uses_kill ||
>>                         ps->info.writes_z ||
>>                         ps->info.writes_stencil ||
>>                         ps->info.writes_samplemask ||
>>                         alpha_to_coverage ||
>>                         si_get_alpha_test_func(sctx) != PIPE_FUNC_ALWAYS;
>> -
>> -        unsigned ps_colormask = 
>> sctx->framebuffer.colorbuf_enabled_4bit &
>> -                    sctx->queued.named.blend->cb_target_mask;
>> -        if 
>> (!ps->info.properties[TGSI_PROPERTY_FS_COLOR0_WRITES_ALL_CBUFS])
>> -            ps_colormask &= ps->colors_written_4bit;
>> +        unsigned ps_colormask = si_get_total_colormask(sctx);
>>           ps_disabled = 
>> sctx->queued.named.rasterizer->rasterizer_discard ||
>>                     (!ps_colormask &&
>>                      !ps_modifies_zs &&
>>                      !ps->info.writes_memory);
>>       }
>>       /* Find out which VS outputs aren't used by the PS. */
>>       uint64_t outputs_written = vs->outputs_written;
>>       uint64_t inputs_read = 0;
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev