On 2 December 2013 11:39, Francisco Jerez <curroje...@riseup.net> wrote:
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 2 +
> src/mesa/drivers/dri/i965/brw_eu.h | 4 ++
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 69
> ++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 ++
> src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 ++
> 8 files changed, 88 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 631473a..9e51e2c 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -784,6 +784,8 @@ enum opcode {
> SHADER_OPCODE_TYPED_SURFACE_READ,
> SHADER_OPCODE_TYPED_SURFACE_WRITE,
>
> + SHADER_OPCODE_MEMORY_FENCE,
> +
> SHADER_OPCODE_GEN4_SCRATCH_READ,
> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> SHADER_OPCODE_GEN7_SCRATCH_READ,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 17822ce..a47c730 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -408,6 +408,10 @@ brw_typed_surface_write(struct brw_compile *p,
> unsigned msg_length,
> unsigned num_channels);
>
> +void
> +brw_memory_fence(struct brw_compile *p,
> + struct brw_reg mrf);
> +
> /***********************************************************************
> * brw_eu_util.c:
> */
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 772be7a..3ee86c6 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2919,6 +2919,75 @@ brw_typed_surface_write(struct brw_compile *p,
> brw_send_indirect_message(p, sfid, dst, mrf, desc);
> }
>
> +static void
> +brw_set_memory_fence_message(struct brw_compile *p,
> + struct brw_instruction *insn,
> + enum brw_message_target sfid,
> + bool commit_enable)
> +{
> + brw_set_message_descriptor(p, insn, sfid,
> + 1 /* message length */,
> + (commit_enable ? 1 : 0) /* response length
> */,
> + true /* header present */,
> + false);
> +
> + switch (sfid) {
> + case GEN6_SFID_DATAPORT_RENDER_CACHE:
> + insn->bits3.gen7_dp.msg_type = GEN7_DATAPORT_RC_MEMORY_FENCE;
> + break;
> + case GEN7_SFID_DATAPORT_DATA_CACHE:
> + insn->bits3.gen7_dp.msg_type = GEN7_DATAPORT_DC_MEMORY_FENCE;
> + break;
> + default:
> + unreachable();
>
Change to an assert.
> + }
> +
> + if (commit_enable)
> + insn->bits3.ud |= 1 << 13;
> +}
> +
> +void
> +brw_memory_fence(struct brw_compile *p,
> + struct brw_reg mrf)
> +{
> + const bool commit_enable = !p->brw->is_haswell;
>
It would be nice to have a comment here with a pointer to Graphics BSpec:
3D-Media-GPGPU Engine > Shared Functions > Shared Functions – Data Port
[Pre-SKL] > Messages > Memory Fence, which explains why the commit enable
is needed on IVB and not HSW.
> + struct brw_instruction *insn;
> +
> + /* Set mrf as destination for dependency tracking, the MEMORY_FENCE
> + * message doesn't write anything back.
> + */
>
I don't think this comment is correct. The MEMORY_FENCE message *does*
write something back, if commit_enable is true. The particular value that
it writes back is reserved, but that's different from not writing anything
back at all.
> + insn = next_insn(p, BRW_OPCODE_SEND);
> + mrf = retype(mrf, BRW_REGISTER_TYPE_UD);
> + brw_set_dest(p, insn, mrf);
> + brw_set_src0(p, insn, mrf);
> + brw_set_memory_fence_message(p, insn, GEN7_SFID_DATAPORT_DATA_CACHE,
> + commit_enable);
> +
> + if (!p->brw->is_haswell) {
> + /* IVB does typed surface access through the render cache, so we
> + * need to flush that too. Use a different register so both
> + * flushes can be pipelined by the hardware.
> + */
> + insn = next_insn(p, BRW_OPCODE_SEND);
> + brw_set_dest(p, insn, offset(mrf, 1));
> + brw_set_src0(p, insn, offset(mrf, 1));
> + brw_set_memory_fence_message(p, insn,
> GEN6_SFID_DATAPORT_RENDER_CACHE,
> + commit_enable);
> +
> + /* Now write the response of the second message into the
> + * response of the first to trigger a pipeline stall -- This way
> + * future render and data cache messages will be properly
> + * ordered with respect to past data and render cache messages
> + * respectively.
> + */
> + brw_push_insn_state(p);
> + brw_set_compression_control(p, BRW_COMPRESSION_NONE);
> + brw_set_mask_control(p, BRW_MASK_DISABLE);
> + brw_MOV(p, mrf, offset(mrf, 1));
> + brw_pop_insn_state(p);
> + }
> +}
> +
> /**
> * This instruction is generated as a single-channel align1 instruction by
> * both the VS and FS stages when using INTEL_DEBUG=shader_time.
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 20cb4b9..cce6ed0 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -786,6 +786,8 @@ fs_visitor::implied_mrf_writes(fs_inst *inst)
> case SHADER_OPCODE_TYPED_SURFACE_READ:
> case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> return 0;
> + case SHADER_OPCODE_MEMORY_FENCE:
> + return 1;
> default:
> assert(!"not reached");
> return inst->mlen;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 9601183..5ead435 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1714,6 +1714,10 @@ fs_generator::generate_code(exec_list *instructions)
> src[0], inst->mlen, src[1].dw1.ud);
> break;
>
> + case SHADER_OPCODE_MEMORY_FENCE:
> + brw_memory_fence(p, brw_message_reg(inst->base_mrf));
> + break;
> +
> case FS_OPCODE_SET_SIMD4X2_OFFSET:
> generate_set_simd4x2_offset(inst, dst, src[0]);
> break;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index fc8d0ff..26300a6 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -663,6 +663,7 @@ backend_instruction::has_side_effects() const
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> case SHADER_OPCODE_TYPED_ATOMIC:
> case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> + case SHADER_OPCODE_MEMORY_FENCE:
> return true;
> default:
> return false;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 04054d5..e98fff5 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -298,6 +298,8 @@ vec4_visitor::implied_mrf_writes(vec4_instruction
> *inst)
> case SHADER_OPCODE_TYPED_SURFACE_READ:
> case SHADER_OPCODE_TYPED_SURFACE_WRITE:
> return 0;
> + case SHADER_OPCODE_MEMORY_FENCE:
> + return 1;
>
It's 1 just for IVB, right? (Since IVB writes to mrf and mrf+1, whereas
HSW just uses mrf).
With those issues fixed, this patch is:
Reviewed-by: Paul Berry <stereotype...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
