Skip to content

Commit

Permalink
fbdev: add mutex for fb_mmap locking
Browse files Browse the repository at this point in the history
Add a mutex to avoid a circular locking problem between the mm layer
semaphore and fbdev ioctl mutex through the fb_mmap() call.

Also, add mutex to all places where smem_start and smem_len fields change
so the mutex inside the fb_mmap() is actually used.  Changing of these
fields before calling the framebuffer_register() are not mutexed.

This is 2.6.31 material.  It removes one lockdep (fb_mmap() and
register_framebuffer()) but there is still another one (fb_release() and
register_framebuffer()).  It also cleans up handling of the smem_start and
smem_len fields used by mutexed section of the fb_mmap().

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Krzysztof Helt authored and Linus Torvalds committed Jul 1, 2009
1 parent 70d6027 commit 537a1bf
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 42 deletions.
7 changes: 6 additions & 1 deletion drivers/video/atafb.c
Original file line number Diff line number Diff line change
Expand Up @@ -2414,7 +2414,10 @@ static int atafb_get_fix(struct fb_fix_screeninfo *fix, struct fb_info *info)
if (err)
return err;
memset(fix, 0, sizeof(struct fb_fix_screeninfo));
return fbhw->encode_fix(fix, &par);
mutex_lock(&info->mm_lock);
err = fbhw->encode_fix(fix, &par);
mutex_unlock(&info->mm_lock);
return err;
}

static int atafb_get_var(struct fb_var_screeninfo *var, struct fb_info *info)
Expand Down Expand Up @@ -2743,7 +2746,9 @@ static int atafb_set_par(struct fb_info *info)

/* Decode wanted screen parameters */
fbhw->decode_var(&info->var, par);
mutex_lock(&info->mm_lock);
fbhw->encode_fix(&info->fix, par);
mutex_unlock(&info->mm_lock);

/* Set new videomode */
ata_set_par(par);
Expand Down
2 changes: 2 additions & 0 deletions drivers/video/atmel_lcdfb.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,9 @@ static int atmel_lcdfb_alloc_video_memory(struct atmel_lcdfb_info *sinfo)

smem_len = (var->xres_virtual * var->yres_virtual
* ((var->bits_per_pixel + 7) / 8));
mutex_lock(&info->mm_lock);
info->fix.smem_len = max(smem_len, sinfo->smem_len);
mutex_unlock(&info->mm_lock);

