[Mesa-dev] [PATCH V4 4/6] i965/fs: add generator support for pixel interpolator query

[Matt Turner]

On Sat, Jul 12, 2014 at 12:06 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Saturday, July 12, 2014 03:20:13 PM Chris Forbes wrote:
>> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
>> ---
>>  src/mesa/drivers/dri/i965/brw_defines.h        |  1 +
>>  src/mesa/drivers/dri/i965/brw_fs.cpp           |  2 ++
>>  src/mesa/drivers/dri/i965/brw_fs.h             |  4 ++++
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 19 +++++++++++++++++++
>>  src/mesa/drivers/dri/i965/brw_shader.h         |  6 ++++++
>>  5 files changed, 32 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
>> index e528232..b4428c1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>> @@ -853,6 +853,7 @@ enum opcode {
>>     FS_OPCODE_UNPACK_HALF_2x16_SPLIT_X,
>>     FS_OPCODE_UNPACK_HALF_2x16_SPLIT_Y,
>>     FS_OPCODE_PLACEHOLDER_HALT,
>> +   FS_OPCODE_PIXEL_INTERPOLATOR_QUERY,
>>
>>     VS_OPCODE_URB_WRITE,
>>     VS_OPCODE_PULL_CONSTANT_LOAD,
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index a3ad375..f431eff 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -370,6 +370,7 @@ fs_inst::is_send_from_grf() const
>>  {
>>     return (opcode == FS_OPCODE_VARYING_PULL_CONSTANT_LOAD_GEN7 ||
>>             opcode == SHADER_OPCODE_SHADER_TIME_ADD ||
>> +           opcode == FS_OPCODE_PIXEL_INTERPOLATOR_QUERY ||
>>             (opcode == FS_OPCODE_UNIFORM_PULL_CONSTANT_LOAD &&
>>              src[1].file == GRF) ||
>>             (is_tex() && src[0].file == GRF));
>> @@ -837,6 +838,7 @@ fs_visitor::implied_mrf_writes(fs_inst *inst)
>>        return 2;
>>     case SHADER_OPCODE_UNTYPED_ATOMIC:
>>     case SHADER_OPCODE_UNTYPED_SURFACE_READ:
>> +   case FS_OPCODE_PIXEL_INTERPOLATOR_QUERY:
>>        return 0;
>>     default:
>>        unreachable("not reached");
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 537f10e..48f1069 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -623,6 +623,10 @@ private:
>>                                                   struct brw_reg offset);
>>     void generate_mov_dispatch_to_flags(fs_inst *inst);
>>
>> +   void generate_pixel_interpolator_query(fs_inst *inst,
>> +                                          struct brw_reg dst,
>> +                                          struct brw_reg src);
>> +
>>     void generate_set_omask(fs_inst *inst,
>>                             struct brw_reg dst,
>>                             struct brw_reg sample_mask);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index eae55f0..fcc9a7d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -995,6 +995,21 @@ fs_generator::generate_mov_dispatch_to_flags(fs_inst
> *inst)
>>     brw_pop_insn_state(p);
>>  }
>>
>> +void
>> +fs_generator::generate_pixel_interpolator_query(fs_inst *inst,
>> +                                                struct brw_reg dst,
>> +                                                struct brw_reg src)
>> +{
>> +   brw_pixel_interpolator_query(p,
>> +         retype(dst, BRW_REGISTER_TYPE_UW),
>> +         src,
>> +         inst->pi_noperspective,
>> +         inst->pi_msg_type,
>> +         inst->pi_msg_data,
>> +         inst->mlen,
>> +         inst->regs_written);
>> +}
>> +
>>
>>  static uint32_t brw_file_from_reg(fs_reg *reg)
>>  {
>> @@ -1730,6 +1745,10 @@ fs_generator::generate_code(exec_list *instructions)
>>           }
>>           break;
>>
>> +      case FS_OPCODE_PIXEL_INTERPOLATOR_QUERY:
>> +         generate_pixel_interpolator_query(inst, dst, src[0]);
>> +         break;
>> +
>>        default:
>>        if (inst->opcode < (int) ARRAY_SIZE(opcode_descs)) {
>>           _mesa_problem(ctx, "Unsupported opcode `%s' in FS",
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h
> b/src/mesa/drivers/dri/i965/brw_shader.h
>> index cfaea9e..6ccc148 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.h
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
>> @@ -112,6 +112,12 @@ struct backend_instruction {
>>     uint8_t mlen; /**< SEND message length */
>>     int8_t base_mrf; /**< First MRF in the SEND message, if mlen is nonzero.
> */
>>     uint8_t target; /**< MRT target. */
>> +   struct {
>> +         /* Descriptor parameters for pixel interpolator messages */
>> +         uint32_t pi_noperspective: 1;
>> +         uint32_t pi_msg_type: 3;
>> +         uint32_t pi_msg_data: 8;
>> +   };
>
> It seems like backend_instruction is the wrong place for these - it's shared
> between VS/GS/HS/DS/FS, and interpolateAt only exists in the FS.  It seems
> like they should just go in fs_inst, instead.
>
> That said, I also think we could eliminate pi_msg_type and pi_msg_data.
>
> For pi_msg_type, it seems like we could just create four separate opcodes:
> - FS_OPCODE_INTERPOLATE_AT_CENTROID
> - FS_OPCODE_INTERPOLATE_AT_SAMPLE_POS
> - ...
> (or whatever you want to call them)
>
> I would just encode pi_msg_data in src[1], which would be an fs_reg of IMM
> type.  That's what we do for pull constant offsets and other things.
>
> Setting pi_noperspective as a flag in fs_inst is probably reasonable.
>
> Perhaps Matt has an opinion as well.

Sounds reasonable to me.