[Freedreno] [PATCH 1/8] freedreno/a3xx: fix alpha-blending on RGBX formats

Rob Clark robdclark at gmail.com
Thu Dec 4 08:37:10 PST 2014


On Wed, Dec 3, 2014 at 9:57 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Expert debugging assistance provided by Chris Forbes.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/gallium/drivers/freedreno/a3xx/fd3_emit.c  | 21 +++++++++---
>  src/gallium/drivers/freedreno/freedreno_util.c | 46 ++++++++++++++++++++++++++
>  src/gallium/drivers/freedreno/freedreno_util.h |  1 +
>  3 files changed, 63 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/freedreno/a3xx/fd3_emit.c b/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
> index 44f8ac4..7a6557a 100644
> --- a/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
> +++ b/src/gallium/drivers/freedreno/a3xx/fd3_emit.c
> @@ -571,11 +571,12 @@ fd3_emit_state(struct fd_context *ctx, struct fd_ringbuffer *ring,
>                 uint32_t i;
>
>                 for (i = 0; i < ARRAY_SIZE(blend->rb_mrt); i++) {
> -                       bool is_float = util_format_is_float(
> -                                       pipe_surface_format(ctx->framebuffer.cbufs[i]));
> -                       bool is_int = util_format_is_pure_integer(
> -                                       pipe_surface_format(ctx->framebuffer.cbufs[i]));
> +                       enum pipe_format format = pipe_surface_format(ctx->framebuffer.cbufs[i]);
> +                       bool is_float = util_format_is_float(format);
> +                       bool is_int = util_format_is_pure_integer(format);
> +                       bool has_alpha = util_format_has_alpha(format);
>                         uint32_t control = blend->rb_mrt[i].control;
> +                       uint32_t blend_control = blend->rb_mrt[i].blend_control;
>
>                         if (is_int) {
>                                 control &= (A3XX_RB_MRT_CONTROL_COMPONENT_ENABLE__MASK |
> @@ -583,11 +584,21 @@ fd3_emit_state(struct fd_context *ctx, struct fd_ringbuffer *ring,
>                                 control |= A3XX_RB_MRT_CONTROL_ROP_CODE(ROP_COPY);
>                         }
>

hmm, this getting a bit sad all the fixup we have to do in emit_state...

Maybe we should populate the blend state objects w/ alternatives?  Ie,
blend->rb_mrt[i].blend_control_a and blend->rb_mrt[i].blend_control_x
and then emit_state code just has to pick the right one?


> +                       if (!has_alpha) {
> +                               blend_control &= ~(
> +                                               A3XX_RB_MRT_BLEND_CONTROL_RGB_SRC_FACTOR__MASK |
> +                                               A3XX_RB_MRT_BLEND_CONTROL_RGB_DEST_FACTOR__MASK);
> +                               blend_control |=
> +                                       A3XX_RB_MRT_BLEND_CONTROL_RGB_SRC_FACTOR(fd_blend_factor_no_dst_alpha(blend->base.rt[i].rgb_src_factor)) |
> +                                       A3XX_RB_MRT_BLEND_CONTROL_RGB_DEST_FACTOR(fd_blend_factor_no_dst_alpha(blend->base.rt[i].rgb_dst_factor));
> +                               control &= ~A3XX_RB_MRT_CONTROL_BLEND2;
> +                       }
> +
>                         OUT_PKT0(ring, REG_A3XX_RB_MRT_CONTROL(i), 1);
>                         OUT_RING(ring, control);
>
>                         OUT_PKT0(ring, REG_A3XX_RB_MRT_BLEND_CONTROL(i), 1);
> -                       OUT_RING(ring, blend->rb_mrt[i].blend_control |
> +                       OUT_RING(ring, blend_control |
>                                         COND(!is_float, A3XX_RB_MRT_BLEND_CONTROL_CLAMP_ENABLE));
>                 }
>         }
> diff --git a/src/gallium/drivers/freedreno/freedreno_util.c b/src/gallium/drivers/freedreno/freedreno_util.c
> index 9892b05..30bcf03 100644
> --- a/src/gallium/drivers/freedreno/freedreno_util.c
> +++ b/src/gallium/drivers/freedreno/freedreno_util.c
> @@ -111,6 +111,52 @@ fd_blend_factor(unsigned factor)
>         }
>  }
>
> +enum adreno_rb_blend_factor
> +fd_blend_factor_no_dst_alpha(unsigned factor)

maybe instead just map pipe blend factor to pipe blend factor, so for
no-alpha we do

  fd_blend_factor(noalpha_factor(factor))

vs

  fd_blend_factor(factor)

?

that should make it a shorter switch statement, plus seems kinda like
the thing others would need (so candidate to put in aux/util instead?)

BR,
-R

> +{
> +       switch (factor) {
> +       case PIPE_BLENDFACTOR_ONE:
> +               return FACTOR_ONE;
> +       case PIPE_BLENDFACTOR_SRC_COLOR:
> +               return FACTOR_SRC_COLOR;
> +       case PIPE_BLENDFACTOR_SRC_ALPHA:
> +               return FACTOR_SRC_ALPHA;
> +       case PIPE_BLENDFACTOR_DST_ALPHA:
> +               return FACTOR_ONE;
> +       case PIPE_BLENDFACTOR_DST_COLOR:
> +               return FACTOR_DST_COLOR;
> +       case PIPE_BLENDFACTOR_SRC_ALPHA_SATURATE:
> +               return FACTOR_SRC_ALPHA_SATURATE;
> +       case PIPE_BLENDFACTOR_CONST_COLOR:
> +               return FACTOR_CONSTANT_COLOR;
> +       case PIPE_BLENDFACTOR_CONST_ALPHA:
> +               return FACTOR_CONSTANT_ALPHA;
> +       case PIPE_BLENDFACTOR_ZERO:
> +       case 0:
> +               return FACTOR_ZERO;
> +       case PIPE_BLENDFACTOR_INV_SRC_COLOR:
> +               return FACTOR_ONE_MINUS_SRC_COLOR;
> +       case PIPE_BLENDFACTOR_INV_SRC_ALPHA:
> +               return FACTOR_ONE_MINUS_SRC_ALPHA;
> +       case PIPE_BLENDFACTOR_INV_DST_ALPHA:
> +               return FACTOR_ZERO;
> +       case PIPE_BLENDFACTOR_INV_DST_COLOR:
> +               return FACTOR_ONE_MINUS_DST_COLOR;
> +       case PIPE_BLENDFACTOR_INV_CONST_COLOR:
> +               return FACTOR_ONE_MINUS_CONSTANT_COLOR;
> +       case PIPE_BLENDFACTOR_INV_CONST_ALPHA:
> +               return FACTOR_ONE_MINUS_CONSTANT_ALPHA;
> +       case PIPE_BLENDFACTOR_INV_SRC1_COLOR:
> +       case PIPE_BLENDFACTOR_INV_SRC1_ALPHA:
> +       case PIPE_BLENDFACTOR_SRC1_COLOR:
> +       case PIPE_BLENDFACTOR_SRC1_ALPHA:
> +               /* I don't think these are supported */
> +       default:
> +               DBG("invalid blend factor: %x", factor);
> +               return 0;
> +       }
> +}
> +
>  enum adreno_pa_su_sc_draw
>  fd_polygon_mode(unsigned mode)
>  {
> diff --git a/src/gallium/drivers/freedreno/freedreno_util.h b/src/gallium/drivers/freedreno/freedreno_util.h
> index 0abc53f..111f45a 100644
> --- a/src/gallium/drivers/freedreno/freedreno_util.h
> +++ b/src/gallium/drivers/freedreno/freedreno_util.h
> @@ -46,6 +46,7 @@
>  enum adreno_rb_depth_format fd_pipe2depth(enum pipe_format format);
>  enum pc_di_index_size fd_pipe2index(enum pipe_format format);
>  enum adreno_rb_blend_factor fd_blend_factor(unsigned factor);
> +enum adreno_rb_blend_factor fd_blend_factor_no_dst_alpha(unsigned factor);
>  enum adreno_pa_su_sc_draw fd_polygon_mode(unsigned mode);
>  enum adreno_stencil_op fd_stencil_op(unsigned op);
>
> --
> 2.0.4
>


More information about the Freedreno mailing list