diff mbox series

drm/i915/gt: Flush to global observation point before breadcrumb write

Message ID 20220927171313.6553-1-nirmoy.das@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/gt: Flush to global observation point before breadcrumb write | expand

Commit Message

Nirmoy Das Sept. 27, 2022, 5:13 p.m. UTC 
From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>

Add flag to pipecontrol instruction to ensure in-flight writes are
flushed to global observation point. Also split the pipecontrol
instruction like we have in gen8.

References: https://gitlab.freedesktop.org/drm/intel/-/issues/5886
Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
---
 drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 29 +++++++++++++++++-------
 1 file changed, 21 insertions(+), 8 deletions(-)

Comments

Nirmoy Das Sept. 27, 2022, 5:17 p.m. UTC | #1 
+ Prathap

On 9/27/2022 7:13 PM, Nirmoy Das wrote:
> From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
>
> Add flag to pipecontrol instruction to ensure in-flight writes are
> flushed to global observation point. Also split the pipecontrol
> instruction like we have in gen8.
>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/5886
> Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 29 +++++++++++++++++-------
>   1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index e49fa6fa6aee..31a2fbd8c4a8 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -583,6 +583,8 @@ u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
>   u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>   {
>   	cs = gen8_emit_pipe_control(cs,
> +				    PIPE_CONTROL_CS_STALL |
> +				    PIPE_CONTROL_TLB_INVALIDATE |
>   				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
>   				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>   				    PIPE_CONTROL_DC_FLUSH_ENABLE,
> @@ -600,15 +602,21 @@ u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>   
>   u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>   {
> +	cs = gen8_emit_pipe_control(cs,
> +				    PIPE_CONTROL_CS_STALL |
> +				    PIPE_CONTROL_TLB_INVALIDATE |
> +				    PIPE_CONTROL_TILE_CACHE_FLUSH |
> +				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +				    PIPE_CONTROL_DC_FLUSH_ENABLE,
> +				    0);
> +
> +	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
>   	cs = gen8_emit_ggtt_write_rcs(cs,
>   				      rq->fence.seqno,
>   				      hwsp_offset(rq),
> -				      PIPE_CONTROL_CS_STALL |
> -				      PIPE_CONTROL_TILE_CACHE_FLUSH |
> -				      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> -				      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> -				      PIPE_CONTROL_DC_FLUSH_ENABLE |
> -				      PIPE_CONTROL_FLUSH_ENABLE);
> +				      PIPE_CONTROL_FLUSH_ENABLE |
> +				      PIPE_CONTROL_CS_STALL);
>   
>   	return gen8_emit_fini_breadcrumb_tail(rq, cs);
>   }
> @@ -715,6 +723,7 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>   {
>   	struct drm_i915_private *i915 = rq->engine->i915;
>   	u32 flags = (PIPE_CONTROL_CS_STALL |
> +		     PIPE_CONTROL_TLB_INVALIDATE |
>   		     PIPE_CONTROL_TILE_CACHE_FLUSH |
>   		     PIPE_CONTROL_FLUSH_L3 |
>   		     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> @@ -731,11 +740,15 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>   	else if (rq->engine->class == COMPUTE_CLASS)
>   		flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>   
> +	cs = gen12_emit_pipe_control(cs, PIPE_CONTROL0_HDC_PIPELINE_FLUSH, flags, 0);
> +
> +	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
>   	cs = gen12_emit_ggtt_write_rcs(cs,
>   				       rq->fence.seqno,
>   				       hwsp_offset(rq),
> -				       PIPE_CONTROL0_HDC_PIPELINE_FLUSH,
> -				       flags);
> +				       0,
> +				       PIPE_CONTROL_FLUSH_ENABLE |
> +				       PIPE_CONTROL_CS_STALL);
>   
>   	return gen12_emit_fini_breadcrumb_tail(rq, cs);
>   }
Andi Shyti Sept. 28, 2022, 1:25 p.m. UTC | #2 
Hi Nirmoy and Prathap,

On Tue, Sep 27, 2022 at 07:13:13PM +0200, Nirmoy Das wrote:
> From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> 
> Add flag to pipecontrol instruction to ensure in-flight writes are
> flushed to global observation point. Also split the pipecontrol
> instruction like we have in gen8.
> 
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/5886
> Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thanks,
Andi

> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 29 +++++++++++++++++-------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index e49fa6fa6aee..31a2fbd8c4a8 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -583,6 +583,8 @@ u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
>  u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
>  	cs = gen8_emit_pipe_control(cs,
> +				    PIPE_CONTROL_CS_STALL |
> +				    PIPE_CONTROL_TLB_INVALIDATE |
>  				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
>  				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
>  				    PIPE_CONTROL_DC_FLUSH_ENABLE,
> @@ -600,15 +602,21 @@ u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  
>  u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
> +	cs = gen8_emit_pipe_control(cs,
> +				    PIPE_CONTROL_CS_STALL |
> +				    PIPE_CONTROL_TLB_INVALIDATE |
> +				    PIPE_CONTROL_TILE_CACHE_FLUSH |
> +				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +				    PIPE_CONTROL_DC_FLUSH_ENABLE,
> +				    0);
> +
> +	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
>  	cs = gen8_emit_ggtt_write_rcs(cs,
>  				      rq->fence.seqno,
>  				      hwsp_offset(rq),
> -				      PIPE_CONTROL_CS_STALL |
> -				      PIPE_CONTROL_TILE_CACHE_FLUSH |
> -				      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> -				      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> -				      PIPE_CONTROL_DC_FLUSH_ENABLE |
> -				      PIPE_CONTROL_FLUSH_ENABLE);
> +				      PIPE_CONTROL_FLUSH_ENABLE |
> +				      PIPE_CONTROL_CS_STALL);
>  
>  	return gen8_emit_fini_breadcrumb_tail(rq, cs);
>  }
> @@ -715,6 +723,7 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  {
>  	struct drm_i915_private *i915 = rq->engine->i915;
>  	u32 flags = (PIPE_CONTROL_CS_STALL |
> +		     PIPE_CONTROL_TLB_INVALIDATE |
>  		     PIPE_CONTROL_TILE_CACHE_FLUSH |
>  		     PIPE_CONTROL_FLUSH_L3 |
>  		     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> @@ -731,11 +740,15 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
>  	else if (rq->engine->class == COMPUTE_CLASS)
>  		flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
>  
> +	cs = gen12_emit_pipe_control(cs, PIPE_CONTROL0_HDC_PIPELINE_FLUSH, flags, 0);
> +
> +	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
>  	cs = gen12_emit_ggtt_write_rcs(cs,
>  				       rq->fence.seqno,
>  				       hwsp_offset(rq),
> -				       PIPE_CONTROL0_HDC_PIPELINE_FLUSH,
> -				       flags);
> +				       0,
> +				       PIPE_CONTROL_FLUSH_ENABLE |
> +				       PIPE_CONTROL_CS_STALL);
>  
>  	return gen12_emit_fini_breadcrumb_tail(rq, cs);
>  }
> -- 
> 2.37.3
Andi Shyti Sept. 29, 2022, 10:57 a.m. UTC | #3 
Hi,

