[Mesa-dev] [PATCH 1/2] i965/skl: Always use a header for SIMD4x2 sampler messages

Kristian Høgsberg krh at bitplanet.net
Wed Jan 7 22:42:17 PST 2015


On Tue, Dec 30, 2014 at 9:38 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Tue, Dec 30, 2014 at 9:24 PM, Kristian Høgsberg <krh at bitplanet.net> wrote:
>> SKL+ overloads the SIMD4x2 SIMD mode to mean either SIMD8D or SIMD4x2
>> depending on bit 22 in the message header.  If the bit is 0 or there is
>> no header we get SIMD8D.  We always wand SIMD4x2 in vec4 and for fs pull
>> constants, so always use a message header and set bit 22 there.
>>
>> Signed-off-by: Kristian Høgsberg <krh at clean-lemon.jf.intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp             |  8 +++++++
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 29 +++++++++++++++++++-----
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 16 +++++++++----
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |  4 +++-
>>  4 files changed, 46 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 78c068f..9a43d1a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -2994,6 +2994,14 @@ fs_visitor::lower_uniform_pull_constant_loads()
>>           const_offset_reg.fixed_hw_reg.dw1.ud /= 4;
>>           fs_reg payload = fs_reg(this, glsl_type::uint_type);
>>
>> +         /* We have to use a message header on Skylake to get SIMD4x2 mode.
>> +          * Reserve space for the register.
>> +          */
>> +         if (brw->gen >= 9) {
>> +            payload.reg_offset++;
>> +            virtual_grf_sizes[payload.reg] = 2;
>> +         }
>> +
>>           /* This is actually going to be a MOV, but since only the first dword
>>            * is accessed, we have a special opcode to do just that one.  Note
>>            * that this needs to be an operation that will be considered a def
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index c652d65..8becf84 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -1017,6 +1017,23 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
>>      */
>>     dst.width = BRW_WIDTH_4;
>>
>> +   struct brw_reg src = offset;
>> +   bool header_present = false;
>> +   int mlen = 1;
>> +
>> +   if (brw->gen >= 9) {
>> +      /* Skylake requires a message header in order to use SIMD4x2 mode. */
>> +      src = retype(brw_vec8_grf(offset.nr - 1, 0), BRW_REGISTER_TYPE_UD);
>> +      mlen = 2;
>> +      header_present = true;
>> +
>> +      brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> +      brw_MOV(p, src, retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
>> +      brw_set_default_access_mode(p, BRW_ALIGN_1);
>> +      brw_MOV(p, get_element_ud(src, 2), brw_imm_ud(1 << 22));
>> +      brw_pop_insn_state(p);
>
> I think you're missing a push_insn_state here.

Indeed, I lost that in the copy and paste.

> Having VPN troubles, so I haven't double checked that bit 22 is what
> we need to set, but we should give it a #define.

Sure.

>> +   }
>> +
>>     if (index.file == BRW_IMMEDIATE_VALUE) {
>>
>>        uint32_t surf_index = index.dw1.ud;
>> @@ -1028,14 +1045,14 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
>>        brw_pop_insn_state(p);
>>
>>        brw_set_dest(p, send, dst);
>> -      brw_set_src0(p, send, offset);
>> +      brw_set_src0(p, send, src);
>>        brw_set_sampler_message(p, send,
>>                                surf_index,
>>                                0, /* LD message ignores sampler unit */
>>                                GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
>>                                1, /* rlen */
>> -                              1, /* mlen */
>> -                              false, /* no header */
>> +                              mlen,
>> +                              header_present,
>>                                BRW_SAMPLER_SIMD_MODE_SIMD4X2,
>>                                0);
>>
>> @@ -1064,8 +1081,8 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
>>                                0 /* sampler */,
>>                                GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
>>                                1 /* rlen */,
>> -                              1 /* mlen */,
>> -                              false /* header */,
>> +                              mlen,
>> +                              header_present,
>>                                BRW_SAMPLER_SIMD_MODE_SIMD4X2,
>>                                0);
>>        brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1);
>> @@ -1077,7 +1094,7 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst,
>>        /* 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, offset);
>> +      brw_set_src0(p, insn_send, src);
>>        brw_set_indirect_send_descriptor(p, insn_send, BRW_SFID_SAMPLER, addr);
>>
>>        brw_pop_insn_state(p);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> index b88a579..e0213fe 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> @@ -328,6 +328,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
>>        } else {
>>           struct brw_reg header =
>>              retype(brw_message_reg(inst->base_mrf), BRW_REGISTER_TYPE_UD);
>> +         uint32_t dw2;
>>
>>           /* Explicitly set up the message header by copying g0 to the MRF. */
>>           brw_push_insn_state(p);
>> @@ -336,11 +337,18 @@ vec4_generator::generate_tex(vec4_instruction *inst,
>>
>>           brw_set_default_access_mode(p, BRW_ALIGN_1);
>>
>> -         if (inst->offset) {
>> +         dw2 = 0;
>
> I'd probably just declare the variable here and initialize it in one go.

Sure.

> Also... :)
>
> >> Signed-off-by: Kristian Høgsberg <krh at clean-lemon.jf.intel.com>

Yup, the MTA already yelled at me when it tried to Cc my broken
Signed-off-by email :)

Committed 2/2, v2 of this one is on the list.

Kristian


More information about the mesa-dev mailing list