[PATCH] fbdev: udl: Make CONFIG_FB_DEVICE optional

[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?

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.
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.

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);
>   }
>