[Mesa-dev] [PATCH 16/16] i965/icl: Add render target flush after uploading binding table

[Anuj Phogat]

On Tue, Feb 13, 2018 at 4:17 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Tuesday, February 13, 2018 11:15:16 AM PST Anuj Phogat wrote:
>> From PIPE_CONTROL command description in gfxspecs:
>>
>> "Whenever a Binding Table Index (BTI) used by a Render Taget Message
>>  points to a different RENDER_SURFACE_STATE, SW must issue a Render
>>  Target Cache Flush by enabling this bit. When render target flush
>>  is set due to new association of BTI, PS Scoreboard Stall bit must
>>  be set in this packet."
>>
>> Fixes a fulsim error and a GPU hang described in below JIRA.
>>
>> JIRA: MD5-322
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_binding_tables.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c b/src/mesa/drivers/dri/i965/brw_binding_tables.c
>> index 73f5e56010..170daebc24 100644
>> --- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
>> +++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
>> @@ -93,6 +93,20 @@ brw_upload_binding_table(struct brw_context *brw,
>>        OUT_BATCH(stage_state->bind_bo_offset);
>>        ADVANCE_BATCH();
>>     }
>> +
>> +   /* From PIPE_CONTROL command description in gfxspecs:
>> +
>> +      "Whenever a Binding Table Index (BTI) used by a Render Taget Message
>> +       points to a different RENDER_SURFACE_STATE, SW must issue a Render
>> +       Target Cache Flush by enabling this bit. When render target flush
>> +       is set due to new association of BTI, PS Scoreboard Stall bit must
>> +       be set in this packet."
>> +   */
>> +   if (devinfo->gen >= 11) {
>> +      brw_emit_pipe_control_flush(brw,
>> +                                  PIPE_CONTROL_RENDER_TARGET_FLUSH |
>> +                                  PIPE_CONTROL_STALL_AT_SCOREBOARD);
>> +   }
>>  }
>>
>>  /**
>>
>
> This is overkill.  It'll do a RT flush when we change the binding
> tables in any stage, for any reason.  Only the pixel shader's binding
> tables have render target surfaces.  And, we might change other surfaces
> (textures, UBOs, etc) without changing the render targets.
>
> I would move this to update_renderbuffer_surfaces() in
> brw_wm_surface_state.c.

I agree. I wasn't very sure either if this is the right place to do
it. Thanks for the suggestion. I'll verify if it still fixes the fulsim
error and gpu hang.