[Mesa-dev] [PATCH] i965/fs: Implement SIMD16 dual source blending.
Jason Ekstrand
jason at jlekstrand.net
Wed Sep 17 11:39:21 PDT 2014
I haven't tested this yet, just looked it over. I've got a couple of
inline comments below. One general comment though: I'm currently working
on a bunch of compiler stuff that reworks the way we do FB writes. In
particular, it reworks things to use GRF registers instead of the MRF. It
probably wouldn't be too bad for me to rebase on top of this or to rebase
your patch on top of what I'm doing. I just thought I'd warn you about the
conflict.
On Wed, Sep 17, 2014 at 4:36 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:
> From the SNB PRM, volume 4, part 1, page 193:
> "The dual source render target messages only have SIMD8 forms due to
> maximum message length limitations. SIMD16 pixel shaders must send two of
> these messages to cover all of the pixels. Each message contains two colors
> (4 channels each) for each pixel in the message payload."
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82831
> ---
> src/mesa/drivers/dri/i965/brw_eu.h | 1 +
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 3 +-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 14 +++++++--
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 41
> +++++++++++++++++++-------
> 4 files changed, 45 insertions(+), 14 deletions(-)
>
> I tested this on SandyBridge and IvyBridge. No piglit regressions in these
> platforms, but would be nice if someone could test this in later platforms
> too.
>
> I only noticed these two tests for dual source blending in piglit though:
> tests/spec/ext_framebuffer_multisample/alpha-to-one-dual-src-blend.cpp
> tests/spec/ext_framebuffer_multisample/alpha-to-coverage-dual-src-blend.cpp
>
> The first one fails, in both platforms with and without my patch. The
> second one
> passes in both platforms, with and without my patch.
>
> I also tested this with a seprate test program to verify that it worked, at
> least, in a simple case.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index e6c26e3..5908ba5 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -266,6 +266,7 @@ void brw_fb_WRITE(struct brw_compile *p,
> unsigned msg_length,
> unsigned response_length,
> bool eot,
> + bool last_render_target,
> bool header_present);
>
> void brw_SAMPLE(struct brw_compile *p,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 39f94e9..ffdbe6d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2251,6 +2251,7 @@ void brw_fb_WRITE(struct brw_compile *p,
> unsigned msg_length,
> unsigned response_length,
> bool eot,
> + bool last_render_target,
> bool header_present)
> {
> struct brw_context *brw = p->brw;
> @@ -2290,7 +2291,7 @@ void brw_fb_WRITE(struct brw_compile *p,
> msg_type,
> msg_length,
> header_present,
> - eot, /* last render target write */
> + last_render_target,
> response_length,
> eot,
> 0 /* send_commit_msg */);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 1bc10f5..a4b84aa 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -121,9 +121,12 @@ fs_generator::fire_fb_write(fs_inst *inst,
>
> if (inst->opcode == FS_OPCODE_REP_FB_WRITE)
> msg_control =
> BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD16_SINGLE_SOURCE_REPLICATED;
> - else if (prog_data->dual_src_blend)
> - msg_control =
> BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD8_DUAL_SOURCE_SUBSPAN01;
> - else if (dispatch_width == 16)
> + else if (prog_data->dual_src_blend) {
> + if (dispatch_width == 8 || !inst->eot)
> + msg_control =
> BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD8_DUAL_SOURCE_SUBSPAN01;
> + else
> + msg_control =
> BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD8_DUAL_SOURCE_SUBSPAN23;
> + } else if (dispatch_width == 16)
> msg_control = BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD16_SINGLE_SOURCE;
> else
> msg_control =
> BRW_DATAPORT_RENDER_TARGET_WRITE_SIMD8_SINGLE_SOURCE_SUBSPAN01;
> @@ -131,6 +134,9 @@ fs_generator::fire_fb_write(fs_inst *inst,
> uint32_t surf_index =
> prog_data->binding_table.render_target_start + inst->target;
>
> + bool last_render_target = inst->eot ||
> + (prog_data->dual_src_blend && dispatch_width
> == 16);
> +
> brw_fb_WRITE(p,
> dispatch_width,
> base_reg,
> @@ -140,6 +146,7 @@ fs_generator::fire_fb_write(fs_inst *inst,
> nr,
> 0,
> inst->eot,
> + last_render_target,
> inst->header_present);
>
> brw_mark_surface_used(&prog_data->base, surf_index);
> @@ -254,6 +261,7 @@ fs_generator::generate_blorp_fb_write(fs_inst *inst)
> inst->mlen,
> 0,
> true,
> + true,
> inst->header_present);
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 5d2e7c8..21dc5bc 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -3064,12 +3064,6 @@ fs_visitor::emit_fb_writes()
> int reg_width = dispatch_width / 8;
> bool src0_alpha_to_render_target = false;
>
> - if (do_dual_src) {
> - no16("GL_ARB_blend_func_extended not yet supported in SIMD16.");
> - if (dispatch_width == 16)
> - do_dual_src = false;
> - }
> -
> /* From the Sandy Bridge PRM, volume 4, page 198:
> *
> * "Dispatched Pixel Enables. One bit per pixel indicating
> @@ -3109,11 +3103,22 @@ fs_visitor::emit_fb_writes()
> nr += 1;
> }
>
> - /* Reserve space for color. It'll be filled in per MRT below. */
> + /* Reserve space for color. It'll be filled in per MRT below.
> + *
> + * From the SNB PRM, volume 4, part 1, page 193:
> + * "The dual source render target messages only have SIMD8 forms due to
> + * maximum message length limitations. SIMD16 pixel shaders must send
> two of
> + * these messages to cover all of the pixels. Each message contains two
> + * colors (4 channels each) for each pixel in the message payload."
> + *
> + * So color data in dual source mode, whether this is a SIMD8 or SIMD16
> + * program, always requires 8 MRF registers.
> + */
> int color_mrf = nr;
> - nr += 4 * reg_width;
> if (do_dual_src)
> - nr += 4;
> + nr += 8;
> + else
> + nr += 4 * reg_width;
> if (src0_alpha_to_render_target)
> nr += reg_width;
>
> @@ -3173,13 +3178,29 @@ fs_visitor::emit_fb_writes()
> inst->target = 0;
> inst->base_mrf = base_mrf;
> inst->mlen = nr - base_mrf;
> - inst->eot = true;
> + inst->eot = dispatch_width == 8;
> inst->header_present = header_present;
> if ((brw->gen >= 8 || brw->is_haswell) && prog_data->uses_kill) {
> inst->predicate = BRW_PREDICATE_NORMAL;
> inst->flag_subreg = 1;
> }
>
It appears as if you don't copy the second half of the colors into the
message before doing the second send. Is there a reason for this? It's
quite possible that the piglit tests are only using solid colors in which
case they might not catch this.
+ /* SIMD16 dual source blending requires to send two SIMD8 dual source
> + * messages, so here goes the second one.
> + */
> + if (dispatch_width == 16) {
> + fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
> + inst->target = 0;
> + inst->base_mrf = base_mrf;
> + inst->mlen = nr - base_mrf;
> + inst->eot = true;
> + inst->header_present = header_present;
> + if ((brw->gen >= 8 || brw->is_haswell) && prog_data->uses_kill) {
> + inst->predicate = BRW_PREDICATE_NORMAL;
> + inst->flag_subreg = 1;
> + }
> + }
> +
> prog_data->dual_src_blend = true;
> this->current_annotation = NULL;
> return;
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140917/09298d43/attachment-0001.html>
More information about the mesa-dev
mailing list