From 92a9c19a89af2ca219fbb040a0059f414a4b7223 Mon Sep 17 00:00:00 2001 From: Kay Sievers Date: Sat, 28 Jan 2012 19:57:46 +0000 Subject: udlfb: remove sysfs framebuffer device with USB .disconnect() The USB graphics card driver delays the unregistering of the framebuffer device to a workqueue, which breaks the userspace visible remove uevent sequence. Recent userspace tools started to support USB graphics card hotplug out-of-the-box and rely on proper events sent by the kernel. The framebuffer device is a direct child of the USB interface which is removed immediately after the USB .disconnect() callback. But the fb device in /sys stays around until its final cleanup, at a time where all the parent devices have been removed already. To work around that, we remove the sysfs fb device directly in the USB .disconnect() callback and leave only the cleanup of the internal fb data to the delayed work. Before: add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb0 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) remove /2-1.2:1.0/graphics/fb0 (graphics) After: add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) add /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb1 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/graphics/fb1 (graphics) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0 (usb) remove /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2 (usb) Cc: stable@vger.kernel.org Tested-by: Bernie Thompson Acked-by: Bernie Thompson Signed-off-by: Kay Sievers Signed-off-by: Florian Tobias Schandinat --- drivers/video/udlfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index a19773149bd7..a40c05ebbdc2 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -1739,7 +1739,7 @@ static void dlfb_usb_disconnect(struct usb_interface *interface) for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) device_remove_file(info->dev, &fb_device_attrs[i]); device_remove_bin_file(info->dev, &edid_attr); - + unlink_framebuffer(info); usb_set_intfdata(interface, NULL); /* if clients still have us open, will be freed on last close */ -- cgit v1.2.1 From 9daee73c81d21f9f07f236f106da5d93c40f7a92 Mon Sep 17 00:00:00 2001 From: Martin Decky Date: Thu, 1 Mar 2012 16:31:11 -0800 Subject: udlfb: Improve debugging printouts with refresh rate It is not very helpful to print a list of the same resolutions without the refresh rate. Signed-off-by: Bernie Thompson --- drivers/video/udlfb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index a40c05ebbdc2..04aea205f021 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -1012,7 +1012,8 @@ static int dlfb_is_valid_mode(struct fb_videomode *mode, return 0; } - pr_info("%dx%d valid mode\n", mode->xres, mode->yres); + pr_info("%dx%d @ %d Hz valid mode\n", mode->xres, mode->yres, + mode->refresh); return 1; } -- cgit v1.2.1 From 8d21547d3c9c3bc653261f26d554cfabc4a083de Mon Sep 17 00:00:00 2001 From: Bernie Thompson Date: Thu, 1 Mar 2012 17:35:48 -0800 Subject: udlfb: fix hcd_buffer_free panic on unplug/replug Fix race conditions with unplug/replug behavior, in particular take care not to hold up USB probe/disconnect for long-running framebuffer operations and rely on usb to handle teardown. Fix for kernel panic reported with new F17 multiseat support. Reported-by: Kay Sievers Signed-off-by: Bernie Thompson --- drivers/video/udlfb.c | 146 +++++++++++++++++++++++++++----------------------- 1 file changed, 80 insertions(+), 66 deletions(-) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index 04aea205f021..cbf030d0dfc4 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -918,10 +918,6 @@ static void dlfb_free(struct kref *kref) { struct dlfb_data *dev = container_of(kref, struct dlfb_data, kref); - /* this function will wait for all in-flight urbs to complete */ - if (dev->urbs.count > 0) - dlfb_free_urb_list(dev); - if (dev->backing_buffer) vfree(dev->backing_buffer); @@ -940,35 +936,42 @@ static void dlfb_release_urb_work(struct work_struct *work) up(&unode->dev->urbs.limit_sem); } -static void dlfb_free_framebuffer_work(struct work_struct *work) +static void dlfb_free_framebuffer(struct dlfb_data *dev) { - struct dlfb_data *dev = container_of(work, struct dlfb_data, - free_framebuffer_work.work); struct fb_info *info = dev->info; - int node = info->node; - unregister_framebuffer(info); + if (info) { + int node = info->node; - if (info->cmap.len != 0) - fb_dealloc_cmap(&info->cmap); - if (info->monspecs.modedb) - fb_destroy_modedb(info->monspecs.modedb); - if (info->screen_base) - vfree(info->screen_base); + unregister_framebuffer(info); - fb_destroy_modelist(&info->modelist); + if (info->cmap.len != 0) + fb_dealloc_cmap(&info->cmap); + if (info->monspecs.modedb) + fb_destroy_modedb(info->monspecs.modedb); + if (info->screen_base) + vfree(info->screen_base); + + fb_destroy_modelist(&info->modelist); - dev->info = 0; + dev->info = NULL; - /* Assume info structure is freed after this point */ - framebuffer_release(info); + /* Assume info structure is freed after this point */ + framebuffer_release(info); - pr_warn("fb_info for /dev/fb%d has been freed\n", node); + pr_warn("fb_info for /dev/fb%d has been freed\n", node); + } /* ref taken in probe() as part of registering framebfufer */ kref_put(&dev->kref, dlfb_free); } +static void dlfb_free_framebuffer_work(struct work_struct *work) +{ + struct dlfb_data *dev = container_of(work, struct dlfb_data, + free_framebuffer_work.work); + dlfb_free_framebuffer(dev); +} /* * Assumes caller is holding info->lock mutex (for open and release at least) */ @@ -1571,14 +1574,15 @@ success: kfree(buf); return true; } + +static void dlfb_init_framebuffer_work(struct work_struct *work); + static int dlfb_usb_probe(struct usb_interface *interface, const struct usb_device_id *id) { struct usb_device *usbdev; struct dlfb_data *dev = 0; - struct fb_info *info = 0; int retval = -ENOMEM; - int i; /* usb initialization */ @@ -1590,9 +1594,7 @@ static int dlfb_usb_probe(struct usb_interface *interface, goto error; } - /* we need to wait for both usb and fbdev to spin down on disconnect */ kref_init(&dev->kref); /* matching kref_put in usb .disconnect fn */ - kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */ dev->udev = usbdev; dev->gdev = &usbdev->dev; /* our generic struct device * */ @@ -1620,10 +1622,39 @@ static int dlfb_usb_probe(struct usb_interface *interface, goto error; } + kref_get(&dev->kref); /* matching kref_put in free_framebuffer_work */ + /* We don't register a new USB class. Our client interface is fbdev */ + /* Workitem keep things fast & simple during USB enumeration */ + INIT_DELAYED_WORK(&dev->init_framebuffer_work, + dlfb_init_framebuffer_work); + schedule_delayed_work(&dev->init_framebuffer_work, 0); + + return 0; + +error: + if (dev) { + + kref_put(&dev->kref, dlfb_free); /* ref for framebuffer */ + kref_put(&dev->kref, dlfb_free); /* last ref from kref_init */ + + /* dev has been deallocated. Do not dereference */ + } + + return retval; +} + +static void dlfb_init_framebuffer_work(struct work_struct *work) +{ + struct dlfb_data *dev = container_of(work, struct dlfb_data, + init_framebuffer_work.work); + struct fb_info *info; + int retval; + int i; + /* allocates framebuffer driver structure, not framebuffer memory */ - info = framebuffer_alloc(0, &interface->dev); + info = framebuffer_alloc(0, dev->gdev); if (!info) { retval = -ENOMEM; pr_err("framebuffer_alloc failed\n"); @@ -1669,15 +1700,13 @@ static int dlfb_usb_probe(struct usb_interface *interface, for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) { retval = device_create_file(info->dev, &fb_device_attrs[i]); if (retval) { - pr_err("device_create_file failed %d\n", retval); - goto err_del_attrs; + pr_warn("device_create_file failed %d\n", retval); } } retval = device_create_bin_file(info->dev, &edid_attr); if (retval) { - pr_err("device_create_bin_file failed %d\n", retval); - goto err_del_attrs; + pr_warn("device_create_bin_file failed %d\n", retval); } pr_info("DisplayLink USB device /dev/fb%d attached. %dx%d resolution." @@ -1685,38 +1714,10 @@ static int dlfb_usb_probe(struct usb_interface *interface, info->var.xres, info->var.yres, ((dev->backing_buffer) ? info->fix.smem_len * 2 : info->fix.smem_len) >> 10); - return 0; - -err_del_attrs: - for (i -= 1; i >= 0; i--) - device_remove_file(info->dev, &fb_device_attrs[i]); + return; error: - if (dev) { - - if (info) { - if (info->cmap.len != 0) - fb_dealloc_cmap(&info->cmap); - if (info->monspecs.modedb) - fb_destroy_modedb(info->monspecs.modedb); - if (info->screen_base) - vfree(info->screen_base); - - fb_destroy_modelist(&info->modelist); - - framebuffer_release(info); - } - - if (dev->backing_buffer) - vfree(dev->backing_buffer); - - kref_put(&dev->kref, dlfb_free); /* ref for framebuffer */ - kref_put(&dev->kref, dlfb_free); /* last ref from kref_init */ - - /* dev has been deallocated. Do not dereference */ - } - - return retval; + dlfb_free_framebuffer(dev); } static void dlfb_usb_disconnect(struct usb_interface *interface) @@ -1736,12 +1737,24 @@ static void dlfb_usb_disconnect(struct usb_interface *interface) /* When non-active we'll update virtual framebuffer, but no new urbs */ atomic_set(&dev->usb_active, 0); - /* remove udlfb's sysfs interfaces */ - for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) - device_remove_file(info->dev, &fb_device_attrs[i]); - device_remove_bin_file(info->dev, &edid_attr); - unlink_framebuffer(info); + /* this function will wait for all in-flight urbs to complete */ + dlfb_free_urb_list(dev); + + if (info) { + + /* remove udlfb's sysfs interfaces */ + for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) + device_remove_file(info->dev, &fb_device_attrs[i]); + device_remove_bin_file(info->dev, &edid_attr); + + /* it's safe to uncomment next line if your kernel + doesn't yet have this function exported */ + unlink_framebuffer(info); + } + usb_set_intfdata(interface, NULL); + dev->udev = NULL; + dev->gdev = NULL; /* if clients still have us open, will be freed on last close */ if (dev->fb_count == 0) @@ -1807,12 +1820,12 @@ static void dlfb_free_urb_list(struct dlfb_data *dev) int ret; unsigned long flags; - pr_notice("Waiting for completes and freeing all render urbs\n"); + pr_notice("Freeing all render urbs\n"); /* keep waiting and freeing, until we've got 'em all */ while (count--) { - /* Getting interrupted means a leak, but ok at shutdown*/ + /* Getting interrupted means a leak, but ok at disconnect */ ret = down_interruptible(&dev->urbs.limit_sem); if (ret) break; @@ -1834,6 +1847,7 @@ static void dlfb_free_urb_list(struct dlfb_data *dev) kfree(node); } + dev->urbs.count = 0; } static int dlfb_alloc_urb_list(struct dlfb_data *dev, int count, size_t size) -- cgit v1.2.1 From b49f184b640dcfab7ede394cf2a1ff4fe3d154f5 Mon Sep 17 00:00:00 2001 From: Ben Collins Date: Sat, 3 Mar 2012 12:43:27 -0800 Subject: udlfb: Make sure to get correct endian keys from vendor descriptor The driver was not using le16_to_cpu when reading keys from the vendor descriptor, causing incorrect parsing. Mainly, sku_pixel_limit was not being parsed on big-endian systems. This would result in a blank screen on big-endian CPUs where the DL chips's max mode was smaller than the monitor's native mode. Signed-off-by: Ben Collins Signed-off-by: Bernie Thompson --- drivers/video/udlfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index cbf030d0dfc4..4330cf4b17a3 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -1541,7 +1541,7 @@ static int dlfb_parse_vendor_descriptor(struct dlfb_data *dev, u8 length; u16 key; - key = *((u16 *) desc); + key = le16_to_cpu(*((u16 *) desc)); desc += sizeof(u16); length = *desc; desc++; -- cgit v1.2.1 From 664c5f18490f2552900c3f1794602204a43acc86 Mon Sep 17 00:00:00 2001 From: Ben Collins Date: Sat, 3 Mar 2012 12:57:37 -0800 Subject: udlfb: Add module_param to allow forcing pixel_limit Some user scenarios need to prioritize performance over maxiumum resolution. Also, some devices may have bad vendor descriptors, and this allows the user to set a pixel limit that matches their specific device to avoid blank screens on higher resolution monitors. 700000 minimum for DL-115, 2360000 maximum for DL-195 Signed-off-by: Ben Collins Signed-off-by: Bernie Thompson --- drivers/video/udlfb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index 4330cf4b17a3..86c8b256e306 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -72,6 +72,7 @@ MODULE_DEVICE_TABLE(usb, id_table); static bool console = 1; /* Allow fbcon to open framebuffer */ static bool fb_defio = 1; /* Detect mmap writes using page faults */ static bool shadow = 1; /* Optionally disable shadow framebuffer */ +static int pixel_limit; /* Optionally force a pixel resolution limit */ /* dlfb keeps a list of urbs for efficient bulk transfers */ static void dlfb_urb_completion(struct urb *urb); @@ -1616,6 +1617,14 @@ static int dlfb_usb_probe(struct usb_interface *interface, goto error; } + if (pixel_limit) { + pr_warn("DL chip limit of %d overriden" + " by module param to %d\n", + dev->sku_pixel_limit, pixel_limit); + dev->sku_pixel_limit = pixel_limit; + } + + if (!dlfb_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) { retval = -ENOMEM; pr_err("dlfb_alloc_urb_list failed\n"); @@ -1963,6 +1972,9 @@ MODULE_PARM_DESC(fb_defio, "Page fault detection of mmap writes"); module_param(shadow, bool, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP); MODULE_PARM_DESC(shadow, "Shadow vid mem. Disable to save mem but lose perf"); +module_param(pixel_limit, int, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP); +MODULE_PARM_DESC(pixel_limit, "Force limit on max mode (in x*y pixels)"); + MODULE_AUTHOR("Roberto De Ioris , " "Jaya Kumar , " "Bernie Thompson "); -- cgit v1.2.1 From e71ff6f265c80b6f04f1d16470b5afa58f0b4648 Mon Sep 17 00:00:00 2001 From: Olivier Sobrie Date: Wed, 29 Feb 2012 08:06:40 +0100 Subject: udlfb: Fix invalid return codes in edid sysfs entry store function Return a negative errno instead of zero in the write function of the sysfs entry in case of error. Also add a check on the return value of dlfb_setup_modes(). Signed-off-by: Olivier Sobrie Acked-by: Bernie Thompson Signed-off-by: Florian Tobias Schandinat --- drivers/video/udlfb.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'drivers/video/udlfb.c') diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c index 86c8b256e306..157df78e5bfc 100644 --- a/drivers/video/udlfb.c +++ b/drivers/video/udlfb.c @@ -1432,19 +1432,22 @@ static ssize_t edid_store( struct device *fbdev = container_of(kobj, struct device, kobj); struct fb_info *fb_info = dev_get_drvdata(fbdev); struct dlfb_data *dev = fb_info->par; + int ret; /* We only support write of entire EDID at once, no offset*/ if ((src_size != EDID_LENGTH) || (src_off != 0)) - return 0; + return -EINVAL; - dlfb_setup_modes(dev, fb_info, src, src_size); + ret = dlfb_setup_modes(dev, fb_info, src, src_size); + if (ret) + return ret; - if (dev->edid && (memcmp(src, dev->edid, src_size) == 0)) { - pr_info("sysfs written EDID is new default\n"); - dlfb_ops_set_par(fb_info); - return src_size; - } else - return 0; + if (!dev->edid || memcmp(src, dev->edid, src_size)) + return -EINVAL; + + pr_info("sysfs written EDID is new default\n"); + dlfb_ops_set_par(fb_info); + return src_size; } static ssize_t metrics_reset_store(struct device *fbdev, -- cgit v1.2.1