[Intel-gfx] [PATCH] drm/i915/execlists: stall on render flush before writing seqno

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Quoting Chris Wilson (2019-08-27 12:54:13)
>> Quite rarely we see that the CS completion event fires before the
>> breadcrumb is coherent. Try rearranging the breadcrumb write sequence
>> such that the CS_STALL is on the post-sync write pipecontrol.
>> 
>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_lrc.c | 17 ++++++++---------
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> index 80a3f1dbb456..669e8bd9f830 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> @@ -2961,18 +2961,17 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
>>  
>>  static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>>  {
>> -       cs = gen8_emit_ggtt_write_rcs(cs,
>> -                                     request->fence.seqno,
>> -                                     request->timeline->hwsp_offset,
>> -                                     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
>> -                                     PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>> -                                     PIPE_CONTROL_DC_FLUSH_ENABLE);
>> -
>>         /* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
>>         cs = gen8_emit_pipe_control(cs,
>> -                                   PIPE_CONTROL_FLUSH_ENABLE |
>> -                                   PIPE_CONTROL_CS_STALL,
>> +                                   PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
>> +                                   PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>> +                                   PIPE_CONTROL_DC_FLUSH_ENABLE,
>>                                     0);
>> +       cs = gen8_emit_ggtt_write_rcs(cs,
>> +                                     request->fence.seqno,
>> +                                     request->timeline->hwsp_offset,
>> +                                     PIPE_CONTROL_FLUSH_ENABLE |
>
> Or perhaps we need PIPE_CONTROL_DC_FLUSH_ENABLE here.
>
> I think that might make more sense (replace DC_FLUSH with whatever might
> flush the post-sync write).

Would it make sense to try to be as much similar as possible
with the ->emit_flush?

If so, iterating further, now with seqno being per context,
be exactly the same as emit_flush and start to use ppgtt seqnos?

-Mika