All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
To: dri-devel@lists.freedesktop.org
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: [PATCH 33/36] drm/omap: fix race conditon in DMM
Date: Tue, 24 Mar 2015 14:15:50 +0200	[thread overview]
Message-ID: <1427199353-4412-34-git-send-email-tomi.valkeinen@ti.com> (raw)
In-Reply-To: <1427199353-4412-1-git-send-email-tomi.valkeinen@ti.com>

The omapdrm DMM code sometimes crashes with:

WARNING: CPU: 0 PID: 1235 at lib/list_debug.c:36 __list_add+0x8c/0xbc()
list_add double add: new=e9265368, prev=e90139c4, next=e9265368.

This is caused by the code calling release_engine() twice for the same
engine.

dmm_txn_commit(wait=true) call is supposed to wait until the DMM
transaction has been finished. And it does that, but it does not wait
for the irq handler to finish.

What happens is that the irq handler is triggered, and it either wakes
up the thread that called dmm_txn_commit(), or that thread never even
slept because the transaction was finished in the HW very quickly. That
thread then continues executing, even if the irq handler is not yet
finished, and a new transaction may be initiated. If that transaction is
async (i.e. wait=false), a 'async' flag is set to true. The original irq
handler, which has yet not finished, then sees the transaction as
'async', even if it was supposed to be 'sync'.

When that happens, the irq handler does an extra release_engine() call
because it thinks it need to release the engine, leading to the crash.

This patch fixes the issue by using completion to ensure that the irq
handler has finished before a dmm_txn_commit(wait=true) may continue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_dmm_priv.h  |  2 +-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
index d96660573076..9f32a83ca507 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
@@ -148,7 +148,7 @@ struct refill_engine {
 
 	bool async;
 
-	wait_queue_head_t wait_for_refill;
+	struct completion compl;
 
 	struct list_head idle_node;
 };
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index a1a824db1dd6..20db850a1656 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -29,6 +29,7 @@
 #include <linux/mm.h>
 #include <linux/time.h>
 #include <linux/list.h>
+#include <linux/completion.h>
 
 #include "omap_dmm_tiler.h"
 #include "omap_dmm_priv.h"
@@ -146,10 +147,10 @@ static irqreturn_t omap_dmm_irq_handler(int irq, void *arg)
 
 	for (i = 0; i < dmm->num_engines; i++) {
 		if (status & DMM_IRQSTAT_LST) {
-			wake_up_interruptible(&dmm->engines[i].wait_for_refill);
-
 			if (dmm->engines[i].async)
 				release_engine(&dmm->engines[i]);
+
+			complete(&dmm->engines[i].compl);
 		}
 
 		status >>= 8;
@@ -273,7 +274,8 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 
 	/* mark whether it is async to denote list management in IRQ handler */
 	engine->async = wait ? false : true;
-	/* verify that the irq handler sees the 'async' value */
+	reinit_completion(&engine->compl);
+	/* verify that the irq handler sees the 'async' and completion value */
 	smp_mb();
 
 	/* kick reload */
@@ -281,9 +283,8 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait)
 		dmm->base + reg[PAT_DESCR][engine->id]);
 
 	if (wait) {
-		if (wait_event_interruptible_timeout(engine->wait_for_refill,
-				wait_status(engine, DMM_PATSTATUS_READY) == 0,
-				msecs_to_jiffies(1)) <= 0) {
+		if (!wait_for_completion_timeout(&engine->compl,
+				msecs_to_jiffies(1))) {
 			dev_err(dmm->dev, "timed out waiting for done\n");
 			ret = -ETIMEDOUT;
 		}
@@ -719,7 +720,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 						(REFILL_BUFFER_SIZE * i);
 		omap_dmm->engines[i].refill_pa = omap_dmm->refill_pa +
 						(REFILL_BUFFER_SIZE * i);
-		init_waitqueue_head(&omap_dmm->engines[i].wait_for_refill);
+		init_completion(&omap_dmm->engines[i].compl);
 
 		list_add(&omap_dmm->engines[i].idle_node, &omap_dmm->idle_head);
 	}
-- 
2.3.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  parent reply	other threads:[~2015-03-24 12:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-24 12:15 [PATCH 00/36] drm/omap: patches for 4.1 Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 01/36] drm: omapdrm: Fix indentation of structure and array initializers Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 02/36] drm: omapdrm: Refactor CRTC creation code Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 03/36] drm: omapdrm: Remove unused variables Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 04/36] drm: omapdrm: Switch to the universal plane API Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 05/36] drm: omapdrm: Rename omap_plane_dpms() to omap_plane_set_enable() Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 06/36] drm: omapdrm: Reset the zorder property when disabling a plane Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 07/36] drm: omapdrm: Fix race condition between GO and vblank IRQ Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 08/36] drm: omapdrm: Remove manual update display support Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 09/36] drm: omapdrm: Remove omap_crtc->full_update field Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 10/36] drm: omapdrm: Avoid function forward declaration in omap_crtc.c Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 11/36] drm: omapdrm: Prefix all plane functions with omap_plane_ Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 12/36] drm: omapdrm: Pass integer source coordinates to omap_plane_mode_set() Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 13/36] drm: omapdrm: Planes are already disabled when destroyed Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 14/36] drm: omapdrm: Reorder CRTC functions Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 15/36] drm/omap: fix encoder-crtc mapping Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 16/36] drm/omap: page_flip: return -EBUSY if flip pending Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 17/36] drm/omap: clear omap_obj->paddr in omap_gem_put_paddr() Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 18/36] drm/omap: add pin refcounting to omap_framebuffer Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 19/36] drm/omap: add a comment why locking is missing Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 20/36] drm/omap: fix operation without fbdev Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 21/36] drm/omap: fix error handling in omap_framebuffer_create() Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 22/36] drm/omap: handle incompatible buffer stride and pixel size Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 23/36] drm/omap: fix TILER on OMAP5 Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 24/36] drm/omap: fix plane's channel selection Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 25/36] drm/omap: tiler: fix race condition with engine->async Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 26/36] drm/omap: remove dummy PM functions Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 27/36] drm/omap: stop connector polling during suspend Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 28/36] drm/omap: use DRM_ERROR_RATELIMITED() for error irqs Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 29/36] drm/omap: fix race with error_irq Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 30/36] drm/omap: only ignore DIGIT SYNC LOST for TV output Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 31/36] drm/omap: do not use BUG_ON(!spin_is_locked(x)) Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 32/36] drm/omap: fix race condition with dev->obj_list Tomi Valkeinen
2015-03-24 12:15 ` Tomi Valkeinen [this message]
2015-03-24 12:15 ` [PATCH 34/36] drm/omap: keep ref to old_fb Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 35/36] drm/omap: add hibernation callbacks Tomi Valkeinen
2015-03-24 12:15 ` [PATCH 36/36] drm/omap: tiler: add hibernation callback Tomi Valkeinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1427199353-4412-34-git-send-email-tomi.valkeinen@ti.com \
    --to=tomi.valkeinen@ti.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart@ideasonboard.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.