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

2019-08-28 Thread Chris Wilson 
Quoting Mika Kuoppala (2019-08-28 10:00:35)
> Chris Wilson  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 
> >> Cc: Mika Kuoppala 
> >> ---
> >>  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?

That's where we started and had to refine due to being able to detect
incoherency. It's not as if we we used emit_flush(EMIT_FLUSH) for
anything other than a delay in emit_pdp...

drivers/gpu/drm/i915/gem/i915_gem_context.c:err = 
engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_lrc.c:ret = 
request->engine->emit_flush(request, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c: ret = 
engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c: ret = 
engine->emit_flush(rq, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c: ret = 
engine->emit_flush(rq, EMIT_FLUSH);
drivers/gpu/drm/i915/gt/intel_ringbuffer.c: ret = 
request->engine->emit_flush(request, EMIT_INVALIDATE);
drivers/gpu/drm/i915/gt/intel_workarounds.c:ret = 
rq->engine->emit_flush(rq, EMIT_BARRIER);
drivers/gpu/drm/i915/gt/intel_workarounds.c:ret = 
rq->engine->emit_flush(rq, EMIT_BARRIER);
drivers/gpu/drm/i915/gvt/mmio_context.c:ret = 
req->engine->emit_flush(req, EMIT_BARRIER);
drivers/gpu/drm/i915/gvt/mmio_context.c:ret = 
req->engine->emit_flush(req, EMIT_BARRIER);

The EMIT_FLUSH in the ringbuffer pd update is also purely arbitrary.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2019-08-28 Thread Mika Kuoppala 
Chris Wilson  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 
>> Cc: Mika Kuoppala 
>> ---
>>  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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2019-08-27 Thread Chris Wilson 
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 
> Cc: Mika Kuoppala 
> ---
>  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).
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2019-08-27 Thread Chris Wilson 
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 
Cc: Mika Kuoppala 
---
 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 |
+ PIPE_CONTROL_CS_STALL);
 
return gen8_emit_fini_breadcrumb_footer(request, cs);
 }
-- 
2.23.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx