Hi Sam, thanks for reviewing. Am 07.06.23 um 21:38 schrieb Sam Ravnborg:
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.
In patch 30/30, sysfs support will be built conditionally. Doing this in the Makefile is so much nicer than having an ifdef conditional in the source files.
I'd also argue that the backlight and framebuffer functions don't belong next to the sysfs code.
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.oThere is "+=" that can be used in Makefile, but people prefer '\' - sigh!fb-$(CONFIG_FB_DEFERRED_IO) += fb_defio.odiff --git a/drivers/video/fbdev/core/fb_backlight.c b/drivers/video/fbdev/core/fb_backlight.cnew 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-laterHmm, can we change the license from 2.0 to 2.0-or-later without any concern? I hope so.
No change here. The backlight function comes from fbsysfs.c, which is also GPL-2.0-or-later.
+ +#include <linux/export.h> +#include <linux/fb.h>#include <linux/mutex.h> - to avoid relying on indirect includes?
I can do that.
+ +#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
Ok. Best regards Thomas
+ +/** + * 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
-- 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)
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
