Split out all FB related data out of struct picolcd_data into a struct picolcd_fb_data that is allocated with fb_info. This way fb_info may cleanly outlive struct picolcd_data for as long as needed for its last user to drop his reference. Access to struct picolcd_data is now protected with struct picolcd_fb_data's lock and tile update reports are only generated while picolcd_fbdata->picolcd is not NULL and is not marked as failed (which indicates unplug in progress). Signed-off-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx> --- drivers/hid/hid-picolcd.h | 19 +++- drivers/hid/hid-picolcd_fb.c | 216 ++++++++++++++++++++++-------------------- 2 files changed, 126 insertions(+), 109 deletions(-) diff --git a/drivers/hid/hid-picolcd.h b/drivers/hid/hid-picolcd.h index 9200be1..dc4c795 100644 --- a/drivers/hid/hid-picolcd.h +++ b/drivers/hid/hid-picolcd.h @@ -90,11 +90,6 @@ struct picolcd_data { #ifdef CONFIG_HID_PICOLCD_FB /* Framebuffer stuff */ - u8 fb_update_rate; - u8 fb_bpp; - u8 fb_force; - u8 *fb_vbitmap; /* local copy of what was sent to PicoLCD */ - u8 *fb_bitmap; /* framebuffer */ struct fb_info *fb_info; #endif /* CONFIG_HID_PICOLCD_FB */ #ifdef CONFIG_HID_PICOLCD_LCD @@ -119,9 +114,21 @@ struct picolcd_data { int status; #define PICOLCD_BOOTLOADER 1 #define PICOLCD_FAILED 2 -#define PICOLCD_READY_FB 4 }; +#ifdef CONFIG_HID_PICOLCD_FB +struct picolcd_fb_data { + /* Framebuffer stuff */ + spinlock_t lock; + struct picolcd_data *picolcd; + u8 update_rate; + u8 bpp; + u8 force; + u8 ready; + u8 *vbitmap; /* local copy of what was sent to PicoLCD */ + u8 *bitmap; /* framebuffer */ +}; +#endif /* CONFIG_HID_PICOLCD_FB */ /* Find a given report */ #define picolcd_in_report(id, dev) picolcd_report(id, dev, HID_INPUT_REPORT) diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c index 4d8e22c..5905bbd 100644 --- a/drivers/hid/hid-picolcd_fb.c +++ b/drivers/hid/hid-picolcd_fb.c @@ -98,19 +98,26 @@ static const struct fb_var_screeninfo picolcdfb_var = { }; /* Send a given tile to PicoLCD */ -static int picolcd_fb_send_tile(struct hid_device *hdev, int chip, int tile) +static int picolcd_fb_send_tile(struct picolcd_data *data, u8 *vbitmap, + int chip, int tile) { - struct picolcd_data *data = hid_get_drvdata(hdev); - struct hid_report *report1 = picolcd_out_report(REPORT_LCD_CMD_DATA, hdev); - struct hid_report *report2 = picolcd_out_report(REPORT_LCD_DATA, hdev); + struct hid_report *report1, *report2; unsigned long flags; u8 *tdata; int i; - if (!report1 || report1->maxfield != 1 || !report2 || report2->maxfield != 1) + report1 = picolcd_out_report(REPORT_LCD_CMD_DATA, data->hdev); + if (!report1 || report1->maxfield != 1) + return -ENODEV; + report2 = picolcd_out_report(REPORT_LCD_DATA, data->hdev); + if (!report2 || report2->maxfield != 1) return -ENODEV; spin_lock_irqsave(&data->lock, flags); + if ((data->status & PICOLCD_FAILED)) { + spin_unlock_irqrestore(&data->lock, flags); + return -ENODEV; + } hid_set_field(report1->field[0], 0, chip << 2); hid_set_field(report1->field[0], 1, 0x02); hid_set_field(report1->field[0], 2, 0x00); @@ -128,7 +135,7 @@ static int picolcd_fb_send_tile(struct hid_device *hdev, int chip, int tile) hid_set_field(report2->field[0], 2, 0x00); hid_set_field(report2->field[0], 3, 32); - tdata = data->fb_vbitmap + (tile * 4 + chip) * 64; + tdata = vbitmap + (tile * 4 + chip) * 64; for (i = 0; i < 64; i++) if (i < 32) hid_set_field(report1->field[0], 11 + i, tdata[i]); @@ -189,6 +196,7 @@ void picolcd_fb_refresh(struct picolcd_data *data) int picolcd_fb_reset(struct picolcd_data *data, int clear) { struct hid_report *report = picolcd_out_report(REPORT_LCD_CMD, data->hdev); + struct picolcd_fb_data *fbdata = data->fb_info->par; int i, j; unsigned long flags; static const u8 mapcmd[8] = { 0x00, 0x02, 0x00, 0x64, 0x3f, 0x00, 0x64, 0xc0 }; @@ -207,20 +215,19 @@ int picolcd_fb_reset(struct picolcd_data *data, int clear) hid_set_field(report->field[0], j, 0); usbhid_submit_report(data->hdev, report, USB_DIR_OUT); } - - data->status |= PICOLCD_READY_FB; spin_unlock_irqrestore(&data->lock, flags); - if (data->fb_bitmap) { - if (clear) { - memset(data->fb_vbitmap, 0, PICOLCDFB_SIZE); - memset(data->fb_bitmap, 0, PICOLCDFB_SIZE*data->fb_bpp); - } - data->fb_force = 1; + if (clear) { + memset(fbdata->vbitmap, 0, PICOLCDFB_SIZE); + memset(fbdata->bitmap, 0, PICOLCDFB_SIZE*fbdata->bpp); } + fbdata->force = 1; /* schedule first output of framebuffer */ - picolcd_fb_refresh(data); + if (fbdata->ready) + schedule_delayed_work(&data->fb_info->deferred_work, 0); + else + fbdata->ready = 1; return 0; } @@ -230,20 +237,15 @@ static void picolcd_fb_update(struct fb_info *info) { int chip, tile, n; unsigned long flags; + struct picolcd_fb_data *fbdata = info->par; struct picolcd_data *data; mutex_lock(&info->lock); - data = info->par; - if (!data) - goto out; - spin_lock_irqsave(&data->lock, flags); - if (!(data->status & PICOLCD_READY_FB)) { - spin_unlock_irqrestore(&data->lock, flags); - picolcd_fb_reset(data, 0); - } else { - spin_unlock_irqrestore(&data->lock, flags); - } + spin_lock_irqsave(&fbdata->lock, flags); + if (!fbdata->ready && fbdata->picolcd) + picolcd_fb_reset(fbdata->picolcd, 0); + spin_unlock_irqrestore(&fbdata->lock, flags); /* * Translate the framebuffer into the format needed by the PicoLCD. @@ -254,32 +256,38 @@ static void picolcd_fb_update(struct fb_info *info) */ n = 0; for (chip = 0; chip < 4; chip++) - for (tile = 0; tile < 8; tile++) - if (picolcd_fb_update_tile(data->fb_vbitmap, - data->fb_bitmap, data->fb_bpp, chip, tile) || - data->fb_force) { - n += 2; - if (n >= HID_OUTPUT_FIFO_SIZE / 2) { - mutex_unlock(&info->lock); - usbhid_wait_io(data->hdev); - mutex_lock(&info->lock); - data = info->par; - if (!data) - goto out; - spin_lock_irqsave(&data->lock, flags); - if (data->status & PICOLCD_FAILED) { - spin_unlock_irqrestore(&data->lock, flags); - goto out; - } - spin_unlock_irqrestore(&data->lock, flags); - n = 0; - } - picolcd_fb_send_tile(data->hdev, chip, tile); + for (tile = 0; tile < 8; tile++) { + if (!fbdata->force && !picolcd_fb_update_tile( + fbdata->vbitmap, fbdata->bitmap, + fbdata->bpp, chip, tile)) + continue; + n += 2; + if (n >= HID_OUTPUT_FIFO_SIZE / 2) { + spin_lock_irqsave(&fbdata->lock, flags); + data = fbdata->picolcd; + spin_unlock_irqrestore(&fbdata->lock, flags); + mutex_unlock(&info->lock); + if (!data) + return; + usbhid_wait_io(data->hdev); + mutex_lock(&info->lock); + n = 0; } - data->fb_force = false; + spin_lock_irqsave(&fbdata->lock, flags); + data = fbdata->picolcd; + spin_unlock_irqrestore(&fbdata->lock, flags); + if (!data || picolcd_fb_send_tile(data, + fbdata->vbitmap, chip, tile)) + goto out; + } + fbdata->force = false; if (n) { + spin_lock_irqsave(&fbdata->lock, flags); + data = fbdata->picolcd; + spin_unlock_irqrestore(&fbdata->lock, flags); mutex_unlock(&info->lock); - usbhid_wait_io(data->hdev); + if (data) + usbhid_wait_io(data->hdev); return; } out: @@ -336,20 +344,22 @@ static ssize_t picolcd_fb_write(struct fb_info *info, const char __user *buf, static int picolcd_fb_blank(int blank, struct fb_info *info) { - if (!info->par) - return -ENODEV; /* We let fb notification do this for us via lcd/backlight device */ return 0; } static void picolcd_fb_destroy(struct fb_info *info) { + struct picolcd_fb_data *fbdata = info->par; + /* make sure no work is deferred */ fb_deferred_io_cleanup(info); + /* No thridparty should ever unregister our framebuffer! */ + WARN_ON(fbdata->picolcd != NULL); + vfree((u8 *)info->fix.smem_start); framebuffer_release(info); - printk(KERN_DEBUG "picolcd_fb_destroy(%p)\n", info); } static int picolcd_fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) @@ -376,17 +386,15 @@ static int picolcd_fb_check_var(struct fb_var_screeninfo *var, struct fb_info *i static int picolcd_set_par(struct fb_info *info) { - struct picolcd_data *data = info->par; + struct picolcd_fb_data *fbdata = info->par; u8 *tmp_fb, *o_fb; - if (!data) - return -ENODEV; - if (info->var.bits_per_pixel == data->fb_bpp) + if (info->var.bits_per_pixel == fbdata->bpp) return 0; /* switch between 1/8 bit depths */ if (info->var.bits_per_pixel != 1 && info->var.bits_per_pixel != 8) return -EINVAL; - o_fb = data->fb_bitmap; + o_fb = fbdata->bitmap; tmp_fb = kmalloc(PICOLCDFB_SIZE*info->var.bits_per_pixel, GFP_KERNEL); if (!tmp_fb) return -ENOMEM; @@ -415,7 +423,7 @@ static int picolcd_set_par(struct fb_info *info) } kfree(tmp_fb); - data->fb_bpp = info->var.bits_per_pixel; + fbdata->bpp = info->var.bits_per_pixel; return 0; } @@ -453,7 +461,8 @@ static ssize_t picolcd_fb_update_rate_show(struct device *dev, struct device_attribute *attr, char *buf) { struct picolcd_data *data = dev_get_drvdata(dev); - unsigned i, fb_update_rate = data->fb_update_rate; + struct picolcd_fb_data *fbdata = data->fb_info->par; + unsigned i, fb_update_rate = fbdata->update_rate; size_t ret = 0; for (i = 1; i <= PICOLCDFB_UPDATE_RATE_LIMIT; i++) @@ -472,6 +481,7 @@ static ssize_t picolcd_fb_update_rate_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct picolcd_data *data = dev_get_drvdata(dev); + struct picolcd_fb_data *fbdata = data->fb_info->par; int i; unsigned u; @@ -487,8 +497,8 @@ static ssize_t picolcd_fb_update_rate_store(struct device *dev, else if (u == 0) u = PICOLCDFB_UPDATE_RATE_DEFAULT; - data->fb_update_rate = u; - data->fb_info->fbdefio->delay = HZ / data->fb_update_rate; + fbdata->update_rate = u; + data->fb_info->fbdefio->delay = HZ / fbdata->update_rate; return count; } @@ -500,30 +510,18 @@ int picolcd_init_framebuffer(struct picolcd_data *data) { struct device *dev = &data->hdev->dev; struct fb_info *info = NULL; + struct picolcd_fb_data *fbdata = NULL; int i, error = -ENOMEM; - u8 *fb_vbitmap = NULL; - u8 *fb_bitmap = NULL; u32 *palette; - fb_bitmap = vmalloc(PICOLCDFB_SIZE*8); - if (fb_bitmap == NULL) { - dev_err(dev, "can't get a free page for framebuffer\n"); - goto err_nomem; - } - - fb_vbitmap = kmalloc(PICOLCDFB_SIZE, GFP_KERNEL); - if (fb_vbitmap == NULL) { - dev_err(dev, "can't alloc vbitmap image buffer\n"); - goto err_nomem; - } - - data->fb_update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT; /* The extra memory is: * - 256*u32 for pseudo_palette * - struct fb_deferred_io */ info = framebuffer_alloc(256 * sizeof(u32) + - sizeof(struct fb_deferred_io), dev); + sizeof(struct fb_deferred_io) + + sizeof(struct picolcd_fb_data) + + PICOLCDFB_SIZE, dev); if (info == NULL) { dev_err(dev, "failed to allocate a framebuffer\n"); goto err_nomem; @@ -531,74 +529,86 @@ int picolcd_init_framebuffer(struct picolcd_data *data) info->fbdefio = info->par; *info->fbdefio = picolcd_fb_defio; - palette = info->par + sizeof(struct fb_deferred_io); + info->par += sizeof(struct fb_deferred_io); + palette = info->par; + info->par += 256 * sizeof(u32); for (i = 0; i < 256; i++) palette[i] = i > 0 && i < 16 ? 0xff : 0; info->pseudo_palette = palette; - info->screen_base = (char __force __iomem *)fb_bitmap; info->fbops = &picolcdfb_ops; info->var = picolcdfb_var; info->fix = picolcdfb_fix; info->fix.smem_len = PICOLCDFB_SIZE*8; - info->fix.smem_start = (unsigned long)fb_bitmap; - info->par = data; info->flags = FBINFO_FLAG_DEFAULT; - data->fb_vbitmap = fb_vbitmap; - data->fb_bitmap = fb_bitmap; - data->fb_bpp = picolcdfb_var.bits_per_pixel; + fbdata = info->par; + spin_lock_init(&fbdata->lock); + fbdata->picolcd = data; + fbdata->update_rate = PICOLCDFB_UPDATE_RATE_DEFAULT; + fbdata->bpp = picolcdfb_var.bits_per_pixel; + fbdata->force = 1; + fbdata->vbitmap = info->par + sizeof(struct picolcd_fb_data); + fbdata->bitmap = vmalloc(PICOLCDFB_SIZE*8); + if (fbdata->bitmap == NULL) { + dev_err(dev, "can't get a free page for framebuffer\n"); + goto err_nomem; + } + info->screen_base = (char __force __iomem *)fbdata->bitmap; + info->fix.smem_start = (unsigned long)fbdata->bitmap; + memset(fbdata->vbitmap, 0xff, PICOLCDFB_SIZE); + data->fb_info = info; + error = picolcd_fb_reset(data, 1); if (error) { dev_err(dev, "failed to configure display\n"); goto err_cleanup; } + error = device_create_file(dev, &dev_attr_fb_update_rate); if (error) { dev_err(dev, "failed to create sysfs attributes\n"); goto err_cleanup; } + fb_deferred_io_init(info); - data->fb_info = info; error = register_framebuffer(info); if (error) { dev_err(dev, "failed to register framebuffer\n"); goto err_sysfs; } - /* schedule first output of framebuffer */ - data->fb_force = 1; - schedule_delayed_work(&info->deferred_work, 0); return 0; err_sysfs: - fb_deferred_io_cleanup(info); device_remove_file(dev, &dev_attr_fb_update_rate); + fb_deferred_io_cleanup(info); err_cleanup: - data->fb_vbitmap = NULL; - data->fb_bitmap = NULL; - data->fb_bpp = 0; data->fb_info = NULL; err_nomem: + if (fbdata) + vfree(fbdata->bitmap); framebuffer_release(info); - vfree(fb_bitmap); - kfree(fb_vbitmap); return error; } void picolcd_exit_framebuffer(struct picolcd_data *data) { struct fb_info *info = data->fb_info; - u8 *fb_vbitmap = data->fb_vbitmap; - - if (!info) - return; + struct picolcd_fb_data *fbdata = info->par; + unsigned long flags; device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate); - info->par = NULL; + + /* disconnect framebuffer from HID dev */ + spin_lock_irqsave(&fbdata->lock, flags); + fbdata->picolcd = NULL; + spin_unlock_irqrestore(&fbdata->lock, flags); + + /* make sure there is no running update - thus that fbdata->picolcd + * once obtained under lock is guaranteed not to get free() under + * the feet of the deferred work */ + flush_delayed_work_sync(&info->deferred_work); + + data->fb_info = NULL; unregister_framebuffer(info); - data->fb_vbitmap = NULL; - data->fb_bitmap = NULL; - data->fb_bpp = 0; - data->fb_info = NULL; - kfree(fb_vbitmap); } -- 1.7.8.6 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html