[PATCH 25/30] fbdev/core: Move framebuffer and backlight helpers into separate files
Thomas Zimmermann
tzimmermann at suse.de
Fri Jun 9 07:19:17 UTC 2023
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 at suse.de>
> Some nits that you decide what to do with.
> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
>
> 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.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.
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 at infradead.org>
>> */
>>
>> -/*
>> - * 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)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230609/32e318f9/attachment-0001.sig>
More information about the dri-devel
mailing list