info->screen_base = dma_alloc_writecombine(info->device, info->fix.smem_len,
(dma_addr_t *)&info->fix.smem_start, GFP_KERNEL);
Expand Down
13 changes: 5 additions & 8 deletions drivers/video/fbmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -1310,8 +1310,6 @@ static long fb_compat_ioctl(struct file *file, unsigned int cmd,

static int
fb_mmap(struct file *file, struct vm_area_struct * vma)
__acquires(&info->lock)
__releases(&info->lock)
{
int fbidx = iminor(file->f_path.dentry->d_inode);
struct fb_info *info = registered_fb[fbidx];
Expand All @@ -1325,30 +1323,28 @@ __releases(&info->lock)
off = vma->vm_pgoff << PAGE_SHIFT;
if (!fb)
return -ENODEV;
mutex_lock(&info->mm_lock);
if (fb->fb_mmap) {
int res;
mutex_lock(&info->lock);
res = fb->fb_mmap(info, vma);
mutex_unlock(&info->lock);
mutex_unlock(&info->mm_lock);
return res;
}

mutex_lock(&info->lock);

/* frame buffer memory */
start = info->fix.smem_start;
len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.smem_len);
if (off >= len) {
/* memory mapped io */
off -= len;
if (info->var.accel_flags) {
mutex_unlock(&info->lock);
mutex_unlock(&info->mm_lock);
return -EINVAL;
}
start = info->fix.mmio_start;
len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
}
mutex_unlock(&info->lock);
mutex_unlock(&info->mm_lock);
start &= PAGE_MASK;
if ((vma->vm_end - vma->vm_start + off) > len)
return -EINVAL;
Expand Down Expand Up @@ -1518,6 +1514,7 @@ register_framebuffer(struct fb_info *fb_info)
break;
fb_info->node = i;
mutex_init(&fb_info->lock);
mutex_init(&fb_info->mm_lock);

fb_info->dev = device_create(fb_class, fb_info->device,
MKDEV(FB_MAJOR, i), NULL, "fb%d", i);
Expand Down
14 changes: 9 additions & 5 deletions drivers/video/fsl-diu-fb.c
Original file line number Diff line number Diff line change
Expand Up @@ -750,24 +750,26 @@ static void update_lcdc(struct fb_info *info)
static int map_video_memory(struct fb_info *info)
{
phys_addr_t phys;
u32 smem_len = info->fix.line_length * info->var.yres_virtual;

pr_debug("info->var.xres_virtual = %d\n", info->var.xres_virtual);
pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
pr_debug("MAP_VIDEO_MEMORY: smem_len = %u\n", smem_len);

info->fix.smem_len = info->fix.line_length * info->var.yres_virtual;
pr_debug("MAP_VIDEO_MEMORY: smem_len = %d\n", info->fix.smem_len);
info->screen_base = fsl_diu_alloc(info->fix.smem_len, &phys);
info->screen_base = fsl_diu_alloc(smem_len, &phys);
if (info->screen_base == NULL) {
printk(KERN_ERR "Unable to allocate fb memory\n");
return -ENOMEM;
}
mutex_lock(&info->mm_lock);
info->fix.smem_start = (unsigned long) phys;
info->fix.smem_len = smem_len;
mutex_unlock(&info->mm_lock);
info->screen_size = info->fix.smem_len;

pr_debug("Allocated fb @ paddr=0x%08lx, size=%d.\n",
info->fix.smem_start,
info->fix.smem_len);
info->fix.smem_start, info->fix.smem_len);
pr_debug("screen base %p\n", info->screen_base);

return 0;
Expand All @@ -776,9 +778,11 @@ static int map_video_memory(struct fb_info *info)
static void unmap_video_memory(struct fb_info *info)
{
fsl_diu_free(info->screen_base, info->fix.smem_len);
mutex_lock(&info->mm_lock);
info->screen_base = NULL;
info->fix.smem_start = 0;
info->fix.smem_len = 0;
mutex_unlock(&info->mm_lock);
}

/*
Expand Down
2 changes: 2 additions & 0 deletions drivers/video/i810/i810_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1090,8 +1090,10 @@ static int encode_fix(struct fb_fix_screeninfo *fix, struct fb_info *info)
memset(fix, 0, sizeof(struct fb_fix_screeninfo));

strcpy(fix->id, "I810");
mutex_lock(&info->mm_lock);
fix->smem_start = par->fb.physical;
fix->smem_len = par->fb.size;
mutex_unlock(&info->mm_lock);
fix->type = FB_TYPE_PACKED_PIXELS;
fix->type_aux = 0;
fix->xpanstep = 8;
Expand Down
3 changes: 3 additions & 0 deletions drivers/video/matrox/matroxfb_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -724,8 +724,10 @@ static void matroxfb_update_fix(WPMINFO2)
struct fb_fix_screeninfo *fix = &ACCESS_FBINFO(fbcon).fix;
DBG(__func__)

mutex_lock(&ACCESS_FBINFO(fbcon).mm_lock);
fix->smem_start = ACCESS_FBINFO(video.base) + ACCESS_FBINFO(curr.ydstorg.bytes);
fix->smem_len = ACCESS_FBINFO(video.len_usable) - ACCESS_FBINFO(curr.ydstorg.bytes);
mutex_unlock(&ACCESS_FBINFO(fbcon).mm_lock);
}

static int matroxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
Expand Down Expand Up @@ -2081,6 +2083,7 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
spin_lock_init(&ACCESS_FBINFO(lock.accel));
init_rwsem(&ACCESS_FBINFO(crtc2.lock));
init_rwsem(&ACCESS_FBINFO(altout.lock));
mutex_init(&ACCESS_FBINFO(fbcon).mm_lock);
ACCESS_FBINFO(irq_flags) = 0;
init_waitqueue_head(&ACCESS_FBINFO(crtc1.vsync.wait));
init_waitqueue_head(&ACCESS_FBINFO(crtc2.vsync.wait));
Expand Down
5 changes: 4 additions & 1 deletion drivers/video/matrox/matroxfb_crtc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,16 @@ static int matroxfb_dh_release(struct fb_info* info, int user) {
#undef m2info
}

static void matroxfb_dh_init_fix(struct matroxfb_dh_fb_info *m2info) {
static void matroxfb_dh_init_fix(struct matroxfb_dh_fb_info *m2info)
{
struct fb_fix_screeninfo *fix = &m2info->fbcon.fix;

strcpy(fix->id, "MATROX DH");

mutex_lock(&m2info->fbcon.mm_lock);
fix->smem_start = m2info->video.base;
fix->smem_len = m2info->video.len_usable;
mutex_unlock(&m2info->fbcon.mm_lock);
fix->ypanstep = 1;
fix->ywrapstep = 0;
fix->xpanstep = 8; /* TBD */
Expand Down
17 changes: 11 additions & 6 deletions drivers/video/mx3fb.c
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ static uint32_t bpp_to_pixfmt(int bpp)
}

