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-sysfs.c b/drivers/video/omap2/omapfb/omapfb-sysfs.c
index 18fa9e1..be5eb07 100644
--- a/drivers/video/omap2/omapfb/omapfb-sysfs.c
+++ b/drivers/video/omap2/omapfb/omapfb-sysfs.c
@@ -49,6 +49,7 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
struct omapfb2_mem_region *rg;
int rot_type;
int r;
@@ -62,12 +63,13 @@
if (!lock_fb_info(fbi))
return -ENODEV;
+ omapfb_lock(fbdev);
r = 0;
if (rot_type == ofbi->rotation_type)
goto out;
- rg = omapfb_get_mem_region(ofbi->region);
+ rg = ofbi->region;
if (rg->size) {
r = -EBUSY;
@@ -81,8 +83,8 @@
* need to do any further parameter checking at this point.
*/
put_region:
- omapfb_put_mem_region(rg);
out:
+ omapfb_unlock(fbdev);
unlock_fb_info(fbi);
return r ? r : count;
@@ -104,6 +106,7 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
bool mirror;
int r;
struct fb_var_screeninfo new_var;
@@ -114,11 +117,10 @@
if (!lock_fb_info(fbi))
return -ENODEV;
+ omapfb_lock(fbdev);
ofbi->mirror = mirror;
- omapfb_get_mem_region(ofbi->region);
-
memcpy(&new_var, &fbi->var, sizeof(new_var));
r = check_fb_var(fbi, &new_var);
if (r)
@@ -133,8 +135,7 @@
r = count;
out:
- omapfb_put_mem_region(ofbi->region);
-
+ omapfb_unlock(fbdev);
unlock_fb_info(fbi);
return r;
@@ -273,15 +274,11 @@
DBG("detaching %d\n", ofbi->overlays[i]->id);
- omapfb_get_mem_region(ofbi->region);
-
omapfb_overlay_enable(ovl, 0);
if (ovl->manager)
ovl->manager->apply(ovl->manager);
- omapfb_put_mem_region(ofbi->region);
-
for (t = i + 1; t < ofbi->num_overlays; t++) {
ofbi->rotation[t-1] = ofbi->rotation[t];
ofbi->overlays[t-1] = ofbi->overlays[t];
@@ -314,12 +311,8 @@
}
if (added) {
- omapfb_get_mem_region(ofbi->region);
-
r = omapfb_apply_changes(fbi, 0);
- omapfb_put_mem_region(ofbi->region);
-
if (r)
goto out;
}
@@ -337,11 +330,13 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
ssize_t l = 0;
int t;
if (!lock_fb_info(fbi))
return -ENODEV;
+ omapfb_lock(fbdev);
for (t = 0; t < ofbi->num_overlays; t++) {
l += snprintf(buf + l, PAGE_SIZE - l, "%s%d",
@@ -350,6 +345,7 @@
l += snprintf(buf + l, PAGE_SIZE - l, "\n");
+ omapfb_unlock(fbdev);
unlock_fb_info(fbi);
return l;
@@ -360,6 +356,7 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
int num_ovls = 0, r, i;
int len;
bool changed = false;
@@ -371,6 +368,7 @@
if (!lock_fb_info(fbi))
return -ENODEV;
+ omapfb_lock(fbdev);
if (len > 0) {
char *p = (char *)buf;
@@ -407,12 +405,7 @@
for (i = 0; i < num_ovls; ++i)
ofbi->rotation[i] = rotation[i];
- omapfb_get_mem_region(ofbi->region);
-
r = omapfb_apply_changes(fbi, 0);
-
- omapfb_put_mem_region(ofbi->region);
-
if (r)
goto out;
@@ -421,6 +414,7 @@
r = count;
out:
+ omapfb_unlock(fbdev);
unlock_fb_info(fbi);
return r;
@@ -431,8 +425,19 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
+ int r;
- return snprintf(buf, PAGE_SIZE, "%lu\n", ofbi->region->size);
+ if (!lock_fb_info(fbi))
+ return -ENODEV;
+ omapfb_lock(fbdev);
+
+ r = snprintf(buf, PAGE_SIZE, "%lu\n", ofbi->region->size);
+
+ omapfb_unlock(fbdev);
+ unlock_fb_info(fbi);
+
+ return r;
}
static ssize_t store_size(struct device *dev, struct device_attribute *attr,
@@ -455,14 +460,14 @@
if (!lock_fb_info(fbi))
return -ENODEV;
+ omapfb_lock(fbdev);
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 (atomic_read(&rg->map_count)) {
r = -EBUSY;
@@ -496,9 +501,8 @@
r = count;
out:
- atomic_dec(&rg->lock_count);
- up_write(&rg->lock);
-
+ mutex_unlock(&fbi->mm_lock);
+ omapfb_unlock(fbdev);
unlock_fb_info(fbi);
return r;
@@ -509,8 +513,19 @@
{
struct fb_info *fbi = dev_get_drvdata(dev);
struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
+ int r;
- return snprintf(buf, PAGE_SIZE, "%0x\n", ofbi->region->paddr);
+ if (!lock_fb_info(fbi))
+ return -ENODEV;
+ omapfb_lock(fbdev);
+
+ r = snprintf(buf, PAGE_SIZE, "%0x\n", ofbi->region->paddr);
+
+ omapfb_unlock(fbdev);
+ unlock_fb_info(fbi);
+
+ return r;
}
static ssize_t show_virt(struct device *dev,
@@ -526,11 +541,20 @@
struct device_attribute *attr, char *buf)
{
struct fb_info *fbi = dev_get_drvdata(dev);
+ struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
enum omapfb_update_mode mode;
int r;
+ if (!lock_fb_info(fbi))
+ return -ENODEV;
+ omapfb_lock(fbdev);
+
r = omapfb_get_update_mode(fbi, &mode);
+ omapfb_unlock(fbdev);
+ unlock_fb_info(fbi);
+
if (r)
return r;
@@ -541,6 +565,8 @@
const char *buf, size_t count)
{
struct fb_info *fbi = dev_get_drvdata(dev);
+ struct omapfb_info *ofbi = FB2OFB(fbi);
+ struct omapfb2_device *fbdev = ofbi->fbdev;
unsigned mode;
int r;
@@ -548,10 +574,17 @@
if (r)
return r;
+ if (!lock_fb_info(fbi))
+ return -ENODEV;
+ omapfb_lock(fbdev);
+
r = omapfb_set_update_mode(fbi, mode);
if (r)
return r;
+ omapfb_unlock(fbdev);
+ unlock_fb_info(fbi);
+
return count;
}