Skip to content

Commit

Permalink
Revert "OMAPFB: simplify locking"
Browse files Browse the repository at this point in the history 
This reverts commit b41deec.

The simpler locking causes huge latencies when two processes use the
omapfb, even if they use different framebuffers.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
  • Loading branch information
Tomi Valkeinen committed Dec 13, 2012
1 parent c7e1eae commit 3ed37d9
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 79 deletions.
68 changes: 59 additions & 9 deletions drivers/video/omap2/omapfb/omapfb-ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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");

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -303,6 +341,8 @@ int omapfb_set_update_mode(struct fb_info *fbi,
r = -EINVAL;
}

omapfb_unlock(fbdev);

return r;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -398,17 +445,22 @@ static int omapfb_set_color_key(struct fb_info *fbi,

r = _omapfb_set_color_key(mgr, ck);
err:
omapfb_unlock(fbdev);

return r;
}

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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);

Expand Down
40 changes: 30 additions & 10 deletions drivers/video/omap2/omapfb/omapfb-main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -1002,16 +1008,15 @@ int omapfb_apply_changes(struct fb_info *fbi, int init)
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;
}
Expand All @@ -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);

Expand All @@ -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;
}
Expand All @@ -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;

Expand All @@ -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;
}
Expand All @@ -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 = {
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 :
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 3ed37d9

Please sign in to comment.