OMAP: DSS/DSSCOMP: Remove race conditions on callback setting and error handling

This fixes lockups and crashes observed during display error
injection tests.

Wrapping prior callback for managers created race condition,
where the manager settings could change between setting
the manager info and wrapping the callback.  Since callbacks
are never set on the original managers, this wrapping functionality
is removed.

The error handling pathway was also prone to race conditions as
the settings could have gotten applied between the mgr->apply failure
and the unsetting of the callback.  Therefore, created a protected
method to unregister a callback (given that it has not been applied).

Finally, callbacks were not called when set_mgr_info failed, but
mgr_apply succeeded.  This is now needed as we are not wrapping the
callback outside of set_mgr_info.

Also fixed two bugs in mgr_blank() where the apply ISR was not
registered, and where we did not wait got GO if GO was not already
present during configure().

Change-Id: Ifb4e07ea3bc17006c394517b8eb216ebfe154b73
Signed-off-by: Iliyan Malchev <malchev@google.com>

5 files changed, 56 insertions, 23 deletions

diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
index 7413dec..5888688 100644
--- a/drivers/video/omap2/dss/manager.c
+++ b/drivers/video/omap2/dss/manager.c
@@ -1628,7 +1628,7 @@ static int omap_dss_mgr_blank(struct omap_overlay_manager *mgr,
 	 * TRICKY: Enable apply irq even if not waiting for vsync, so that
 	 * DISPC programming takes place in case GO bit was on.
 	 */
-	if (dss_cache.irq_enabled) {
+	if (!dss_cache.irq_enabled) {
 		u32 mask;
 
 		mask = DISPC_IRQ_VSYNC	| DISPC_IRQ_EVSYNC_ODD |
@@ -1675,7 +1675,7 @@ static int omap_dss_mgr_blank(struct omap_overlay_manager *mgr,
 
 	spin_unlock_irqrestore(&dss_cache.lock, flags);
 
-	if (wait_for_go && !r)
+	if (wait_for_go)
 		mgr->wait_for_go(mgr);
 
 	if (!r_get)
@@ -1684,6 +1684,22 @@ static int omap_dss_mgr_blank(struct omap_overlay_manager *mgr,
 	return r;
 }
 
+int omap_dss_manager_unregister_callback(struct omap_overlay_manager *mgr,
+					 struct omapdss_ovl_cb *cb)
+{
+	unsigned long flags;
+	int r = 0;
+	spin_lock_irqsave(&dss_cache.lock, flags);
+	if (mgr->info_dirty &&
+	    mgr->info.cb.fn == cb->fn &&
+	    mgr->info.cb.data == cb->data)
+		mgr->info.cb.fn = NULL;
+	else
+		r = -EPERM;
+	spin_unlock_irqrestore(&dss_cache.lock, flags);
+	return r;
+}
+
 static int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
 {
 	struct overlay_cache_data *oc;
diff --git a/drivers/video/omap2/dsscomp/base.c b/drivers/video/omap2/dsscomp/base.c
index d02269b..ad7ade6 100644
--- a/drivers/video/omap2/dsscomp/base.c
+++ b/drivers/video/omap2/dsscomp/base.c
@@ -382,7 +382,7 @@ struct omap_overlay_manager *find_dss_mgr(int display_ix)
 	return NULL;
 }
 
-int set_dss_mgr_info(struct dss2_mgr_info *mi)
+int set_dss_mgr_info(struct dss2_mgr_info *mi, struct omapdss_ovl_cb *cb)
 {
 	struct omap_overlay_manager_info info;
 	struct omap_overlay_manager *mgr;
@@ -404,6 +404,7 @@ int set_dss_mgr_info(struct dss2_mgr_info *mi)
 
 	info.cpr_coefs = mi->cpr_coefs;
 	info.cpr_enable = mi->cpr_enabled;
+	info.cb = *cb;
 
 	return mgr->set_manager_info(mgr, &info);
 }
diff --git a/drivers/video/omap2/dsscomp/dsscomp.h b/drivers/video/omap2/dsscomp/dsscomp.h
index 6e423f6..8edfaa7 100644
--- a/drivers/video/omap2/dsscomp/dsscomp.h
+++ b/drivers/video/omap2/dsscomp/dsscomp.h
@@ -105,7 +105,6 @@ struct dsscomp_data {
 	u32 ovl_mask;		/* overlays used on this frame */
 	u32 ovl_dmask;		/* overlays disabled on this frame */
 	u32 ix;			/* manager index that this frame is on */
-	struct omapdss_ovl_cb cb;
 	struct dsscomp_setup_mgr_data frm;
 	struct dss2_ovl_info ovls[5];
 	void (*extra_cb)(void *data, int status);
@@ -142,7 +141,7 @@ int dsscomp_state_notifier(struct notifier_block *nb,
 
 /* basic operation - if not using queues */
 int set_dss_ovl_info(struct dss2_ovl_info *oi);
-int set_dss_mgr_info(struct dss2_mgr_info *mi);
+int set_dss_mgr_info(struct dss2_mgr_info *mi, struct omapdss_ovl_cb *cb);
 struct omap_overlay_manager *find_dss_mgr(int display_ix);
 void swap_rb_in_ovl_info(struct dss2_ovl_info *oi);
 void swap_rb_in_mgr_info(struct dss2_mgr_info *mi);
diff --git a/drivers/video/omap2/dsscomp/queue.c b/drivers/video/omap2/dsscomp/queue.c
index a079474..ef778bb 100644
--- a/drivers/video/omap2/dsscomp/queue.c
+++ b/drivers/video/omap2/dsscomp/queue.c
@@ -425,9 +425,6 @@ static u32 dsscomp_mgr_callback(void *data, int id, int status)
 {
 	struct dsscomp_data *comp = data;
 
-	/* do any other callbacks */
-	dss_ovl_cb(&comp->cb, id, status);
-
 	if (status == DSS_COMPLETION_PROGRAMMED ||
 	    (status == DSS_COMPLETION_DISPLAYED &&
 	     comp->state != DSSCOMP_STATE_DISPLAYED) ||
@@ -440,7 +437,7 @@ static u32 dsscomp_mgr_callback(void *data, int id, int status)
 	}
 
 	/* get each callback only once */
-	return ~status | (comp->cb.fn ? comp->cb.mask : 0);
+	return ~status;
 }
 
 static inline bool dssdev_manually_updated(struct omap_dss_device *dev)
@@ -461,6 +458,14 @@ static int dsscomp_apply(dsscomp_t comp)
 	struct omap_overlay *ovl;
 	struct dsscomp_setup_mgr_data *d;
 	u32 oix;
+	bool cb_programmed = false;
+
+	struct omapdss_ovl_cb cb = {
+		.fn = dsscomp_mgr_callback,
+		.data = comp,
+		.mask = DSS_COMPLETION_DISPLAYED |
+		DSS_COMPLETION_PROGRAMMED | DSS_COMPLETION_RELEASED,
+	};
 
 	BUG_ON(comp->state != DSSCOMP_STATE_APPLYING);
 
@@ -535,8 +540,10 @@ skip_ovl_set:
 	 * so if it succeeds, we will use the callback to complete the
 	 * composition.  Otherwise, we can skip the composition now.
 	 */
-	if (!r || comp->must_apply)
-		r = set_dss_mgr_info(&d->mgr);
+	if (!r || comp->must_apply) {
+		r = set_dss_mgr_info(&d->mgr, &cb);
+		cb_programmed = r == 0;
+	}
 
 	if (r && !comp->must_apply) {
 		dev_err(DEV(cdev), "[%p] set failed %d\n", comp, r);
@@ -547,14 +554,8 @@ skip_ovl_set:
 		if (r)
 			dev_warn(DEV(cdev), "[%p] ignoring set failure %d\n",
 								comp, r);
-		/* override manager's callback to avoid eclipsed cb */
 		comp->blank = dmask == comp->ovl_mask;
 		comp->ovl_dmask = dmask;
-		comp->cb = mgr->info.cb;
-		mgr->info.cb.fn = dsscomp_mgr_callback;
-		mgr->info.cb.data = comp;
-		mgr->info.cb.mask = DSS_COMPLETION_DISPLAYED |
-			DSS_COMPLETION_PROGRAMMED | DSS_COMPLETION_RELEASED;
 
 		/*
 		 * Check other overlays that may also use this display.
@@ -595,15 +596,28 @@ skip_ovl_set:
 		r = mgr->apply(mgr);
 		if (r)
 			dev_err(DEV(cdev), "failed while applying %d", r);
+		/* keep error if set_mgr_info failed */
+		if (!r && !cb_programmed)
+			r = -EINVAL;
 	}
 	mutex_unlock(&mtx);
 
-	/* ignore this error if callback has already been registered */
-	if (!mgr->info_dirty)
-		r = 0;
-	else if (r)
-		/* otherwise reset original callback */
-		mgr->info.cb = comp->cb;
+	/*
+	 * TRICKY: try to unregister callback to see if callbacks have
+	 * been applied (moved into DSS2 pipeline).  Unregistering also
+	 * avoids having to unnecessarily kick out compositions (which
+	 * would result in screen blinking).  If callbacks failed to apply,
+	 * (e.g. could not set them or apply them) we will need to call
+	 * them ourselves (we note this by returning an error).
+	 */
+	if (cb_programmed && r) {
+		/* clear error if callback already registered */
+		if (omap_dss_manager_unregister_callback(mgr, &cb))
+			r = 0;
+	}
+	/* if failed to apply, kick out prior composition */
+	if (comp->must_apply && r)
+		mgr->blank(mgr, true);
 
 	if (!r && (d->mode & DSSCOMP_SETUP_MODE_DISPLAY)) {
 		/* cannot handle update errors, so ignore them */
diff --git a/include/video/omapdss.h b/include/video/omapdss.h
index f3c314a..ae8d4d4 100644
--- a/include/video/omapdss.h
+++ b/include/video/omapdss.h
@@ -735,6 +735,9 @@ int omap_rfbi_update(struct omap_dss_device *dssdev,
 int omap_rfbi_configure(struct omap_dss_device *dssdev, int pixel_size,
 		int data_lines);
 
+int omap_dss_manager_unregister_callback(struct omap_overlay_manager *mgr,
+					 struct omapdss_ovl_cb *cb);
+
 /* generic callback handling */
 static inline void dss_ovl_cb(struct omapdss_ovl_cb *cb, int id, int status)
 {