[3/9] drm/omap: fix race issue with vsync irq and apply

Commit Message

Tomi Valkeinen Sept. 24, 2014, 1:11 p.m. UTC

omap_crtc's apply_worker does:

	omap_irq_register(dev, &omap_crtc->apply_irq);
	dispc_mgr_go(channel);

This is supposed to enable the vsync irq, and set the GO bit. The vsync
handler will later check if the HW has cleared the GO bit and queue
apply work.

However, what may happen is that the vsync irq is enabled, and it gets
ran before the GO bit is set by the apply_worker. In this case the vsync
handler will see the GO bit as cleared, and queues the apply work too
early.

This causes WARN'ings from dispc driver, and possibly other problems.

This patch fixes the issue by adding a new variable, 'go_bit_set' which
is used to track the supposed state of GO bit and helps the apply worker
and irq handler handle the job without a race.

Signed-off-by: Tomi Valkeinen <[e-mail]>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 0812b0f80167..193979f97bdb 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -47,6 +47,9 @@  struct omap_crtc {
 	bool enabled;
 	bool full_update;
 
+	/* tracks the state of GO bit between irq handler and apply worker */
+	bool go_bit_set;
+
 	struct omap_drm_apply apply;
 
 	struct omap_drm_irq apply_irq;
@@ -442,11 +445,16 @@  static void omap_crtc_apply_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	if (!omap_crtc->error_irq.registered)
 		__omap_irq_register(crtc->dev, &omap_crtc->error_irq);
 
-	if (!dispc_mgr_go_busy(omap_crtc->channel)) {
+	/* make sure we see the most recent 'go_bit_set' */
+	rmb();
+	if (omap_crtc->go_bit_set && !dispc_mgr_go_busy(omap_crtc->channel)) {
 		struct omap_drm_private *priv =
 				crtc->dev->dev_private;
 		DBG("%s: apply done", omap_crtc->name);
 		__omap_irq_unregister(crtc->dev, &omap_crtc->apply_irq);
+		omap_crtc->go_bit_set = false;
+		/* make sure apple_worker sees 'go_bit_set = false' */
+		wmb();
 		queue_work(priv->wq, &omap_crtc->apply_work);
 	}
 }
@@ -472,7 +480,9 @@  static void apply_worker(struct work_struct *work)
 	 * If we are still pending a previous update, wait.. when the
 	 * pending update completes, we get kicked again.
 	 */
-	if (omap_crtc->apply_irq.registered)
+	/* make sure we see the most recent 'go_bit_set' */
+	rmb();
+	if (omap_crtc->go_bit_set)
 		goto out;
 
 	/* finish up previous apply's: */
@@ -502,6 +512,9 @@  static void apply_worker(struct work_struct *work)
 		if (dispc_mgr_is_enabled(channel)) {
 			omap_irq_register(dev, &omap_crtc->apply_irq);
 			dispc_mgr_go(channel);
+			omap_crtc->go_bit_set = true;
+			/* make sure the irq handler sees 'go_bit_set' */
+			wmb();
 		} else {
 			struct omap_drm_private *priv = dev->dev_private;
 			queue_work(priv->wq, &omap_crtc->apply_work);