Re: [RFT PATCH] drm/msm: Trigger fence completion from GPU

[Rob Clark]

On Wed, Feb 14, 2018 at 1:46 AM, Bjorn Andersson
 wrote:
> Interrupt commands causes the CP to trigger an interrupt as the command
> is processed, regardless of the GPU being done processing previous
> commands. This is seen by the interrupt being delivered before the
> fence is written on 8974 and is likely the cause of the additional
> CP_WAIT_FOR_IDLE workaround found for a306, which would cause the CP to
> wait for the GPU to go idle before triggering the interrupt.
>
> Instead we can set the (undocumented) BIT(31) of the CACHE_FLUSH_TS
> which will cause a special CACHE_FLUSH_TS interrupt to be triggered from
> the GPU as the write event is processed.
>
> Add CACHE_FLUSH_TS to the IRQ masks of A3xx and A4xx and remove the
> workaround for A306.
>
> Suggested-by: Jordan Crouse 
> Signed-off-by: Bjorn Andersson 
> ---
>
> This is only tested on 8974.

Tested on db410c (8016).. thanks

BR,
-R

>
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 18 ++
>  3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> index 4baef2738178..a3a43be920d0 100644
> --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> @@ -35,6 +35,7 @@
>  A3XX_INT0_CP_RB_INT | \
>  A3XX_INT0_CP_REG_PROTECT_FAULT |  \
>  A3XX_INT0_CP_AHB_ERROR_HALT | \
> +A3XX_INT0_CACHE_FLUSH_TS |\
>  A3XX_INT0_UCHE_OOB_ACCESS)
>
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 8199a4b9f2fa..b44cd0d90621 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -27,6 +27,7 @@
>  A4XX_INT0_CP_RB_INT | \
>  A4XX_INT0_CP_REG_PROTECT_FAULT |  \
>  A4XX_INT0_CP_AHB_ERROR_HALT | \
> +A4XX_INT0_CACHE_FLUSH_TS |\
>  A4XX_INT0_UCHE_OOB_ACCESS)
>
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index de63ff26a062..5806f9942514 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -293,26 +293,12 @@ void adreno_submit(struct msm_gpu *gpu, struct 
> msm_gem_submit *submit,
> OUT_RING(ring, 0x);
> }
>
> +   /* BIT(31) of CACHE_FLUSH_TS triggers CACHE_FLUSH_TS IRQ from GPU */
> OUT_PKT3(ring, CP_EVENT_WRITE, 3);
> -   OUT_RING(ring, CACHE_FLUSH_TS);
> +   OUT_RING(ring, CACHE_FLUSH_TS | BIT(31));
> OUT_RING(ring, rbmemptr(ring, fence));
> OUT_RING(ring, submit->seqno);
>
> -   /* we could maybe be clever and only CP_COND_EXEC the interrupt: */
> -   OUT_PKT3(ring, CP_INTERRUPT, 1);
> -   OUT_RING(ring, 0x8000);
> -
> -   /* Workaround for missing irq issue on 8x16/a306.  Unsure if the
> -* root cause is a platform issue or some a306 quirk, but this
> -* keeps things humming along:
> -*/
> -   if (adreno_is_a306(adreno_gpu)) {
> -   OUT_PKT3(ring, CP_WAIT_FOR_IDLE, 1);
> -   OUT_RING(ring, 0x);
> -   OUT_PKT3(ring, CP_INTERRUPT, 1);
> -   OUT_RING(ring, 0x8000);
> -   }
> -
>  #if 0
> if (adreno_is_a3xx(adreno_gpu)) {
> /* Dummy set-constant to trigger context rollover */
> --
> 2.15.0
>

Re: [RFT PATCH] drm/msm: Trigger fence completion from GPU

[Rob Clark]

On Wed, Feb 14, 2018 at 1:46 AM, Bjorn Andersson
 wrote:
> Interrupt commands causes the CP to trigger an interrupt as the command
> is processed, regardless of the GPU being done processing previous
> commands. This is seen by the interrupt being delivered before the
> fence is written on 8974 and is likely the cause of the additional
> CP_WAIT_FOR_IDLE workaround found for a306, which would cause the CP to
> wait for the GPU to go idle before triggering the interrupt.
>
> Instead we can set the (undocumented) BIT(31) of the CACHE_FLUSH_TS
> which will cause a special CACHE_FLUSH_TS interrupt to be triggered from
> the GPU as the write event is processed.
>
> Add CACHE_FLUSH_TS to the IRQ masks of A3xx and A4xx and remove the
> workaround for A306.
>
> Suggested-by: Jordan Crouse 
> Signed-off-by: Bjorn Andersson 
> ---
>
> This is only tested on 8974.

Tested on db410c (8016).. thanks

BR,
-R

>
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 18 ++
>  3 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> index 4baef2738178..a3a43be920d0 100644
> --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> @@ -35,6 +35,7 @@
>  A3XX_INT0_CP_RB_INT | \
>  A3XX_INT0_CP_REG_PROTECT_FAULT |  \
>  A3XX_INT0_CP_AHB_ERROR_HALT | \
> +A3XX_INT0_CACHE_FLUSH_TS |\
>  A3XX_INT0_UCHE_OOB_ACCESS)
>
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 8199a4b9f2fa..b44cd0d90621 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -27,6 +27,7 @@
>  A4XX_INT0_CP_RB_INT | \
>  A4XX_INT0_CP_REG_PROTECT_FAULT |  \
>  A4XX_INT0_CP_AHB_ERROR_HALT | \
> +A4XX_INT0_CACHE_FLUSH_TS |\
>  A4XX_INT0_UCHE_OOB_ACCESS)
>
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index de63ff26a062..5806f9942514 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -293,26 +293,12 @@ void adreno_submit(struct msm_gpu *gpu, struct 
> msm_gem_submit *submit,
> OUT_RING(ring, 0x);
> }
>
> +   /* BIT(31) of CACHE_FLUSH_TS triggers CACHE_FLUSH_TS IRQ from GPU */
> OUT_PKT3(ring, CP_EVENT_WRITE, 3);
> -   OUT_RING(ring, CACHE_FLUSH_TS);
> +   OUT_RING(ring, CACHE_FLUSH_TS | BIT(31));
> OUT_RING(ring, rbmemptr(ring, fence));
> OUT_RING(ring, submit->seqno);
>
> -   /* we could maybe be clever and only CP_COND_EXEC the interrupt: */
> -   OUT_PKT3(ring, CP_INTERRUPT, 1);
> -   OUT_RING(ring, 0x8000);
> -
> -   /* Workaround for missing irq issue on 8x16/a306.  Unsure if the
> -* root cause is a platform issue or some a306 quirk, but this
> -* keeps things humming along:
> -*/
> -   if (adreno_is_a306(adreno_gpu)) {
> -   OUT_PKT3(ring, CP_WAIT_FOR_IDLE, 1);
> -   OUT_RING(ring, 0x);
> -   OUT_PKT3(ring, CP_INTERRUPT, 1);
> -   OUT_RING(ring, 0x8000);
> -   }
> -
>  #if 0
> if (adreno_is_a3xx(adreno_gpu)) {
> /* Dummy set-constant to trigger context rollover */
> --
> 2.15.0
>

Re: [RFT PATCH] drm/msm: Trigger fence completion from GPU

[Jordan Crouse]

On Tue, Feb 13, 2018 at 10:46:58PM -0800, Bjorn Andersson wrote:
> Interrupt commands causes the CP to trigger an interrupt as the command
> is processed, regardless of the GPU being done processing previous
> commands. This is seen by the interrupt being delivered before the
> fence is written on 8974 and is likely the cause of the additional
> CP_WAIT_FOR_IDLE workaround found for a306, which would cause the CP to
> wait for the GPU to go idle before triggering the interrupt.
> 
> Instead we can set the (undocumented) BIT(31) of the CACHE_FLUSH_TS
> which will cause a special CACHE_FLUSH_TS interrupt to be triggered from
> the GPU as the write event is processed.
> 
> Add CACHE_FLUSH_TS to the IRQ masks of A3xx and A4xx and remove the
> workaround for A306.
> 
> Suggested-by: Jordan Crouse 
> Signed-off-by: Bjorn Andersson 

LGTM. This should help your races.

> ---
> 
> This is only tested on 8974.
> 
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 18 ++
>  3 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> index 4baef2738178..a3a43be920d0 100644
> --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> @@ -35,6 +35,7 @@
>A3XX_INT0_CP_RB_INT | \
>A3XX_INT0_CP_REG_PROTECT_FAULT |  \
>A3XX_INT0_CP_AHB_ERROR_HALT | \
> +  A3XX_INT0_CACHE_FLUSH_TS |\
>A3XX_INT0_UCHE_OOB_ACCESS)
>  
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 8199a4b9f2fa..b44cd0d90621 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -27,6 +27,7 @@
>A4XX_INT0_CP_RB_INT | \
>A4XX_INT0_CP_REG_PROTECT_FAULT |  \
>A4XX_INT0_CP_AHB_ERROR_HALT | \
> +  A4XX_INT0_CACHE_FLUSH_TS |\
>A4XX_INT0_UCHE_OOB_ACCESS)
>  
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index de63ff26a062..5806f9942514 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -293,26 +293,12 @@ void adreno_submit(struct msm_gpu *gpu, struct 
> msm_gem_submit *submit,
>   OUT_RING(ring, 0x);
>   }
>  
> + /* BIT(31) of CACHE_FLUSH_TS triggers CACHE_FLUSH_TS IRQ from GPU */
>   OUT_PKT3(ring, CP_EVENT_WRITE, 3);
> - OUT_RING(ring, CACHE_FLUSH_TS);
> + OUT_RING(ring, CACHE_FLUSH_TS | BIT(31));
>   OUT_RING(ring, rbmemptr(ring, fence));
>   OUT_RING(ring, submit->seqno);
>  
> - /* we could maybe be clever and only CP_COND_EXEC the interrupt: */
> - OUT_PKT3(ring, CP_INTERRUPT, 1);
> - OUT_RING(ring, 0x8000);
> -
> - /* Workaround for missing irq issue on 8x16/a306.  Unsure if the
> -  * root cause is a platform issue or some a306 quirk, but this
> -  * keeps things humming along:
> -  */
> - if (adreno_is_a306(adreno_gpu)) {
> - OUT_PKT3(ring, CP_WAIT_FOR_IDLE, 1);
> - OUT_RING(ring, 0x);
> - OUT_PKT3(ring, CP_INTERRUPT, 1);
> - OUT_RING(ring, 0x8000);
> - }
> -
>  #if 0
>   if (adreno_is_a3xx(adreno_gpu)) {
>   /* Dummy set-constant to trigger context rollover */
> -- 
> 2.15.0
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFT PATCH] drm/msm: Trigger fence completion from GPU