>    Series: drm/i915/gt: Flush to global observation point before
>    breadcrumb write
>    URL: [1]https://patchwork.freedesktop.org/series/109133/
>    State: failure
>    Details:
>    [2]https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109133v1/index.ht
>    ml
> 
>      CI Bug Log - changes from CI_DRM_12193_full -> Patchwork_109133v1_full
> 
> Summary
> 
>    FAILURE
> 
>    Serious unknown changes coming with Patchwork_109133v1_full absolutely
>    need to be
>    verified manually.
> 
>    If you think the reported changes have nothing to do with the changes
>    introduced in Patchwork_109133v1_full, please notify your bug team to
>    allow them
>    to document this new failure mode, which will reduce false positives in
>    CI.
> 
> Participating hosts (10 -> 10)
> 
>    No changes in participating hosts
> 
> Possible new issues
> 
>    Here are the unknown changes that may have been introduced in
>    Patchwork_109133v1_full:
> 
>   IGT changes
> 
>     Possible regressions
> 
>      * igt@kms_flip_scaled_crc@flip-32bpp-xtile-to-64bpp-xtile-downscaling
>        @pipe-a-valid-mode:
>           + shard-tglb: [3]PASS -> [4]INCOMPLETE

This looks a false positive to me, not even an unrelated failure.
Can anyone from display have a look and confirm, please?

