<div dir="ltr">On 2 December 2013 11:39, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">--- 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(); </blockquote><div> </div><div>Change to an assert. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + } + + 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; </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + struct brw_instruction *insn; + + /* Set mrf as destination for dependency tracking, the MEMORY_FENCE + * message doesn't write anything back. + */ </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + 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; </blockquote><div> </div><div>It's 1 just for IVB, right? (Since IVB writes to mrf and mrf+1, whereas HSW just uses mrf). </div><div>With those issues fixed, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div></div></div></div>