[Jordan Crouse]

On Tue, Feb 13, 2018 at 10:46:58PM -0800, Bjorn Andersson wrote:
> Interrupt commands causes the CP to trigger an interrupt as the command
> is processed, regardless of the GPU being done processing previous
> commands. This is seen by the interrupt being delivered before the
> fence is written on 8974 and is likely the cause of the additional
> CP_WAIT_FOR_IDLE workaround found for a306, which would cause the CP to
> wait for the GPU to go idle before triggering the interrupt.
> 
> Instead we can set the (undocumented) BIT(31) of the CACHE_FLUSH_TS
> which will cause a special CACHE_FLUSH_TS interrupt to be triggered from
> the GPU as the write event is processed.
> 
> Add CACHE_FLUSH_TS to the IRQ masks of A3xx and A4xx and remove the
> workaround for A306.
> 
> Suggested-by: Jordan Crouse 
> Signed-off-by: Bjorn Andersson 

LGTM. This should help your races.

> ---
> 
> This is only tested on 8974.
> 
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c   |  1 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 18 ++
>  3 files changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> index 4baef2738178..a3a43be920d0 100644
> --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> @@ -35,6 +35,7 @@
>A3XX_INT0_CP_RB_INT | \
>A3XX_INT0_CP_REG_PROTECT_FAULT |  \
>A3XX_INT0_CP_AHB_ERROR_HALT | \
> +  A3XX_INT0_CACHE_FLUSH_TS |\
>A3XX_INT0_UCHE_OOB_ACCESS)
>  
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index 8199a4b9f2fa..b44cd0d90621 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -27,6 +27,7 @@
>A4XX_INT0_CP_RB_INT | \
>A4XX_INT0_CP_REG_PROTECT_FAULT |  \
>A4XX_INT0_CP_AHB_ERROR_HALT | \
> +  A4XX_INT0_CACHE_FLUSH_TS |\
>A4XX_INT0_UCHE_OOB_ACCESS)
>  
>  extern bool hang_debug;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index de63ff26a062..5806f9942514 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -293,26 +293,12 @@ void adreno_submit(struct msm_gpu *gpu, struct 
> msm_gem_submit *submit,
>   OUT_RING(ring, 0x);
>   }
>  
> + /* BIT(31) of CACHE_FLUSH_TS triggers CACHE_FLUSH_TS IRQ from GPU */
>   OUT_PKT3(ring, CP_EVENT_WRITE, 3);
> - OUT_RING(ring, CACHE_FLUSH_TS);
> + OUT_RING(ring, CACHE_FLUSH_TS | BIT(31));
>   OUT_RING(ring, rbmemptr(ring, fence));
>   OUT_RING(ring, submit->seqno);
>  
> - /* we could maybe be clever and only CP_COND_EXEC the interrupt: */
> - OUT_PKT3(ring, CP_INTERRUPT, 1);
> - OUT_RING(ring, 0x8000);
> -
> - /* Workaround for missing irq issue on 8x16/a306.  Unsure if the
> -  * root cause is a platform issue or some a306 quirk, but this
> -  * keeps things humming along:
> -  */
> - if (adreno_is_a306(adreno_gpu)) {
> - OUT_PKT3(ring, CP_WAIT_FOR_IDLE, 1);
> - OUT_RING(ring, 0x);
> - OUT_PKT3(ring, CP_INTERRUPT, 1);
> - OUT_RING(ring, 0x8000);
> - }
> -
>  #if 0
>   if (adreno_is_a3xx(adreno_gpu)) {
>   /* Dummy set-constant to trigger context rollover */
> -- 
> 2.15.0
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project