Ben Widawsky <benjamin.widaw...@intel.com> writes:
> Gen9 adds the ability to write out a stencil value, so we need to expand the
> virtual payload by one. Abstracting this now makes that change easier to read.
>
> I was admittedly confused early on about some of the hardcoding. If people
> believe the resulting code is inferior, I am not super attached to the patch.
>
> v2:
> Remove explicit numbering from the enumeration (Matt).
> Use a real naming scheme, and reference it in the opcode definition (Curro)
> - LOGICAL_SRC_SRC_DEPTH kinda sucks... but it's consistent
> Add a missed hardcoded logical position in get_lowered_simd_width (Ben)
> Add an assertion to make sure the component numbering is correct (Ben)
>
> Cc: Matt Turner <matts...@gmail.com>
> Cc: Francisco Jerez <curroje...@riseup.net>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 22 +++++++++++++---------
> src/mesa/drivers/dri/i965/brw_fs.cpp | 24 +++++++++++++-----------
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 1 +
> 3 files changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index e61ad54..a2f59ea 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -911,15 +911,9 @@ enum opcode {
>
> /**
> * Same as FS_OPCODE_FB_WRITE but expects its arguments separately as
> - * individual sources instead of as a single payload blob:
> - *
> - * Source 0: [required] Color 0.
> - * Source 1: [optional] Color 1 (for dual source blend messages).
> - * Source 2: [optional] Src0 Alpha.
> - * Source 3: [optional] Source Depth (gl_FragDepth)
> - * Source 4: [optional (gen4-5)] Destination Depth passthrough from thread
> - * Source 5: [optional] Sample Mask (gl_SampleMask).
> - * Source 6: [required] Number of color components (as a UD immediate).
> + * individual sources instead of as a single payload blob. The
> + * position/ordering of the arguments are defined by the enum
> + * fb_write_logical_srcs.
> */
> FS_OPCODE_FB_WRITE_LOGICAL,
>
> @@ -1318,6 +1312,16 @@ enum brw_urb_write_flags {
> BRW_URB_WRITE_ALLOCATE | BRW_URB_WRITE_COMPLETE,
> };
>
> +enum fb_write_logical_srcs {
> + FB_WRITE_LOGICAL_SRC_COLOR0, /* REQUIRED */
> + FB_WRITE_LOGICAL_SRC_COLOR1, /* for dual source blend messages */
> + FB_WRITE_LOGICAL_SRC_SRC0_ALPHA,
> + FB_WRITE_LOGICAL_SRC_SRC_DEPTH, /* gl_FragDepth */
> + FB_WRITE_LOGICAL_SRC_DST_DEPTH, /* GEN4-5: passthrough from thread */
> + FB_WRITE_LOGICAL_SRC_OMASK, /* Sample Mask (gl_SampleMask) */
> + FB_WRITE_LOGICAL_SRC_COMPONENTS, /* REQUIRED */
> +};
> +
> #ifdef __cplusplus
> /**
> * Allow brw_urb_write_flags enums to be ORed together.
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index da90467..ef06a70 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -695,10 +695,10 @@ fs_inst::components_read(unsigned i) const
> return 2;
>
> case FS_OPCODE_FB_WRITE_LOGICAL:
> - assert(src[6].file == IMM);
> + assert(src[FB_WRITE_LOGICAL_SRC_COMPONENTS].file == IMM);
> /* First/second FB write color. */
> if (i < 2)
> - return src[6].fixed_hw_reg.dw1.ud;
> + return src[FB_WRITE_LOGICAL_SRC_COMPONENTS].fixed_hw_reg.dw1.ud;
> else
> return 1;
>
> @@ -3339,15 +3339,16 @@ lower_fb_write_logical_send(const fs_builder &bld,
> fs_inst *inst,
> const brw_wm_prog_key *key,
> const fs_visitor::thread_payload &payload)
> {
> - assert(inst->src[6].file == IMM);
> + assert(inst->src[FB_WRITE_LOGICAL_SRC_COMPONENTS].file == IMM);
> const brw_device_info *devinfo = bld.shader->devinfo;
> - const fs_reg &color0 = inst->src[0];
> - const fs_reg &color1 = inst->src[1];
> - const fs_reg &src0_alpha = inst->src[2];
> - const fs_reg &src_depth = inst->src[3];
> - const fs_reg &dst_depth = inst->src[4];
> - fs_reg sample_mask = inst->src[5];
> - const unsigned components = inst->src[6].fixed_hw_reg.dw1.ud;
> + const fs_reg &color0 = inst->src[FB_WRITE_LOGICAL_SRC_COLOR0];
> + const fs_reg &color1 = inst->src[FB_WRITE_LOGICAL_SRC_COLOR1];
> + const fs_reg &src0_alpha = inst->src[FB_WRITE_LOGICAL_SRC_SRC0_ALPHA];
> + const fs_reg &src_depth = inst->src[FB_WRITE_LOGICAL_SRC_SRC_DEPTH];
> + const fs_reg &dst_depth = inst->src[FB_WRITE_LOGICAL_SRC_DST_DEPTH];
> + fs_reg sample_mask = inst->src[FB_WRITE_LOGICAL_SRC_OMASK];
> + const unsigned components =
> + inst->src[FB_WRITE_LOGICAL_SRC_COMPONENTS].fixed_hw_reg.dw1.ud;
>
> /* We can potentially have a message length of up to 15, so we have to set
> * base_mrf to either 0 or 1 in order to fit in m0..m15.
> @@ -4175,7 +4176,8 @@ get_lowered_simd_width(const struct brw_device_info
> *devinfo,
> /* Gen6 doesn't support SIMD16 depth writes but we cannot handle them
> * here.
> */
> - assert(devinfo->gen != 6 || inst->src[3].file == BAD_FILE ||
> + assert(devinfo->gen != 6 ||
> + inst->src[FB_WRITE_LOGICAL_SRC_SRC_DEPTH].file == BAD_FILE ||
> inst->exec_size == 8);
> /* Dual-source FB writes are unsupported in SIMD16 mode. */
> return (inst->src[1].file != BAD_FILE ? 8 : inst->exec_size);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index f825fed..87fdc32 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -709,6 +709,7 @@ fs_visitor::emit_single_fb_write(const fs_builder &bld,
> color0, color1, src0_alpha, src_depth, dst_depth, sample_mask,
> fs_reg(components)
> };
> + assert(ARRAY_SIZE(sources) - 1 == FB_WRITE_LOGICAL_SRC_COMPONENTS);
> fs_inst *write = bld.emit(FS_OPCODE_FB_WRITE_LOGICAL, fs_reg(),
> sources, ARRAY_SIZE(sources));
>
> --
> 2.6.1Thanks, Reviewed-by: Francisco Jerez <curroje...@riseup.net>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
