diff mbox series

[v3,3/3] gpu: ipu-v3: pre: don't use fixed timeout when waiting for safe window

Message ID 20240517104549.3648939-3-l.stach@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] gpu: ipu-v3: pre: move state into struct | expand

Commit Message

Lucas Stach May 17, 2024, 10:45 a.m. UTC 
The timeout when waiting for the PRE safe window is rather short, as
normally we would only need to wait a few dozen usecs for the problematic
scanline region to pass and we don't want to spin too long in case
something goes wrong. This however mixes badly with preemption, as we
can easily get scheduled away from the CPU for a longer time than our
timeout, in which case we would hit a spurious timeout and wrongly skip
the PRE update.

Instead of disabling preemption across the wait loop, potentially
impacting the overall system latency, use a wait loop with a fixed
max number of iterations, so time spent away from the CPU is not
accounted against the timeout budget.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
v3: new patch
---
 drivers/gpu/ipu-v3/ipu-pre.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

Philipp Zabel June 24, 2024, 9:57 a.m. UTC | #1 
On Fr, 2024-05-17 at 12:45 +0200, Lucas Stach wrote:
> The timeout when waiting for the PRE safe window is rather short, as
> normally we would only need to wait a few dozen usecs for the problematic
> scanline region to pass and we don't want to spin too long in case
> something goes wrong. This however mixes badly with preemption, as we
> can easily get scheduled away from the CPU for a longer time than our
> timeout, in which case we would hit a spurious timeout and wrongly skip
> the PRE update.
> 
> Instead of disabling preemption across the wait loop, potentially
> impacting the overall system latency, use a wait loop with a fixed
> max number of iterations, so time spent away from the CPU is not
> accounted against the timeout budget.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
diff mbox series

Patch

diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index 2db797b50436..5b174cb8aefc 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -5,6 +5,7 @@ 
 
 #include <drm/drm_fourcc.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/genalloc.h>
 #include <linux/module.h>
@@ -259,10 +260,6 @@  void ipu_pre_configure(struct ipu_pre *pre, unsigned int width,
 
 void ipu_pre_update(struct ipu_pre *pre, uint64_t modifier, unsigned int bufaddr)
 {
-	unsigned long timeout = jiffies + msecs_to_jiffies(5);
-	unsigned short current_yblock;
-	u32 val;
-
 	if (bufaddr == pre->cur.bufaddr &&
 	    modifier == pre->cur.modifier)
 		return;
@@ -273,8 +270,11 @@  void ipu_pre_update(struct ipu_pre *pre, uint64_t modifier, unsigned int bufaddr
 	if (modifier != pre->cur.modifier)
 		ipu_pre_configure_modifier(pre, modifier);
 
-	do {
-		if (time_after(jiffies, timeout)) {
+	for (int i = 0;;i++) {
+		unsigned short current_yblock;
+		u32 val;
+
+		if (i > 500) {
 			dev_warn(pre->dev, "timeout waiting for PRE safe window\n");
 			return;
 		}
@@ -283,8 +283,14 @@  void ipu_pre_update(struct ipu_pre *pre, uint64_t modifier, unsigned int bufaddr
 		current_yblock =
 			(val >> IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_SHIFT) &
 			IPU_PRE_STORE_ENG_STATUS_STORE_BLOCK_Y_MASK;
-	} while (current_yblock == 0 ||
-		 current_yblock >= pre->cur.safe_window_end);
+
+		if (current_yblock != 0 &&
+		    current_yblock < pre->cur.safe_window_end)
+			break;
+
+		udelay(10);
+		cpu_relax();
+	}
 
 	writel(pre->cur.ctrl | IPU_PRE_CTRL_SDW_UPDATE,
 	       pre->regs + IPU_PRE_CTRL);