[PATCH] fbdev: udl: Make CONFIG_FB_DEVICE optional
Thomas Zimmermann
tzimmermann at suse.de
Mon Oct 28 08:41:41 UTC 2024
Hi Helge
Am 25.10.24 um 17:37 schrieb Helge Deller:
> On 10/25/24 11:25, Gonzalo Silvalde Blanco wrote:
>> The fb_udl driver currently depends on CONFIG_FB_DEVICE to create sysfs
>> entries and access framebuffer device information. This patch wraps the
>> relevant code blocks with #ifdef CONFIG_FB_DEVICE, allowing the
>> driver to
>> be built and used even if CONFIG_FB_DEVICE is not selected.
>>
>> The sysfs setting only controls access to certain framebuffer attributes
>> and is not required for the basic display functionality to work
>> correctly.
>> (For information on DisplayLink devices and their Linux support, see:
>> https://wiki.archlinux.org/title/DisplayLink).
>>
>> Tested by building with and without CONFIG_FB_DEVICE, both of which
>> compiled and ran without issues.
>
> Gonzalo, I don't like this patch very much.
>
> It adds lots of #ifdefs around functions like dev_dbg().
> Instead of ifdefs, aren't there other possibilities, e.g.
> using fb_dbg() if appropriate?
> Or using any other generic dbg() info or simply dropping the line?
I talked Gonzalo into sending this patch. I think dev_dbg() calls should
be replaced with fb_dbg(), same for _info() and _err(). That's probably
worth doing anyway.
>
> But more important:
> This is a fbdev driver and currently depends on CONFIG_FB_DEVICE.
> If I'm right, the only reason to disable CONFIG_FB_DEVICE is if
> you want fbdev output at bootup, but otherwise just want to use DRM.
It's unrelated to booting. CONFIG_FB_DEVICE enables/disables userspace
interfaces (/dev/fb*, /sys/graphics/fb*). Even without, there's still
fbcon that runs on top of the fbdev driver.
> But then, doesn't there exist a native DRM driver for this graphics
> card which can be used instead?
> If so, I suggest to not change this fbdev driver at all.
Or can we talk about removing udlfb entirely? I tried before, but there
was one person still using it. [1] He had concerns about udl's (the DRM
driver) stability. I think DRM's udl has matured enough and is in better
shape than udlfb. Maybe we can try again.
[1]
https://lore.kernel.org/dri-devel/20201130125200.10416-1-tzimmermann@suse.de/
Best regards
Thomas
>
> Helge
>
>> Signed-off-by: Gonzalo Silvalde Blanco <gonzalo.silvalde at gmail.com>> ---
>> drivers/video/fbdev/Kconfig | 1 -
>> drivers/video/fbdev/udlfb.c | 41 ++++++++++++++++++++++---------------
>> 2 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>> index ea36c6956bf3..9bf6cf74b9cb 100644
>> --- a/drivers/video/fbdev/Kconfig
>> +++ b/drivers/video/fbdev/Kconfig
>> @@ -1588,7 +1588,6 @@ config FB_SMSCUFX
>> config FB_UDL
>> tristate "Displaylink USB Framebuffer support"
>> depends on FB && USB
>> - depends on FB_DEVICE
>> select FB_MODE_HELPERS
>> select FB_SYSMEM_HELPERS_DEFERRED
>> help
>> diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
>> index 71ac9e36f67c..de4800f09dc7 100644
>> --- a/drivers/video/fbdev/udlfb.c
>> +++ b/drivers/video/fbdev/udlfb.c
>> @@ -341,10 +341,10 @@ static int dlfb_ops_mmap(struct fb_info *info,
>> struct vm_area_struct *vma)
>> return -EINVAL;
>>
>> pos = (unsigned long)info->fix.smem_start + offset;
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> dev_dbg(info->dev, "mmap() framebuffer addr:%lu size:%lu\n",
>> pos, size);
>> -
>> +#endif
>> while (size > 0) {
>> page = vmalloc_to_pfn((void *)pos);
>> if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED))
>> @@ -929,10 +929,10 @@ static int dlfb_ops_open(struct fb_info *info,
>> int user)
>> info->fbdefio = fbdefio;
>> fb_deferred_io_init(info);
>> }
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> dev_dbg(info->dev, "open, user=%d fb_info=%p count=%d\n",
>> user, info, dlfb->fb_count);
>> -
>> +#endif
>> return 0;
>> }
>>
>> @@ -982,9 +982,9 @@ static int dlfb_ops_release(struct fb_info *info,
>> int user)
>> kfree(info->fbdefio);
>> info->fbdefio = NULL;
>> }
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> dev_dbg(info->dev, "release, user=%d count=%d\n", user,
>> dlfb->fb_count);
>> -
>> +#endif
>> return 0;
>> }
>>
>> @@ -1095,10 +1095,10 @@ static int dlfb_ops_blank(int blank_mode,
>> struct fb_info *info)
>> struct dlfb_data *dlfb = info->par;
>> char *bufptr;
>> struct urb *urb;
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> dev_dbg(info->dev, "blank, mode %d --> %d\n",
>> dlfb->blank_mode, blank_mode);
>> -
>> +#endif
>> if ((dlfb->blank_mode == FB_BLANK_POWERDOWN) &&
>> (blank_mode != FB_BLANK_POWERDOWN)) {
>>
>> @@ -1190,7 +1190,9 @@ static int dlfb_realloc_framebuffer(struct
>> dlfb_data *dlfb, struct fb_info *info
>> */
>> new_fb = vmalloc(new_len);
>> if (!new_fb) {
>> +#ifdef CONFIG_FB_DEVICE
>> dev_err(info->dev, "Virtual framebuffer alloc failed\n");
>> +#endif
>> return -ENOMEM;
>> }
>> memset(new_fb, 0xff, new_len);
>> @@ -1213,9 +1215,11 @@ static int dlfb_realloc_framebuffer(struct
>> dlfb_data *dlfb, struct fb_info *info
>> */
>> if (shadow)
>> new_back = vzalloc(new_len);
>> +#ifdef CONFIG_FB_DEVICE
>> if (!new_back)
>> dev_info(info->dev,
>> "No shadow/backing buffer allocated\n");
>> +#endif
>> else {
>> dlfb_deferred_vfree(dlfb, dlfb->backing_buffer);
>> dlfb->backing_buffer = new_back;
>> @@ -1247,14 +1251,14 @@ static int dlfb_setup_modes(struct dlfb_data
>> *dlfb,
>> struct device *dev = info->device;
>> struct fb_videomode *mode;
>> const struct fb_videomode *default_vmode = NULL;
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> if (info->dev) {
>> /* only use mutex if info has been registered */
>> mutex_lock(&info->lock);
>> /* parent device is used otherwise */
>> dev = info->dev;
>> }
>> -
>> +#endif
>> edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
>> if (!edid) {
>> result = -ENOMEM;
>> @@ -1375,10 +1379,10 @@ static int dlfb_setup_modes(struct dlfb_data
>> *dlfb,
>> error:
>> if (edid && (dlfb->edid != edid))
>> kfree(edid);
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> if (info->dev)
>> mutex_unlock(&info->lock);
>> -
>> +#endif
>> return result;
>> }
>>
>> @@ -1597,8 +1601,10 @@ static int dlfb_parse_vendor_descriptor(struct
>> dlfb_data *dlfb,
>> static int dlfb_usb_probe(struct usb_interface *intf,
>> const struct usb_device_id *id)
>> {
>> +#ifdef CONFIG_FB_DEVICE
>> int i;
>> const struct device_attribute *attr;
>> +#endif
>> struct dlfb_data *dlfb;
>> struct fb_info *info;
>> int retval;
>> @@ -1701,7 +1707,7 @@ static int dlfb_usb_probe(struct usb_interface
>> *intf,
>> retval);
>> goto error;
>> }
>> -
>> +#ifdef CONFIG_FB_DEVICE
>> for (i = 0; i < ARRAY_SIZE(fb_device_attrs); i++) {
>> attr = &fb_device_attrs[i];
>> retval = device_create_file(info->dev, attr);
>> @@ -1710,17 +1716,16 @@ static int dlfb_usb_probe(struct
>> usb_interface *intf,
>> "failed to create '%s' attribute: %d\n",
>> attr->attr.name, retval);
>> }
>> -
>> retval = device_create_bin_file(info->dev, &edid_attr);
>> if (retval)
>> dev_warn(info->device, "failed to create '%s' attribute:
>> %d\n",
>> edid_attr.attr.name, retval);
>> -
>> dev_info(info->device,
>> "%s is DisplayLink USB device (%dx%d, %dK framebuffer
>> memory)\n",
>> dev_name(info->dev), info->var.xres, info->var.yres,
>> ((dlfb->backing_buffer) ?
>> info->fix.smem_len * 2 : info->fix.smem_len) >> 10);
>> +#endif
>> return 0;
>>
>> error:
>> @@ -1737,8 +1742,9 @@ static void dlfb_usb_disconnect(struct
>> usb_interface *intf)
>> {
>> struct dlfb_data *dlfb;
>> struct fb_info *info;
>> +#ifdef CONFIG_FB_DEVICE
>> int i;
>> -
>> +#endif
>> dlfb = usb_get_intfdata(intf);
>> info = dlfb->info;
>>
>> @@ -1754,10 +1760,11 @@ static void dlfb_usb_disconnect(struct
>> usb_interface *intf)
>> dlfb_free_urb_list(dlfb);
>>
>> /* remove udlfb's sysfs interfaces */
>> +#ifdef CONFIG_FB_DEVICE
>> 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);
>> -
>> +#endif
>> unregister_framebuffer(info);
>> }
>>
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list