Re: [PATCH 4/5] drm/msm: Handle fence rollover

[Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx>]

On 11/9/2021 11:41 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@xxxxxxxxxxxx>
>
> Add some helpers for fence comparision, which handle rollover properly,
> and stop open coding fence seqno comparisions.
>
> Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/msm_fence.h | 12 ++++++++++++
>   drivers/gpu/drm/msm/msm_gpu.c   |  6 +++---
>   drivers/gpu/drm/msm/msm_gpu.h   |  2 +-
>   3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_fence.h b/drivers/gpu/drm/msm/msm_fence.h
> index 4783db528bcc..17ee3822b423 100644
> --- a/drivers/gpu/drm/msm/msm_fence.h
> +++ b/drivers/gpu/drm/msm/msm_fence.h
> @@ -60,4 +60,16 @@ void msm_update_fence(struct msm_fence_context *fctx, uint32_t fence);
>   
>   struct dma_fence * msm_fence_alloc(struct msm_fence_context *fctx);
>   
> +static inline bool
> +fence_before(uint32_t a, uint32_t b)
> +{
> +   return (int32_t)(a - b) < 0;

This is good enough when a and b have close values. And that is a good 
assumption for KMD generated seqno.

Reviewed-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx>

-Akhil.

> +}
> +
> +static inline bool
> +fence_after(uint32_t a, uint32_t b)
> +{
> +   return (int32_t)(a - b) > 0;
> +}
> +
>   #endif
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 13de1241d595..0f78c2615272 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -172,7 +172,7 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
>   
>   	spin_lock_irqsave(&ring->submit_lock, flags);
>   	list_for_each_entry(submit, &ring->submits, node) {
> -		if (submit->seqno > fence)
> +		if (fence_after(submit->seqno, fence))
>   			break;
>   
>   		msm_update_fence(submit->ring->fctx,
> @@ -509,7 +509,7 @@ static void hangcheck_handler(struct timer_list *t)
>   	if (fence != ring->hangcheck_fence) {
>   		/* some progress has been made.. ya! */
>   		ring->hangcheck_fence = fence;
> -	} else if (fence < ring->seqno) {
> +	} else if (fence_before(fence, ring->seqno)) {
>   		/* no progress and not done.. hung! */
>   		ring->hangcheck_fence = fence;
>   		DRM_DEV_ERROR(dev->dev, "%s: hangcheck detected gpu lockup rb %d!\n",
> @@ -523,7 +523,7 @@ static void hangcheck_handler(struct timer_list *t)
>   	}
>   
>   	/* if still more pending work, reset the hangcheck timer: */
> -	if (ring->seqno > ring->hangcheck_fence)
> +	if (fence_after(ring->seqno, ring->hangcheck_fence))
>   		hangcheck_timer_reset(gpu);
>   
>   	/* workaround for missing irq: */
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 0dcc31c27ac3..bd4e0024033e 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -258,7 +258,7 @@ static inline bool msm_gpu_active(struct msm_gpu *gpu)
>   	for (i = 0; i < gpu->nr_rings; i++) {
>   		struct msm_ringbuffer *ring = gpu->rb[i];
>   
> -		if (ring->seqno > ring->memptrs->fence)
> +		if (fence_after(ring->seqno, ring->memptrs->fence))
>   			return true;
>   	}
>