[Mesa-dev] [PATCH] r600g: fix lockups with dual_src_blend mode

[Vadim Girlin]

On Mon, 2012-08-20 at 23:27 +0200, Marek Olšák wrote:
> I am not so familiar with this piece of code, but the idea sounds good.
> 
> In the future, we should just disable the dual src blending instead.
> Shader recompilations certainly take more time than changing a couple
> of blending registers.

Yeah, I agree that it should be better to disable dual src blending,
probably I'll try to implement it that way. This patch is just a first
idea that worked. Maybe somebody with complete docs knows what is the
best solution?

Also I have some doubts about the test itself - the situation with
enabled dual_src_blend and the only pixel export in the shader happens
during DrawPixels call without user's fragment shader. I haven't looked
much into it, but AFAIK the result is undefined in that case, so maybe
we also need to fix something in the test or in the gallium to handle it
correctly.

Vadim

> Marek
> 
> On Mon, Aug 20, 2012 at 10:35 PM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
> > Add fake pixel exports if we have less than two when dual_src_blend is enabled.
> >
> > Fixes lockups with ext_framebuffer_multisample-
> > alpha-to-coverage-dual-src-blend piglit test on juniper.
> >
> > Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>
> > ---
> >
> > Tested on evergreen only.
> >
> > Don't know if it's documented somewhere, possibly we only need to adjust some
> > config regs without adding fake export to the shader code.
> >
> >  src/gallium/drivers/r600/r600_shader.c | 37 +++++++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> > index 834c0b3..aa65e4e 100644
> > --- a/src/gallium/drivers/r600/r600_shader.c
> > +++ b/src/gallium/drivers/r600/r600_shader.c
> > @@ -1198,7 +1198,6 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> >         ctx.colors_used = 0;
> >         ctx.clip_vertex_write = 0;
> >
> > -       shader->nr_ps_color_exports = 0;
> >         shader->nr_ps_max_color_exports = 0;
> >
> >         shader->two_side = (ctx.type == TGSI_PROCESSOR_FRAGMENT) && rctx->two_side;
> > @@ -1587,7 +1586,6 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> >                                 output[j].swizzle_w = rctx->alpha_to_one && rctx->multisample_enable && !rctx->cb0_is_integer ? 5 : 3;
> >                                 output[j].array_base = next_pixel_base++;
> >                                 output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> > -                               shader->nr_ps_color_exports++;
> >                                 if (shader->fs_write_all && (rctx->chip_class >= EVERGREEN)) {
> >                                         for (k = 1; k < rctx->nr_cbufs; k++) {
> >                                                 j++;
> > @@ -1603,7 +1601,6 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> >                                                 output[j].array_base = next_pixel_base++;
> >                                                 output[j].inst = BC_INST(ctx.bc, V_SQ_CF_ALLOC_EXPORT_WORD1_SQ_CF_INST_EXPORT);
> >                                                 output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> > -                                               shader->nr_ps_color_exports++;
> >                                         }
> >                                 }
> >                         } else if (shader->output[i].name == TGSI_SEMANTIC_POSITION) {
> > @@ -1653,23 +1650,27 @@ static int r600_shader_from_tgsi(struct r600_context * rctx, struct r600_pipe_sh
> >                         j++;
> >         }
> >
> > -       /* add fake pixel export */
> > -       if (ctx.type == TGSI_PROCESSOR_FRAGMENT && next_pixel_base == 0) {
> > -               memset(&output[j], 0, sizeof(struct r600_bytecode_output));
> > -               output[j].gpr = 0;
> > -               output[j].elem_size = 3;
> > -               output[j].swizzle_x = 7;
> > -               output[j].swizzle_y = 7;
> > -               output[j].swizzle_z = 7;
> > -               output[j].swizzle_w = 7;
> > -               output[j].burst_count = 1;
> > -               output[j].barrier = 1;
> > -               output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> > -               output[j].array_base = 0;
> > -               output[j].inst = BC_INST(ctx.bc, V_SQ_CF_ALLOC_EXPORT_WORD1_SQ_CF_INST_EXPORT);
> > -               j++;
> > +       /* add fake pixel exports if we have less than
> > +        * (1 + rctx->dual_src_blend) pixel exports */
> > +       if (ctx.type == TGSI_PROCESSOR_FRAGMENT) {
> > +               while (next_pixel_base <= rctx->dual_src_blend) {
> > +                       memset(&output[j], 0, sizeof(struct r600_bytecode_output));
> > +                       output[j].gpr = 0;
> > +                       output[j].elem_size = 3;
> > +                       output[j].swizzle_x = 7;
> > +                       output[j].swizzle_y = 7;
> > +                       output[j].swizzle_z = 7;
> > +                       output[j].swizzle_w = 7;
> > +                       output[j].burst_count = 1;
> > +                       output[j].barrier = 1;
> > +                       output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
> > +                       output[j].array_base = next_pixel_base++;
> > +                       output[j].inst = BC_INST(ctx.bc, V_SQ_CF_ALLOC_EXPORT_WORD1_SQ_CF_INST_EXPORT);
> > +                       j++;
> > +               }
> >         }
> >
> > +       shader->nr_ps_color_exports = next_pixel_base;
> >         noutput = j;
> >
> >         /* set export done on last export of each type */
> > --
> > 1.7.11.4
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev