[Intel-xe] [PATCH v2 2/2] drm/xe/mmio: Make xe_mmio_wait32() aware of interrupts

[Gustavo Sousa]

Quoting Gustavo Sousa (2023-11-16 12:39:02-03:00)
>With the current implementation, a preemption or other kind of interrupt
>might happen between xe_mmio_read32() and ktime_get_raw(). Such an
>interruption (specially in the case of preemption) might be long enough
>to cause a timeout without giving a chance of a new check on the
>register value on a next iteration, which would have happened otherwise.
>
>This issue causes some sporadic timeouts in some code paths. As an
>example, we were experiencing some rare timeouts when waiting for PLL
>unlock for C10/C20 PHYs (see intel_cx0pll_disable()). After debugging,
>we found out that the PLL unlock was happening within the expected time
>period (20us), which suggested a bug in xe_mmio_wait32().
>
>To fix the issue, ensure that we do a last check out of the loop if
>necessary.
>
>This change was tested with the aforementioned PLL unlocking code path.
>Experiments showed that, before this change, we observed reported
>timeouts in 54 of 5000 runs; and, after this change, no timeouts were
>reported in 5000 runs.

Well, I'm using a crystal ball here :-)

This is under test as I type this (I just checked and it is at run #1053
of 5000), I'll come back later once the test is fully finished.

--
Gustavo Sousa

>
>v2:
>  - Prefer an implementation without a barrier (v1 switched the order of
>    xe_mmio_read32() and ktime_get_raw() calls and added a barrier() in
>    between). (Lucas, Rodrigo)
>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>---
> drivers/gpu/drm/xe/xe_mmio.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>index 12df698f6764..16b3ca97f7b4 100644
>--- a/drivers/gpu/drm/xe/xe_mmio.c
>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>@@ -536,6 +536,12 @@ int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 mask, u32 val, u32 t
>                 wait <<= 1;
>         }
> 
>+        if (ret != 0) {
>+                read = xe_mmio_read32(gt, reg);
>+                if ((read & mask) == val)
>+                        ret = 0;
>+        }
>+
>         if (out_val)
>                 *out_val = read;
> 
>-- 
>2.42.0
>