OMAPFB: simplify locking

Kernel lock verification code has lately detected possible circular
locking in omapfb. The exact problem is unclear, but omapfb's current
locking seems to be overly complex.

This patch simplifies the locking in the following ways:

- Remove explicit omapfb mem region locking. I couldn't figure out the
  need for this, as long as we take care to take omapfb lock.

- Get omapfb lock always, even if the operation is possibly only related
  to one fb_info. Better safe than sorry, and normally there's only one
  user for the fb so this shouldn't matter.

- Make sure fb_info lock is taken first, then omapfb lock.

With this patch the warnings about possible circular locking does not
happen anymore.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

1 files changed, 9 insertions, 59 deletions

diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c b/drivers/video/omap2/omapfb/omapfb-ioctl.c
index d30b45d7264..dccb158b0d0 100644
--- a/drivers/video/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c
@@ -85,16 +85,6 @@ static int omapfb_setup_plane(struct fb_info *fbi, struct omapfb_plane_info *pi)
 		goto out;
 	}
 
-	/* Take the locks in a specific order to keep lockdep happy */
-	if (old_rg->id < new_rg->id) {
-		omapfb_get_mem_region(old_rg);
-		omapfb_get_mem_region(new_rg);
-	} else if (new_rg->id < old_rg->id) {
-		omapfb_get_mem_region(new_rg);
-		omapfb_get_mem_region(old_rg);
-	} else
-		omapfb_get_mem_region(old_rg);
-
 	if (pi->enabled && !new_rg->size) {
 		/*
 		 * This plane's memory was freed, can't enable it
@@ -146,16 +136,6 @@ static int omapfb_setup_plane(struct fb_info *fbi, struct omapfb_plane_info *pi)
 			goto undo;
 	}
 
-	/* Release the locks in a specific order to keep lockdep happy */
-	if (old_rg->id > new_rg->id) {
-		omapfb_put_mem_region(old_rg);
-		omapfb_put_mem_region(new_rg);
-	} else if (new_rg->id > old_rg->id) {
-		omapfb_put_mem_region(new_rg);
-		omapfb_put_mem_region(old_rg);
-	} else
-		omapfb_put_mem_region(old_rg);
-
 	return 0;
 
  undo:
@@ -166,15 +146,6 @@ static int omapfb_setup_plane(struct fb_info *fbi, struct omapfb_plane_info *pi)
 
 	ovl->set_overlay_info(ovl, &old_info);
  put_mem:
-	/* Release the locks in a specific order to keep lockdep happy */
-	if (old_rg->id > new_rg->id) {
-		omapfb_put_mem_region(old_rg);
-		omapfb_put_mem_region(new_rg);
-	} else if (new_rg->id > old_rg->id) {
-		omapfb_put_mem_region(new_rg);
-		omapfb_put_mem_region(old_rg);
-	} else
-		omapfb_put_mem_region(old_rg);
  out:
 	dev_err(fbdev->dev, "setup_plane failed\n");
 
@@ -224,10 +195,9 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 	if (display && display->driver->sync)
 		display->driver->sync(display);
 
-	rg = ofbi->region;
+	mutex_lock(&fbi->mm_lock);
 
-	down_write_nested(&rg->lock, rg->id);
-	atomic_inc(&rg->lock_count);
+	rg = ofbi->region;
 
 	if (rg->size == size && rg->type == mi->type)
 		goto out;
@@ -261,9 +231,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 	}
 
  out:
-	atomic_dec(&rg->lock_count);
-	up_write(&rg->lock);
-
+	mutex_unlock(&fbi->mm_lock);
 	return r;
 }
 
@@ -272,14 +240,12 @@ static int omapfb_query_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 	struct omapfb_info *ofbi = FB2OFB(fbi);
 	struct omapfb2_mem_region *rg;
 
-	rg = omapfb_get_mem_region(ofbi->region);
+	rg = ofbi->region;
 	memset(mi, 0, sizeof(*mi));
 
 	mi->size = rg->size;
 	mi->type = rg->type;
 
-	omapfb_put_mem_region(rg);
-
 	return 0;
 }
 
@@ -318,14 +284,10 @@ int omapfb_set_update_mode(struct fb_info *fbi,
 	if (mode != OMAPFB_AUTO_UPDATE && mode != OMAPFB_MANUAL_UPDATE)
 		return -EINVAL;
 
-	omapfb_lock(fbdev);
-
 	d = get_display_data(fbdev, display);
 
-	if (d->update_mode == mode) {
-		omapfb_unlock(fbdev);
+	if (d->update_mode == mode)
 		return 0;
-	}
 
 	r = 0;
 
@@ -341,8 +303,6 @@ int omapfb_set_update_mode(struct fb_info *fbi,
 			r = -EINVAL;
 	}
 
-	omapfb_unlock(fbdev);
-
 	return r;
 }
 
@@ -357,14 +317,10 @@ int omapfb_get_update_mode(struct fb_info *fbi,
 	if (!display)
 		return -EINVAL;
 
-	omapfb_lock(fbdev);
-
 	d = get_display_data(fbdev, display);
 
 	*mode = d->update_mode;
 
-	omapfb_unlock(fbdev);
-
 	return 0;
 }
 
@@ -424,13 +380,10 @@ static int omapfb_set_color_key(struct fb_info *fbi,
 		struct omapfb_color_key *ck)
 {
 	struct omapfb_info *ofbi = FB2OFB(fbi);
-	struct omapfb2_device *fbdev = ofbi->fbdev;
 	int r;
 	int i;
 	struct omap_overlay_manager *mgr = NULL;
 
-	omapfb_lock(fbdev);
-
 	for (i = 0; i < ofbi->num_overlays; i++) {
 		if (ofbi->overlays[i]->manager) {
 			mgr = ofbi->overlays[i]->manager;
@@ -445,8 +398,6 @@ static int omapfb_set_color_key(struct fb_info *fbi,
 
 	r = _omapfb_set_color_key(mgr, ck);
 err:
-	omapfb_unlock(fbdev);
-
 	return r;
 }
 
@@ -454,13 +405,10 @@ static int omapfb_get_color_key(struct fb_info *fbi,
 		struct omapfb_color_key *ck)
 {
 	struct omapfb_info *ofbi = FB2OFB(fbi);
-	struct omapfb2_device *fbdev = ofbi->fbdev;
 	struct omap_overlay_manager *mgr = NULL;
 	int r = 0;
 	int i;
 
-	omapfb_lock(fbdev);
-
 	for (i = 0; i < ofbi->num_overlays; i++) {
 		if (ofbi->overlays[i]->manager) {
 			mgr = ofbi->overlays[i]->manager;
@@ -475,8 +423,6 @@ static int omapfb_get_color_key(struct fb_info *fbi,
 
 	*ck = omapfb_color_keys[mgr->id];
 err:
-	omapfb_unlock(fbdev);
-
 	return r;
 }
 
@@ -603,6 +549,8 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 
 	int r = 0;
 
+	omapfb_lock(fbdev);
+
 	switch (cmd) {
 	case OMAPFB_SYNC_GFX:
 		DBG("ioctl SYNC_GFX\n");
@@ -908,6 +856,8 @@ int omapfb_ioctl(struct fb_info *fbi, unsigned int cmd, unsigned long arg)
 		r = -EINVAL;
 	}
 
+	omapfb_unlock(fbdev);
+
 	if (r < 0)
 		DBG("ioctl failed: %d\n", r);