From 3ed37d9aba486dece93e05d68f691b80ee100900 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 13 Dec 2012 13:19:05 +0200 Subject: [PATCH] Revert "OMAPFB: simplify locking" This reverts commit b41deecbda70067b26a3a7704fdf967a7940935b. The simpler locking causes huge latencies when two processes use the omapfb, even if they use different framebuffers. Signed-off-by: Tomi Valkeinen --- drivers/video/omap2/omapfb/omapfb-ioctl.c | 68 ++++++++++++++++++++---- drivers/video/omap2/omapfb/omapfb-main.c | 40 ++++++++++---- drivers/video/omap2/omapfb/omapfb-sysfs.c | 87 ++++++++++--------------------- drivers/video/omap2/omapfb/omapfb.h | 16 ++++++ 4 files changed, 132 insertions(+), 79 deletions(-) diff --git a/drivers/video/omap2/omapfb/omapfb-ioctl.c b/drivers/video/omap2/omapfb/omapfb-ioctl.c index dccb158..d30b45d 100644 --- a/drivers/video/omap2/omapfb/omapfb-ioctl.c +++ b/drivers/video/omap2/omapfb/omapfb-ioctl.c @@ -85,6 +85,16 @@ 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 @@ -136,6 +146,16 @@ 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: @@ -146,6 +166,15 @@ 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"); @@ -195,10 +224,11 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi) if (display && display->driver->sync) display->driver->sync(display); - mutex_lock(&fbi->mm_lock); - rg = ofbi->region; + down_write_nested(&rg->lock, rg->id); + atomic_inc(&rg->lock_count); + if (rg->size == size && rg->type == mi->type) goto out; @@ -231,7 +261,9 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi) } out: - mutex_unlock(&fbi->mm_lock); + atomic_dec(&rg->lock_count); + up_write(&rg->lock); + return r; } @@ -240,12 +272,14 @@ 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 = ofbi->region; + rg = omapfb_get_mem_region(ofbi->region); memset(mi, 0, sizeof(*mi)); mi->size = rg->size; mi->type = rg->type; + omapfb_put_mem_region(rg); + return 0; } @@ -284,10 +318,14 @@ 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) + if (d->update_mode == mode) { + omapfb_unlock(fbdev); return 0; + } r = 0; @@ -303,6 +341,8 @@ int omapfb_set_update_mode(struct fb_info *fbi, r = -EINVAL; } + omapfb_unlock(fbdev); + return r; } @@ -317,10 +357,14 @@ 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; } @@ -380,10 +424,13 @@ 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; @@ -398,6 +445,8 @@ static int omapfb_set_color_key(struct fb_info *fbi, r = _omapfb_set_color_key(mgr, ck); err: + omapfb_unlock(fbdev); + return r; } @@ -405,10 +454,13 @@ 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; @@ -423,6 +475,8 @@ static int omapfb_get_color_key(struct fb_info *fbi, *ck = omapfb_color_keys[mgr->id]; err: + omapfb_unlock(fbdev); + return r; } @@ -549,8 +603,6 @@ 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"); @@ -856,8 +908,6 @@ 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); diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c index 948dfb9..ca585ef 100644 --- a/drivers/video/omap2/omapfb/omapfb-main.c +++ b/drivers/video/omap2/omapfb/omapfb-main.c @@ -672,6 +672,8 @@ int check_fb_var(struct fb_info *fbi, struct fb_var_screeninfo *var) DBG("check_fb_var %d\n", ofbi->id); + WARN_ON(!atomic_read(&ofbi->region->lock_count)); + r = fb_mode_to_dss_mode(var, &mode); if (r) { DBG("cannot convert var to omap dss mode\n"); @@ -853,6 +855,8 @@ int omapfb_setup_overlay(struct fb_info *fbi, struct omap_overlay *ovl, int rotation = var->rotate; int i; + WARN_ON(!atomic_read(&ofbi->region->lock_count)); + for (i = 0; i < ofbi->num_overlays; i++) { if (ovl != ofbi->overlays[i]) continue; @@ -944,6 +948,8 @@ int omapfb_apply_changes(struct fb_info *fbi, int init) fill_fb(fbi); #endif + WARN_ON(!atomic_read(&ofbi->region->lock_count)); + for (i = 0; i < ofbi->num_overlays; i++) { ovl = ofbi->overlays[i]; @@ -1002,16 +1008,15 @@ err: static int omapfb_check_var(struct fb_var_screeninfo *var, struct fb_info *fbi) { struct omapfb_info *ofbi = FB2OFB(fbi); - struct omapfb2_device *fbdev = ofbi->fbdev; int r; DBG("check_var(%d)\n", FB2OFB(fbi)->id); - omapfb_lock(fbdev); + omapfb_get_mem_region(ofbi->region); r = check_fb_var(fbi, var); - omapfb_unlock(fbdev); + omapfb_put_mem_region(ofbi->region); return r; } @@ -1020,12 +1025,11 @@ static int omapfb_check_var(struct fb_var_screeninfo *var, struct fb_info *fbi) static int omapfb_set_par(struct fb_info *fbi) { struct omapfb_info *ofbi = FB2OFB(fbi); - struct omapfb2_device *fbdev = ofbi->fbdev; int r; DBG("set_par(%d)\n", FB2OFB(fbi)->id); - omapfb_lock(fbdev); + omapfb_get_mem_region(ofbi->region); set_fb_fix(fbi); @@ -1036,7 +1040,7 @@ static int omapfb_set_par(struct fb_info *fbi) r = omapfb_apply_changes(fbi, 0); out: - omapfb_unlock(fbdev); + omapfb_put_mem_region(ofbi->region); return r; } @@ -1045,7 +1049,6 @@ static int omapfb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi) { struct omapfb_info *ofbi = FB2OFB(fbi); - struct omapfb2_device *fbdev = ofbi->fbdev; struct fb_var_screeninfo new_var; int r; @@ -1061,11 +1064,11 @@ static int omapfb_pan_display(struct fb_var_screeninfo *var, fbi->var = new_var; - omapfb_lock(fbdev); + omapfb_get_mem_region(ofbi->region); r = omapfb_apply_changes(fbi, 0); - omapfb_unlock(fbdev); + omapfb_put_mem_region(ofbi->region); return r; } @@ -1074,14 +1077,18 @@ static void mmap_user_open(struct vm_area_struct *vma) { struct omapfb2_mem_region *rg = vma->vm_private_data; + omapfb_get_mem_region(rg); atomic_inc(&rg->map_count); + omapfb_put_mem_region(rg); } static void mmap_user_close(struct vm_area_struct *vma) { struct omapfb2_mem_region *rg = vma->vm_private_data; + omapfb_get_mem_region(rg); atomic_dec(&rg->map_count); + omapfb_put_mem_region(rg); } static struct vm_operations_struct mmap_user_ops = { @@ -1105,7 +1112,7 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma) return -EINVAL; off = vma->vm_pgoff << PAGE_SHIFT; - rg = ofbi->region; + rg = omapfb_get_mem_region(ofbi->region); start = omapfb_get_region_paddr(ofbi); len = fix->smem_len; @@ -1133,9 +1140,13 @@ static int omapfb_mmap(struct fb_info *fbi, struct vm_area_struct *vma) /* vm_ops.open won't be called for mmap itself. */ atomic_inc(&rg->map_count); + omapfb_put_mem_region(rg); + return 0; error: + omapfb_put_mem_region(ofbi->region); + return r; } @@ -1902,6 +1913,7 @@ static int omapfb_create_framebuffers(struct omapfb2_device *fbdev) ofbi->region = &fbdev->regions[i]; ofbi->region->id = i; + init_rwsem(&ofbi->region->lock); /* assign these early, so that fb alloc can use them */ ofbi->rotation_type = def_vrfb ? OMAP_DSS_ROT_VRFB : @@ -1933,8 +1945,12 @@ static int omapfb_create_framebuffers(struct omapfb2_device *fbdev) /* setup fb_infos */ for (i = 0; i < fbdev->num_fbs; i++) { struct fb_info *fbi = fbdev->fbs[i]; + struct omapfb_info *ofbi = FB2OFB(fbi); + omapfb_get_mem_region(ofbi->region); r = omapfb_fb_init(fbdev, fbi); + omapfb_put_mem_region(ofbi->region); + if (r) { dev_err(fbdev->dev, "failed to setup fb_info\n"); return r; @@ -1966,8 +1982,12 @@ static int omapfb_create_framebuffers(struct omapfb2_device *fbdev) for (i = 0; i < fbdev->num_fbs; i++) { struct fb_info *fbi = fbdev->fbs[i]; + struct omapfb_info *ofbi = FB2OFB(fbi); + omapfb_get_mem_region(ofbi->region); r = omapfb_apply_changes(fbi, 1); + omapfb_put_mem_region(ofbi->region); + if (r) { dev_err(fbdev->dev, "failed to change mode\n"); return r; diff --git a/drivers/video/omap2/omapfb/omapfb-sysfs.c b/drivers/video/omap2/omapfb/omapfb-sysfs.c index be5eb07..18fa9e1 100644 --- a/drivers/video/omap2/omapfb/omapfb-sysfs.c +++ b/drivers/video/omap2/omapfb/omapfb-sysfs.c @@ -49,7 +49,6 @@ static ssize_t store_rotate_type(struct device *dev, { 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; @@ -63,13 +62,12 @@ static ssize_t store_rotate_type(struct device *dev, if (!lock_fb_info(fbi)) return -ENODEV; - omapfb_lock(fbdev); r = 0; if (rot_type == ofbi->rotation_type) goto out; - rg = ofbi->region; + rg = omapfb_get_mem_region(ofbi->region); if (rg->size) { r = -EBUSY; @@ -83,8 +81,8 @@ static ssize_t store_rotate_type(struct device *dev, * 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; @@ -106,7 +104,6 @@ static ssize_t store_mirror(struct device *dev, { 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; @@ -117,10 +114,11 @@ static ssize_t store_mirror(struct device *dev, 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) @@ -135,7 +133,8 @@ static ssize_t store_mirror(struct device *dev, r = count; out: - omapfb_unlock(fbdev); + omapfb_put_mem_region(ofbi->region); + unlock_fb_info(fbi); return r; @@ -274,11 +273,15 @@ static ssize_t store_overlays(struct device *dev, struct device_attribute *attr, 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]; @@ -311,8 +314,12 @@ static ssize_t store_overlays(struct device *dev, struct device_attribute *attr, } if (added) { + omapfb_get_mem_region(ofbi->region); + r = omapfb_apply_changes(fbi, 0); + omapfb_put_mem_region(ofbi->region); + if (r) goto out; } @@ -330,13 +337,11 @@ static ssize_t show_overlays_rotate(struct device *dev, { 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", @@ -345,7 +350,6 @@ static ssize_t show_overlays_rotate(struct device *dev, l += snprintf(buf + l, PAGE_SIZE - l, "\n"); - omapfb_unlock(fbdev); unlock_fb_info(fbi); return l; @@ -356,7 +360,6 @@ static ssize_t store_overlays_rotate(struct device *dev, { 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; @@ -368,7 +371,6 @@ static ssize_t store_overlays_rotate(struct device *dev, if (!lock_fb_info(fbi)) return -ENODEV; - omapfb_lock(fbdev); if (len > 0) { char *p = (char *)buf; @@ -405,7 +407,12 @@ static ssize_t store_overlays_rotate(struct device *dev, 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; @@ -414,7 +421,6 @@ static ssize_t store_overlays_rotate(struct device *dev, r = count; out: - omapfb_unlock(fbdev); unlock_fb_info(fbi); return r; @@ -425,19 +431,8 @@ static ssize_t show_size(struct device *dev, { struct fb_info *fbi = dev_get_drvdata(dev); struct omapfb_info *ofbi = FB2OFB(fbi); - struct omapfb2_device *fbdev = ofbi->fbdev; - int r; - 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; + return snprintf(buf, PAGE_SIZE, "%lu\n", ofbi->region->size); } static ssize_t store_size(struct device *dev, struct device_attribute *attr, @@ -460,15 +455,15 @@ static ssize_t store_size(struct device *dev, struct device_attribute *attr, if (!lock_fb_info(fbi)) return -ENODEV; - omapfb_lock(fbdev); if (display && display->driver->sync) display->driver->sync(display); - mutex_lock(&fbi->mm_lock); - rg = ofbi->region; + down_write_nested(&rg->lock, rg->id); + atomic_inc(&rg->lock_count); + if (atomic_read(&rg->map_count)) { r = -EBUSY; goto out; @@ -501,8 +496,9 @@ static ssize_t store_size(struct device *dev, struct device_attribute *attr, r = count; out: - mutex_unlock(&fbi->mm_lock); - omapfb_unlock(fbdev); + atomic_dec(&rg->lock_count); + up_write(&rg->lock); + unlock_fb_info(fbi); return r; @@ -513,19 +509,8 @@ static ssize_t show_phys(struct device *dev, { struct fb_info *fbi = dev_get_drvdata(dev); struct omapfb_info *ofbi = FB2OFB(fbi); - struct omapfb2_device *fbdev = ofbi->fbdev; - int r; - 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; + return snprintf(buf, PAGE_SIZE, "%0x\n", ofbi->region->paddr); } static ssize_t show_virt(struct device *dev, @@ -541,20 +526,11 @@ static ssize_t show_upd_mode(struct device *dev, 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; @@ -565,8 +541,6 @@ static ssize_t store_upd_mode(struct device *dev, struct device_attribute *attr, 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; @@ -574,17 +548,10 @@ static ssize_t store_upd_mode(struct device *dev, struct device_attribute *attr, 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; } diff --git a/drivers/video/omap2/omapfb/omapfb.h b/drivers/video/omap2/omapfb/omapfb.h index 2b52644..623cd87 100644 --- a/drivers/video/omap2/omapfb/omapfb.h +++ b/drivers/video/omap2/omapfb/omapfb.h @@ -62,6 +62,8 @@ struct omapfb2_mem_region { bool alloc; /* allocated by the driver */ bool map; /* kernel mapped by the driver */ atomic_t map_count; + struct rw_semaphore lock; + atomic_t lock_count; }; /* appended to fb_info */ @@ -189,4 +191,18 @@ static inline int omapfb_overlay_enable(struct omap_overlay *ovl, return ovl->disable(ovl); } +static inline struct omapfb2_mem_region * +omapfb_get_mem_region(struct omapfb2_mem_region *rg) +{ + down_read_nested(&rg->lock, rg->id); + atomic_inc(&rg->lock_count); + return rg; +} + +static inline void omapfb_put_mem_region(struct omapfb2_mem_region *rg) +{ + atomic_dec(&rg->lock_count); + up_read(&rg->lock); +} + #endif -- 2.7.4