Andi
Andi Shyti Sept. 29, 2022, 2:10 p.m. UTC | #4 
Hi Nirmoy,

> From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> 
> Add flag to pipecontrol instruction to ensure in-flight writes are
> flushed to global observation point. Also split the pipecontrol
> instruction like we have in gen8.
> 
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/5886
> Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>

pushed in drm-intel-gt-next.

Thanks,
Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
index e49fa6fa6aee..31a2fbd8c4a8 100644
--- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
@@ -583,6 +583,8 @@  u32 *gen8_emit_fini_breadcrumb_xcs(struct i915_request *rq, u32 *cs)
 u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 {
 	cs = gen8_emit_pipe_control(cs,
+				    PIPE_CONTROL_CS_STALL |
+				    PIPE_CONTROL_TLB_INVALIDATE |
 				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
 				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
 				    PIPE_CONTROL_DC_FLUSH_ENABLE,
@@ -600,15 +602,21 @@  u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 
 u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 {
+	cs = gen8_emit_pipe_control(cs,
+				    PIPE_CONTROL_CS_STALL |
+				    PIPE_CONTROL_TLB_INVALIDATE |
+				    PIPE_CONTROL_TILE_CACHE_FLUSH |
+				    PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
+				    PIPE_CONTROL_DEPTH_CACHE_FLUSH |
+				    PIPE_CONTROL_DC_FLUSH_ENABLE,
+				    0);
+
+	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
 	cs = gen8_emit_ggtt_write_rcs(cs,
 				      rq->fence.seqno,
 				      hwsp_offset(rq),
-				      PIPE_CONTROL_CS_STALL |
-				      PIPE_CONTROL_TILE_CACHE_FLUSH |
-				      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
-				      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
-				      PIPE_CONTROL_DC_FLUSH_ENABLE |
-				      PIPE_CONTROL_FLUSH_ENABLE);
+				      PIPE_CONTROL_FLUSH_ENABLE |
+				      PIPE_CONTROL_CS_STALL);
 
 	return gen8_emit_fini_breadcrumb_tail(rq, cs);
 }
@@ -715,6 +723,7 @@  u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 {
 	struct drm_i915_private *i915 = rq->engine->i915;
 	u32 flags = (PIPE_CONTROL_CS_STALL |
+		     PIPE_CONTROL_TLB_INVALIDATE |
 		     PIPE_CONTROL_TILE_CACHE_FLUSH |
 		     PIPE_CONTROL_FLUSH_L3 |
 		     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
@@ -731,11 +740,15 @@  u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs)
 	else if (rq->engine->class == COMPUTE_CLASS)
 		flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS;
 
+	cs = gen12_emit_pipe_control(cs, PIPE_CONTROL0_HDC_PIPELINE_FLUSH, flags, 0);
+
+	/*XXX: Look at gen8_emit_fini_breadcrumb_rcs */
 	cs = gen12_emit_ggtt_write_rcs(cs,
 				       rq->fence.seqno,
 				       hwsp_offset(rq),
-				       PIPE_CONTROL0_HDC_PIPELINE_FLUSH,
-				       flags);
+				       0,
+				       PIPE_CONTROL_FLUSH_ENABLE |
+				       PIPE_CONTROL_CS_STALL);
 
 	return gen12_emit_fini_breadcrumb_tail(rq, cs);
 }