On Wed, Mar 11, 2015 at 07:25:14PM +0200, Francisco Jerez wrote:
> "Pohjolainen, Topi" <topi.pohjolai...@intel.com> writes:
>
> > On Fri, Feb 27, 2015 at 05:34:44PM +0200, Francisco Jerez wrote:
> >> ---
> >> src/mesa/drivers/dri/i965/brw_eu.h | 19 ++++++--
> >> src/mesa/drivers/dri/i965/brw_eu_emit.c | 58
> >> ++++++++++++++++++------
> >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 55
> >> +++++-----------------
> >> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 37 ++++-----------
> >> 4 files changed, 77 insertions(+), 92 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> >> index 1b954c8..9b1e0e2 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> @@ -205,11 +205,6 @@ void brw_set_sampler_message(struct brw_compile *p,
> >> unsigned simd_mode,
> >> unsigned return_format);
> >>
> >> -void brw_set_indirect_send_descriptor(struct brw_compile *p,
> >> - brw_inst *insn,
> >> - unsigned sfid,
> >> - struct brw_reg descriptor);
> >> -
> >> void brw_set_dp_read_message(struct brw_compile *p,
> >> brw_inst *insn,
> >> unsigned binding_table_index,
> >> @@ -243,6 +238,20 @@ void brw_urb_WRITE(struct brw_compile *p,
> >> unsigned offset,
> >> unsigned swizzle);
> >>
> >> +/**
> >> + * Send message to shared unit \p sfid with a possibly indirect
> >> descriptor \p
> >> + * desc. If the descriptor is not an immediate it will be transparently
> >> + * loaded to an address register using an OR instruction that will be
> >> returned
> >> + * to the caller so additional descriptor bits can be specified with the
> >> usual
> >> + * brw_set_*_message() helper functions.
> >> + */
> >> +struct brw_inst *
> >> +brw_send_indirect_message(struct brw_compile *p,
> >> + unsigned sfid,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg desc);
> >> +
> >> void brw_ff_sync(struct brw_compile *p,
> >> struct brw_reg dest,
> >> unsigned msg_reg_nr,
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> index e69840a..cd2ce92 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> @@ -751,21 +751,6 @@ brw_set_sampler_message(struct brw_compile *p,
> >> }
> >> }
> >>
> >> -void brw_set_indirect_send_descriptor(struct brw_compile *p,
> >> - brw_inst *insn,
> >> - unsigned sfid,
> >> - struct brw_reg descriptor)
> >> -{
> >> - /* Only a0.0 may be used as SEND's descriptor operand. */
> >> - assert(descriptor.file == BRW_ARCHITECTURE_REGISTER_FILE);
> >> - assert(descriptor.type == BRW_REGISTER_TYPE_UD);
> >> - assert(descriptor.nr == BRW_ARF_ADDRESS);
> >> - assert(descriptor.subnr == 0);
> >> -
> >> - brw_set_message_descriptor(p, insn, sfid, 0, 0, false, false);
> >> - brw_set_src1(p, insn, descriptor);
> >> -}
> >> -
> >> static void
> >> gen7_set_dp_scratch_message(struct brw_compile *p,
> >> brw_inst *inst,
> >> @@ -2490,6 +2475,49 @@ void brw_urb_WRITE(struct brw_compile *p,
> >> swizzle);
> >> }
> >>
> >> +struct brw_inst *
> >> +brw_send_indirect_message(struct brw_compile *p,
> >> + unsigned sfid,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg desc)
> >> +{
> >> + const struct brw_context *brw = p->brw;
> >> + struct brw_inst *send, *setup;
> >> +
> >> + assert(desc.type == BRW_REGISTER_TYPE_UD);
> >> +
> >> + if (desc.file == BRW_IMMEDIATE_VALUE) {
> >> + setup = send = next_insn(p, BRW_OPCODE_SEND);
> >> + brw_set_src1(p, send, desc);
> >> +
> >> + } else {
> >> + struct brw_reg addr = retype(brw_address_reg(0),
> >> BRW_REGISTER_TYPE_UD);
> >> +
> >> + brw_push_insn_state(p);
> >> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> >> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> >> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> >> +
> >> + /* Load the indirect descriptor to an address register using OR so
> >> the
> >> + * caller can specify additional descriptor bits with the usual
> >> + * brw_set_*_message() helper functions.
> >> + */
> >> + setup = brw_OR(p, addr, desc, brw_imm_ud(0));
> >> +
> >> + brw_pop_insn_state(p);
> >> +
> >> + send = next_insn(p, BRW_OPCODE_SEND);
> >> + brw_set_src1(p, send, addr);
> >> + }
> >> +
> >> + brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UD));
> >> + brw_set_src0(p, send, retype(payload, BRW_REGISTER_TYPE_UD));
> >> + brw_inst_set_sfid(brw, send, sfid);
> >> +
> >> + return setup;
> >> +}
> >> +
> >> static int
> >> brw_find_next_block_end(struct brw_compile *p, int start_offset)
> >> {
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> index 2ebbfe6..a54a274 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> >> @@ -754,9 +754,10 @@ fs_generator::generate_tex(fs_inst *inst, struct
> >> brw_reg dst, struct brw_reg src
> >> brw_AND(p, addr, addr, brw_imm_ud(0x0ff));
> >> brw_OR(p, addr, addr, temp);
> >>
> >> - /* a0.0 |= <descriptor> */
> >> - brw_inst *insn_or = brw_next_insn(p, BRW_OPCODE_OR);
> >> - brw_set_sampler_message(p, insn_or,
> >> + /* dst = send(offset, a0.0 | <descriptor>) */
> >> + brw_inst *insn = brw_send_indirect_message(
> >> + p, BRW_SFID_SAMPLER, dst, src, addr);
> >> + brw_set_sampler_message(p, insn,
> >> 0 /* surface */,
> >> 0 /* sampler */,
> >> msg_type,
> >> @@ -765,17 +766,6 @@ fs_generator::generate_tex(fs_inst *inst, struct
> >> brw_reg dst, struct brw_reg src
> >> inst->header_present /* header */,
> >> simd_mode,
> >> return_format);
> >> - brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1);
> >> - brw_inst_set_src1_reg_type(p->brw, insn_or, BRW_REGISTER_TYPE_UD);
> >> - brw_set_src0(p, insn_or, addr);
> >> - brw_set_dest(p, insn_or, addr);
> >> -
> >> -
> >> - /* dst = send(offset, a0.0) */
> >> - brw_inst *insn_send = brw_next_insn(p, BRW_OPCODE_SEND);
> >> - brw_set_dest(p, insn_send, dst);
> >> - brw_set_src0(p, insn_send, src);
> >> - brw_set_indirect_send_descriptor(p, insn_send, BRW_SFID_SAMPLER,
> >> addr);
> >>
> >> brw_pop_insn_state(p);
> >>
> >> @@ -1049,9 +1039,10 @@
> >> fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
> >> brw_set_src0(p, insn_and, vec1(retype(index,
> >> BRW_REGISTER_TYPE_UD)));
> >> brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));
> >>
> >> - /* a0.0 |= <descriptor> */
> >> - brw_inst *insn_or = brw_next_insn(p, BRW_OPCODE_OR);
> >> - brw_set_dp_read_message(p, insn_or,
> >> + /* dst = send(payload, a0.0 | <descriptor>) */
> >> + brw_inst *insn = brw_send_indirect_message(
> >> + p, GEN6_SFID_DATAPORT_CONSTANT_CACHE, vec16(dst), payload, addr);
> >> + brw_set_dp_read_message(p, insn,
> >> 0, /* surface */
> >> BRW_DATAPORT_OWORD_BLOCK_4_OWORDS,
> >> GEN7_DATAPORT_DC_OWORD_BLOCK_READ,
> >> @@ -1059,17 +1050,6 @@
> >> fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
> >> 1, /* mlen */
> >> true, /* header */
> >> 2); /* rlen */
> >> - brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1);
> >> - brw_inst_set_src1_reg_type(p->brw, insn_or, BRW_REGISTER_TYPE_UD);
> >> - brw_set_src0(p, insn_or, addr);
> >> - brw_set_dest(p, insn_or, addr);
> >> -
> >> - /* dst = send(payload, a0.0) */
> >> - brw_inst *insn_send = brw_next_insn(p, BRW_OPCODE_SEND);
> >> - brw_set_dest(p, insn_send, vec16(dst));
> >> - brw_set_src0(p, insn_send, payload);
> >> - brw_set_indirect_send_descriptor(p, insn_send,
> >> - GEN6_SFID_DATAPORT_CONSTANT_CACHE,
> >> addr);
> >>
> >> brw_pop_insn_state(p);
> >>
> >> @@ -1206,10 +1186,10 @@
> >> fs_generator::generate_varying_pull_constant_load_gen7(fs_inst *inst,
> >> brw_set_src0(p, insn_and, vec1(retype(index,
> >> BRW_REGISTER_TYPE_UD)));
> >> brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));
> >>
> >> -
> >> - /* a0.0 |= <descriptor> */
> >> - brw_inst *insn_or = brw_next_insn(p, BRW_OPCODE_OR);
> >> - brw_set_sampler_message(p, insn_or,
> >> + /* dst = send(offset, a0.0 | <descriptor>) */
> >> + brw_inst *insn = brw_send_indirect_message(
> >> + p, BRW_SFID_SAMPLER, dst, offset, addr);
> >> + brw_set_sampler_message(p, insn,
> >> 0 /* surface */,
> >> 0 /* sampler */,
> >> GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
> >> @@ -1218,17 +1198,6 @@
> >> fs_generator::generate_varying_pull_constant_load_gen7(fs_inst *inst,
> >> false /* header */,
> >> simd_mode,
> >> 0);
> >> - brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1);
> >> - brw_inst_set_src1_reg_type(p->brw, insn_or, BRW_REGISTER_TYPE_UD);
> >> - brw_set_src0(p, insn_or, addr);
> >> - brw_set_dest(p, insn_or, addr);
> >> -
> >> -
> >> - /* dst = send(offset, a0.0) */
> >> - brw_inst *insn_send = brw_next_insn(p, BRW_OPCODE_SEND);
> >> - brw_set_dest(p, insn_send, retype(dst, BRW_REGISTER_TYPE_UW));
> >
> > I'm just reading this through again and noticed that the destination type
> > changes here to BRW_REGISTER_TYPE_UD (set in brw_send_indirect_message()).
>
> Ah, yes, that change is intentional. The type being set to UW was a
> remnant from Gen4-5 times -- Those used to require the destination type
> of SEND to be W/UW when doing 16-wide (even if the message was actually
> writing dwords back...). None of the codepaths modified in this patch
> (or in the rest of the series) should be executed on Gen4 or 5.
>
> Anyway good catch. :)
This might be a good thing to add to the commit message. Anyway it has my
r-b - I'm fine with your reasoning after the long debate. I'll just post some
comments back to the thread and then I'll continue reading patch number five.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
