[PATCH v5 05/21] gpu: host1x: Use HW-equivalent syncpoint expiration check

Mikko Perttunen
2021-01-11T13:02:15Z

Make syncpoint expiration checks always use the same logic used by
the hardware. This ensures that there are no race conditions that
could occur because of the hardware triggering a syncpoint interrupt
and then the driver disagreeing.

One situation where this could occur is if a job incremented a
syncpoint too many times -- then the hardware would trigger an
interrupt, but the driver would assume that a syncpoint value
greater than the syncpoint's max value is in the future, and not
clean up the job.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
drivers/gpu/host1x/syncpt.c | 51 ++-----------------------------------
1 file changed, 2 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/host1x/syncpt.c b/drivers/gpu/host1x/syncpt.c
index e48b4595cf53..9ccdf7709946 100644
--- a/drivers/gpu/host1x/syncpt.c
+++ b/drivers/gpu/host1x/syncpt.c
@@ -306,59 +306,12 @@ EXPORT_SYMBOL(host1x_syncpt_wait);
bool host1x_syncpt_is_expired(struct host1x_syncpt *sp, u32 thresh)
{
u32 current_val;
- u32 future_val;

smp_rmb();

current_val = (u32)atomic_read(&sp->min_val);
- future_val = (u32)atomic_read(&sp->max_val);
-
- /* Note the use of unsigned arithmetic here (mod 1<<32).
- *
- * c = current_val = min_val = the current value of the syncpoint.
- * t = thresh = the value we are checking
- * f = future_val = max_val = the value c will reach when all
- * outstanding increments have completed.
- *
- * Note that c always chases f until it reaches f.
- *
- * Dtf = (f - t)
- * Dtc = (c - t)
- *
- * Consider all cases:
- *
- * A) .....c..t..f..... Dtf < Dtc need to wait
- * B) .....c.....f..t.. Dtf > Dtc expired
- * C) ..t..c.....f..... Dtf > Dtc expired (Dct very large)
- *
- * Any case where f==c: always expired (for any t). Dtf == Dcf
- * Any case where t==c: always expired (for any f). Dtf >= Dtc (because Dtc==0)
- * Any case where t==f!=c: always wait. Dtf < Dtc (because Dtf==0,
- * Dtc!=0)
- *
- * Other cases:
- *
- * A) .....t..f..c..... Dtf < Dtc need to wait
- * A) .....f..c..t..... Dtf < Dtc need to wait
- * A) .....f..t..c..... Dtf > Dtc expired
- *
- * So:
- * Dtf >= Dtc implies EXPIRED (return true)
- * Dtf < Dtc implies WAIT (return false)
- *
- * Note: If t is expired then we *cannot* wait on it. We would wait
- * forever (hang the system).
- *
- * Note: do NOT get clever and remove the -thresh from both sides. It
- * is NOT the same.
- *
- * If future valueis zero, we have a client managed sync point. In that
- * case we do a direct comparison.
- */
- if (!host1x_syncpt_client_managed(sp))
- return future_val - thresh >= current_val - thresh;
- else
- return (s32)(current_val - thresh) >= 0;
+
+ return ((current_val - thresh) & 0x80000000U) == 0U;
}

int host1x_syncpt_init(struct host1x *host)
--
2.30.0