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>
diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c b/drivers/video/omap2/omapfb/omapfb-ioctl.c
index d30b45d..dccb158 100644
--- a/drivers/video/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c
@@ -85,16 +85,6 @@
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 @@
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 @@
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 @@
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 @@
}
out:
- atomic_dec(&rg->lock_count);
- up_write(&rg->lock);
-
+ mutex_unlock(&fbi->mm_lock);
return r;
}
@@ -272,14 +240,12 @@
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 @@
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 @@
r = -EINVAL;
}
- omapfb_unlock(fbdev);
-
return r;
}
@@ -357,14 +317,10 @@
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 @@
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 @@
r = _omapfb_set_color_key(mgr, ck);
err:
- omapfb_unlock(fbdev);
-
return r;
}
@@ -454,13 +405,10 @@
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 @@
*ck = omapfb_color_keys[mgr->id];
err:
- omapfb_unlock(fbdev);
-
return r;
}
@@ -603,6 +549,8 @@
int r = 0;
+ omapfb_lock(fbdev);
+
switch (cmd) {
case OMAPFB_SYNC_GFX:
DBG("ioctl SYNC_GFX\n");
@@ -908,6 +856,8 @@
r = -EINVAL;
}
+ omapfb_unlock(fbdev);
+
if (r < 0)
DBG("ioctl failed: %d\n", r);