Hi Thomas,
On Mon, Jun 05, 2023 at 04:48:07PM +0200, Thomas Zimmermann wrote:
> Move framebuffer and backlight helpers into separate files. Leave
> fbsysfs.c to sysfs-related code. No functional changes.
>
> The framebuffer helpers are not in fbmem.c because they are under
> GPL-2.0-or-later copyright, while fbmem.c is GPL-2.0.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Some nits that you decide what to do with.
Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
I do not get why they are moved out in separate files.
I guess the picture will materialize later.
Sam
> ---
> drivers/video/fbdev/core/Makefile | 4 +-
> drivers/video/fbdev/core/fb_backlight.c | 32 +++++++
> drivers/video/fbdev/core/fb_info.c | 76 ++++++++++++++++
> drivers/video/fbdev/core/fbsysfs.c | 110 +-----------------------
> 4 files changed, 112 insertions(+), 110 deletions(-)
> create mode 100644 drivers/video/fbdev/core/fb_backlight.c
> create mode 100644 drivers/video/fbdev/core/fb_info.c
>
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index 8f0060160ffb..eee3295bc225 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -1,7 +1,9 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_FB_NOTIFY) += fb_notify.o
> obj-$(CONFIG_FB) += fb.o
> -fb-y := fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> +fb-y := fb_backlight.o \
> + fb_info.o \
> + fbmem.o fbmon.o fbcmap.o fbsysfs.o \
> modedb.o fbcvt.o fb_cmdline.o fb_io_fops.o
There is "+=" that can be used in Makefile, but people prefer '\' -
sigh!
> fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.o
>
> diff --git a/drivers/video/fbdev/core/fb_backlight.c b/drivers/video/fbdev/core/fb_backlight.c
> new file mode 100644
> index 000000000000..feffe6c68039
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_backlight.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
Hmm, can we change the license from 2.0 to 2.0-or-later without any
concern? I hope so.
> +
> +#include <linux/export.h>
> +#include <linux/fb.h>
#include <linux/mutex.h> - to avoid relying on indirect includes?
> +
> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> +/*
> + * This function generates a linear backlight curve
> + *
> + * 0: off
> + * 1-7: min
> + * 8-127: linear from min to max
> + */
> +void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
> +{
> + unsigned int i, flat, count, range = (max - min);
> +
> + mutex_lock(&fb_info->bl_curve_mutex);
> +
> + fb_info->bl_curve[0] = off;
> +
> + for (flat = 1; flat < (FB_BACKLIGHT_LEVELS / 16); ++flat)
> + fb_info->bl_curve[flat] = min;
> +
> + count = FB_BACKLIGHT_LEVELS * 15 / 16;
> + for (i = 0; i < count; ++i)
> + fb_info->bl_curve[flat + i] = min + (range * (i + 1) / count);
> +
> + mutex_unlock(&fb_info->bl_curve_mutex);
> +}
> +EXPORT_SYMBOL_GPL(fb_bl_default_curve);
> +#endif
> diff --git a/drivers/video/fbdev/core/fb_info.c b/drivers/video/fbdev/core/fb_info.c
> new file mode 100644
> index 000000000000..fb5b75009ee7
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_info.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/export.h>
> +#include <linux/fb.h>
Same as above, consider including mutex.h
Also slab.h
> +
> +/**
> + * framebuffer_alloc - creates a new frame buffer info structure
> + *
> + * @size: size of driver private data, can be zero
> + * @dev: pointer to the device for this fb, this can be NULL
> + *
> + * Creates a new frame buffer info structure. Also reserves @size bytes
> + * for driver private data (info->par). info->par (if any) will be
> + * aligned to sizeof(long).
> + *
> + * Returns the new structure, or NULL if an error occurred.
> + *
> + */
> +struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
> +{
> +#define BYTES_PER_LONG (BITS_PER_LONG/8)
> +#define PADDING (BYTES_PER_LONG - (sizeof(struct fb_info) % BYTES_PER_LONG))
> + int fb_info_size = sizeof(struct fb_info);
> + struct fb_info *info;
> + char *p;
> +
> + if (size)
> + fb_info_size += PADDING;
> +
> + p = kzalloc(fb_info_size + size, GFP_KERNEL);
> +
> + if (!p)
> + return NULL;
> +
> + info = (struct fb_info *) p;
> +
> + if (size)
> + info->par = p + fb_info_size;
> +
> + info->device = dev;
> + info->fbcon_rotate_hint = -1;
> +
> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> + mutex_init(&info->bl_curve_mutex);
> +#endif
> +
> + return info;
> +#undef PADDING
> +#undef BYTES_PER_LONG
> +}
> +EXPORT_SYMBOL(framebuffer_alloc);
> +
> +/**
> + * framebuffer_release - marks the structure available for freeing
> + *
> + * @info: frame buffer info structure
> + *
> + * Drop the reference count of the device embedded in the
> + * framebuffer info structure.
> + *
> + */
> +void framebuffer_release(struct fb_info *info)
> +{
> + if (!info)
> + return;
> +
> + if (WARN_ON(refcount_read(&info->count)))
> + return;
> +
> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> + mutex_destroy(&info->bl_curve_mutex);
> +#endif
> +
> + kfree(info);
> +}
> +EXPORT_SYMBOL(framebuffer_release);
> diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
> index 0c33c4adcd79..849073f1ca06 100644
> --- a/drivers/video/fbdev/core/fbsysfs.c
> +++ b/drivers/video/fbdev/core/fbsysfs.c
> @@ -5,93 +5,12 @@
> * Copyright (c) 2004 James Simmons <jsimmons@xxxxxxxxxxxxx>
> */
>
> -/*
> - * Note: currently there's only stubs for framebuffer_alloc and
> - * framebuffer_release here. The reson for that is that until all drivers
> - * are converted to use it a sysfsification will open OOPSable races.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> +#include <linux/console.h>
> #include <linux/fb.h>
> #include <linux/fbcon.h>
> -#include <linux/console.h>
> -#include <linux/module.h>
>
> #define FB_SYSFS_FLAG_ATTR 1
>
> -/**
> - * framebuffer_alloc - creates a new frame buffer info structure
> - *
> - * @size: size of driver private data, can be zero
> - * @dev: pointer to the device for this fb, this can be NULL
> - *
> - * Creates a new frame buffer info structure. Also reserves @size bytes
> - * for driver private data (info->par). info->par (if any) will be
> - * aligned to sizeof(long).
> - *
> - * Returns the new structure, or NULL if an error occurred.
> - *
> - */
> -struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
> -{
> -#define BYTES_PER_LONG (BITS_PER_LONG/8)
> -#define PADDING (BYTES_PER_LONG - (sizeof(struct fb_info) % BYTES_PER_LONG))
> - int fb_info_size = sizeof(struct fb_info);
> - struct fb_info *info;
> - char *p;
> -
> - if (size)
> - fb_info_size += PADDING;
> -
> - p = kzalloc(fb_info_size + size, GFP_KERNEL);
> -
> - if (!p)
> - return NULL;
> -
> - info = (struct fb_info *) p;
> -
> - if (size)
> - info->par = p + fb_info_size;
> -
> - info->device = dev;
> - info->fbcon_rotate_hint = -1;
> -
> -#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> - mutex_init(&info->bl_curve_mutex);
> -#endif
> -
> - return info;
> -#undef PADDING
> -#undef BYTES_PER_LONG
> -}
> -EXPORT_SYMBOL(framebuffer_alloc);
> -
> -/**
> - * framebuffer_release - marks the structure available for freeing
> - *
> - * @info: frame buffer info structure
> - *
> - * Drop the reference count of the device embedded in the
> - * framebuffer info structure.
> - *
> - */
> -void framebuffer_release(struct fb_info *info)
> -{
> - if (!info)
> - return;
> -
> - if (WARN_ON(refcount_read(&info->count)))
> - return;
> -
> -#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> - mutex_destroy(&info->bl_curve_mutex);
> -#endif
> -
> - kfree(info);
> -}
> -EXPORT_SYMBOL(framebuffer_release);
> -
> static int activate(struct fb_info *fb_info, struct fb_var_screeninfo *var)
> {
> int err;
> @@ -551,30 +470,3 @@ void fb_cleanup_device(struct fb_info *fb_info)
> fb_info->class_flag &= ~FB_SYSFS_FLAG_ATTR;
> }
> }
> -
> -#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> -/* This function generates a linear backlight curve
> - *
> - * 0: off
> - * 1-7: min
> - * 8-127: linear from min to max
> - */
> -void fb_bl_default_curve(struct fb_info *fb_info, u8 off, u8 min, u8 max)
> -{
> - unsigned int i, flat, count, range = (max - min);
> -
> - mutex_lock(&fb_info->bl_curve_mutex);
> -
> - fb_info->bl_curve[0] = off;
> -
> - for (flat = 1; flat < (FB_BACKLIGHT_LEVELS / 16); ++flat)
> - fb_info->bl_curve[flat] = min;
> -
> - count = FB_BACKLIGHT_LEVELS * 15 / 16;
> - for (i = 0; i < count; ++i)
> - fb_info->bl_curve[flat + i] = min + (range * (i + 1) / count);
> -
> - mutex_unlock(&fb_info->bl_curve_mutex);
> -}
> -EXPORT_SYMBOL_GPL(fb_bl_default_curve);
> -#endif
> --
> 2.40.1