static int mx3fb_blank(int blank, struct fb_info *fbi);
static int mx3fb_map_video_memory(struct fb_info *fbi);
static int mx3fb_map_video_memory(struct fb_info *fbi, unsigned int mem_len);
static int mx3fb_unmap_video_memory(struct fb_info *fbi);

/**
Expand Down Expand Up @@ -742,8 +742,7 @@ static int mx3fb_set_par(struct fb_info *fbi)
if (fbi->fix.smem_start)
mx3fb_unmap_video_memory(fbi);

fbi->fix.smem_len = mem_len;
if (mx3fb_map_video_memory(fbi) < 0) {
if (mx3fb_map_video_memory(fbi, mem_len) < 0) {
mutex_unlock(&mx3_fbi->mutex);
return -ENOMEM;
}
Expand Down Expand Up @@ -1198,30 +1197,34 @@ static int mx3fb_resume(struct platform_device *pdev)
/**
* mx3fb_map_video_memory() - allocates the DRAM memory for the frame buffer.
* @fbi: framebuffer information pointer
* @mem_len: length of mapped memory
* @return: Error code indicating success or failure
*
* This buffer is remapped into a non-cached, non-buffered, memory region to
* allow palette and pixel writes to occur without flushing the cache. Once this
* area is remapped, all virtual memory access to the video memory should occur
* at the new region.
*/
static int mx3fb_map_video_memory(struct fb_info *fbi)
static int mx3fb_map_video_memory(struct fb_info *fbi, unsigned int mem_len)
{
int retval = 0;
dma_addr_t addr;

fbi->screen_base = dma_alloc_writecombine(fbi->device,
fbi->fix.smem_len,
mem_len,
&addr, GFP_DMA);

if (!fbi->screen_base) {
dev_err(fbi->device, "Cannot allocate %u bytes framebuffer memory\n",
fbi->fix.smem_len);
mem_len);
retval = -EBUSY;
goto err0;
}

mutex_lock(&fbi->mm_lock);
fbi->fix.smem_start = addr;
fbi->fix.smem_len = mem_len;
mutex_unlock(&fbi->mm_lock);

dev_dbg(fbi->device, "allocated fb @ p=0x%08x, v=0x%p, size=%d.\n",
(uint32_t) fbi->fix.smem_start, fbi->screen_base, fbi->fix.smem_len);
Expand Down Expand Up @@ -1251,8 +1254,10 @@ static int mx3fb_unmap_video_memory(struct fb_info *fbi)
fbi->screen_base, fbi->fix.smem_start);

fbi->screen_base = 0;
mutex_lock(&fbi->mm_lock);
fbi->fix.smem_start = 0;
fbi->fix.smem_len = 0;
mutex_unlock(&fbi->mm_lock);
return 0;
}

Expand Down
4 changes: 4 additions & 0 deletions drivers/video/omap/omapfb_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,10 @@ static void set_fb_fix(struct fb_info *fbi)

rg = &plane->fbdev->mem_desc.region[plane->idx];
fbi->screen_base = rg->vaddr;
mutex_lock(&fbi->mm_lock);
fix->smem_start = rg->paddr;
fix->smem_len = rg->size;
mutex_unlock(&fbi->mm_lock);

