[Mesa-dev] [PATCH V4 3/6] i965: add low-level support for send to pixel interpolator

[Chris Forbes]

Yeah, I can inline brw_set_pixel_interpolator_query_message into its one caller.

On Sat, Jul 12, 2014 at 6:56 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Saturday, July 12, 2014 03:20:12 PM Chris Forbes wrote:
>> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
>> ---
>>  src/mesa/drivers/dri/i965/brw_eu.h      | 10 ++++++++
>>  src/mesa/drivers/dri/i965/brw_eu_emit.c | 43
> +++++++++++++++++++++++++++++++++
>>  2 files changed, 53 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
>> index 3164c80..9ae09d7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> @@ -372,6 +372,16 @@ brw_untyped_surface_read(struct brw_compile *p,
>>                           unsigned msg_length,
>>                           unsigned response_length);
>>
>> +void
>> +brw_pixel_interpolator_query(struct brw_compile *p,
>> +                             struct brw_reg dest,
>> +                             struct brw_reg mrf,
>> +                             bool noperspective,
>> +                             GLuint mode,
>> +                             GLuint data,
>> +                             GLuint msg_length,
>> +                             GLuint response_length);
>> +
>>  /***********************************************************************
>>   * 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 3f00e4d..0a1a223 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -2633,6 +2633,49 @@ brw_untyped_surface_read(struct brw_compile *p,
>>        brw_inst_access_mode(brw, insn) == BRW_ALIGN_1);
>>  }
>>
>> +static void
>> +brw_set_pixel_interpolator_query_message(struct brw_compile *p,
>> +                                         struct brw_inst *insn,
>> +                                         bool noperspective,
>> +                                         GLuint mode,
>> +                                         GLuint data,
>> +                                         GLuint msg_length,
>> +                                         GLuint response_length)
>> +{
>> +   const struct brw_context *brw = p->brw;
>> +   const unsigned dispatch_width =
>> +      (brw_inst_exec_size(brw, insn) == BRW_EXECUTE_16 ? 16 : 8);
>> +
>> +   brw_set_message_descriptor(p, insn, GEN7_SFID_PIXEL_INTERPOLATOR,
>> +                              msg_length, response_length,
>> +                              false /* header is never present for PI */,
>> +                              false);
>> +
>> +   brw_inst_set_pi_simd_mode(brw, insn, dispatch_width == 16);
>> +   brw_inst_set_pi_slot_group(brw, insn, 0); /* zero unless 32/64px
> dispatch */
>> +   brw_inst_set_pi_nopersp(brw, insn, noperspective);
>> +   brw_inst_set_pi_message_type(brw, insn, mode);
>> +   brw_inst_set_pi_message_data(brw, insn, data);
>> +}
>> +
>> +void
>> +brw_pixel_interpolator_query(struct brw_compile *p,
>> +                             struct brw_reg dest,
>> +                             struct brw_reg mrf,
>> +                             bool noperspective,
>> +                             GLuint mode,
>> +                             GLuint data,
>> +                             GLuint msg_length,
>> +                             GLuint response_length)
>> +{
>> +   struct brw_inst *insn = next_insn(p, BRW_OPCODE_SEND);
>> +
>> +   brw_set_dest(p, insn, dest);
>> +   brw_set_src0(p, insn, mrf);
>> +   brw_set_pixel_interpolator_query_message(
>> +      p, insn, noperspective, mode, data, msg_length, response_length);
>> +}
>> +
>>  /**
>>   * This instruction is generated as a single-channel align1 instruction by
>>   * both the VS and FS stages when using INTEL_DEBUG=shader_time.
>>
>
> What do you think about combining these two functions?  I know we've always
> defined separate brw_set_*_message functions, but I'm not really convinced
> that it's useful to do so.  I can't imagine
> brw_set_pixel_interpolator_query_message would be called by anything other
> than brw_pixel_interpolator_query.
>
> Either way's fine - it's just an idea.
>
> Patches 1-3 are:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>