[Mesa-dev] [PATCH 01/10] i965: Use Ivybridge's "Legacy Data Port" for reads/writes.

[Kenneth Graunke]

On 10/11/2011 09:13 PM, Eric Anholt wrote:
> On Mon, 10 Oct 2011 16:31:44 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> Using the constant cache for reads isn't going to work for scratch
>> reads (variably-indexed arrays or register spills), as these aren't
>> constant at all.
>>
>> Also, in the new VS backend, use the proper message number for OWord
>> Dual Block Write messages.  It's now 10, instead of 9.  Prior to this,
>> we were attempting to use the render cache with message 9, which was
>> "Memory Barrier."  Not quite the desired effect.

Hmm, this part of the commit message is crap.  Memory Fence is 7.  I
must've been lost in documentation somewhere.

>> +205 piglits.
>>
>> NOTE: This is a candidate for the 7.11 branch.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> index 5caebfc..bcd640e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -546,6 +546,13 @@ brw_set_dp_write_message(struct brw_compile *p,
>>     brw_set_src1(p, insn, brw_imm_ud(0));
>>  
>>     if (intel->gen >= 7) {
>> +      /* Use the Render Cache for RT writes; otherwise use the Data Cache */
>> +      unsigned sfid = GEN7_MESSAGE_TARGET_DP_DATA_CACHE;
>> +      if (msg_type == GEN6_DATAPORT_WRITE_MESSAGE_RENDER_TARGET_WRITE)
>> +	 sfid = GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
>> +
>> +      insn->header.destreg__conditionalmod = sfid;
>> +
>>        insn->bits3.gen7_dp.binding_table_index = binding_table_index;
>>        insn->bits3.gen7_dp.msg_control = msg_control;
>>        insn->bits3.gen7_dp.pixel_scoreboard_clear = pixel_scoreboard_clear;
>> @@ -554,9 +561,6 @@ brw_set_dp_write_message(struct brw_compile *p,
>>        insn->bits3.gen7_dp.response_length = response_length;
>>        insn->bits3.gen7_dp.msg_length = msg_length;
>>        insn->bits3.gen7_dp.end_of_thread = end_of_thread;
>> -
>> -      /* We always use the render cache for write messages */
>> -      insn->header.destreg__conditionalmod = GEN6_MESSAGE_TARGET_DP_RENDER_CACHE;
>>     } else if (intel->gen == 6) {
>>        insn->bits3.gen6_dp.binding_table_index = binding_table_index;
>>        insn->bits3.gen6_dp.msg_control = msg_control;
>> @@ -618,7 +622,7 @@ brw_set_dp_read_message(struct brw_compile *p,
>>        insn->bits3.gen7_dp.response_length = response_length;
>>        insn->bits3.gen7_dp.msg_length = msg_length;
>>        insn->bits3.gen7_dp.end_of_thread = 0;
>> -      insn->header.destreg__conditionalmod = GEN6_MESSAGE_TARGET_DP_CONST_CACHE;
>> +      insn->header.destreg__conditionalmod = GEN7_MESSAGE_TARGET_DP_DATA_CACHE;
> 
> So you're writing through the render cache but reading through the data
> cache?  I'm amazed this works.  Seems like you want to be doing what
> gen6 does here, and use render cache if asked for it in the target_cache
> argument.

I think you may have misread...

The hunk in brw_set_dp_write_message (first in this mail) uses the
Render Cache _only_ for FB writes (the "Render Target Write" message).
For everything else, writes go through the Data Cache.

For reads (this hunk), I always use the Data Cache.  IVB's render cache
does not even have a "OWord Dual Block Read" message; I'd have to use
the new "Typed Surface Read" operation...which doesn't exist on the Data
Cache...

When would you want to read through the render cache anyway?  I'm
thinking of these uses of the data port:
- Register spilling (write/read through DC)
- Variable-indexed arrays (read/write through DC)
- Pull constants (never written; read through DC for now, ideally switch
  to Constant Cache later)
- FB writes (write through Render Cache; never read)

I'm not seeing a problem here...

>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
>> index ccbad25..750b6c3 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp
>> @@ -450,7 +450,9 @@ vec4_visitor::generate_scratch_write(vec4_instruction *inst,
>>  
>>     uint32_t msg_type;
>>  
>> -   if (intel->gen >= 6)
>> +   if (intel->gen >= 7)
>> +      msg_type = GEN7_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
>> +   else if (intel->gen == 6)
>>        msg_type = GEN6_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
>>     else
>>        msg_type = BRW_DATAPORT_WRITE_MESSAGE_OWORD_DUAL_BLOCK_WRITE;
> 
> This hunk (and the hunk of brw_defines.h, and the paragraph of commit
> message) looks like a separate commit.

Hmmmmm.  I was trying to avoid intermediate breakage.  I was trying to
avoid sending a Reserved message type (1001) to the DC DP, but...now
that I re-read, I realize I wasn't sending Memory Barrier after all..but
sending a reserved message type to the RC DP.

Heh.  I'll split it back out.  Thanks.