fix->type = FB_TYPE_PACKED_PIXELS;
bpp = var->bits_per_pixel;
Expand Down Expand Up @@ -886,8 +888,10 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
* plane memory is dealloce'd, the other
* screen parameters in var / fix are invalid.
*/
mutex_lock(&fbi->mm_lock);
fbi->fix.smem_start = 0;
fbi->fix.smem_len = 0;
mutex_unlock(&fbi->mm_lock);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions drivers/video/platinumfb.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ static int platinumfb_set_par (struct fb_info *info)
offset = 0x10;

info->screen_base = pinfo->frame_buffer + init->fb_offset + offset;
mutex_lock(&info->mm_lock);
info->fix.smem_start = (pinfo->frame_buffer_phys) + init->fb_offset + offset;
mutex_unlock(&info->mm_lock);
info->fix.visual = (pinfo->cmode == CMODE_8) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_DIRECTCOLOR;
info->fix.line_length = vmode_attrs[pinfo->vmode-1].hres * (1<<pinfo->cmode)
Expand Down
2 changes: 2 additions & 0 deletions drivers/video/pxafb.c
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,10 @@ static int overlayfb_map_video_memory(struct pxafb_layer *ofb)
ofb->video_mem_phys = virt_to_phys(ofb->video_mem);
ofb->video_mem_size = size;

mutex_lock(&ofb->fb.mm_lock);
ofb->fb.fix.smem_start = ofb->video_mem_phys;
ofb->fb.fix.smem_len = ofb->fb.fix.line_length * var->yres_virtual;
mutex_unlock(&ofb->fb.mm_lock);
ofb->fb.screen_base = ofb->video_mem;
return 0;
}
Expand Down
19 changes: 6 additions & 13 deletions drivers/video/sh7760fb.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,6 @@ static int sh7760_setcolreg (u_int regno,
return 0;
}

static void encode_fix(struct fb_fix_screeninfo *fix, struct fb_info *info,
unsigned long stride)
{
memset(fix, 0, sizeof(struct fb_fix_screeninfo));
strcpy(fix->id, "sh7760-lcdc");

fix->smem_start = (unsigned long)info->screen_base;
fix->smem_len = info->screen_size;

fix->line_length = stride;
}

static int sh7760fb_get_color_info(struct device *dev,
u16 lddfr, int *bpp, int *gray)
{
Expand Down Expand Up @@ -334,7 +322,8 @@ static int sh7760fb_set_par(struct fb_info *info)

iowrite32(ldsarl, par->base + LDSARL); /* mem for lower half of DSTN */

encode_fix(&info->fix, info, stride);
info->fix.line_length = stride;

sh7760fb_check_var(&info->var, info);

sh7760fb_blank(FB_BLANK_UNBLANK, info); /* panel on! */
Expand Down Expand Up @@ -435,6 +424,8 @@ static int sh7760fb_alloc_mem(struct fb_info *info)

info->screen_base = fbmem;
info->screen_size = vram;
info->fix.smem_start = (unsigned long)info->screen_base;
info->fix.smem_len = info->screen_size;

return 0;
}
Expand Down Expand Up @@ -520,6 +511,8 @@ static int __devinit sh7760fb_probe(struct platform_device *pdev)
info->var.transp.length = 0;
info->var.transp.msb_right = 0;

strcpy(info->fix.id, "sh7760-lcdc");

/* set the DON2 bit now, before cmap allocation, as it will randomize
* palette memory.
*/
Expand Down
2 changes: 2 additions & 0 deletions drivers/video/sis/sis_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1847,8 +1847,10 @@ sisfb_get_fix(struct fb_fix_screeninfo *fix, int con, struct fb_info *info)

strcpy(fix->id, ivideo->myid);

mutex_lock(&info->mm_lock);
fix->smem_start = ivideo->video_base + ivideo->video_offset;
fix->smem_len = ivideo->sisfb_mem;
mutex_unlock(&info->mm_lock);
fix->type = FB_TYPE_PACKED_PIXELS;
fix->type_aux = 0;
fix->visual = (ivideo->video_bpp == 8) ? FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;
Expand Down
Loading

0 comments on commit 537a1bf

Please sign in to comment.