fbdev: make FB_BACKLIGHT a tristate

PATCH In review Rob Clark 2 participants 6 messages Oct 10, 2018 – Oct 11, 2018

Subsystems (2)

Framebuffer layer Framebuffer core

lore

Share & tools

lore

Rob Clark

author

2018-10-10 15:17 UTC #0 lore raw

PATCH 1/1 fbdev: make FB_BACKLIGHT a tristate

BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.

Signed-off-by Rob Clark <[email protected]>

---
 drivers/video/fbdev/Kconfig        | 2 +-
 drivers/video/fbdev/core/fbsysfs.c | 8 ++++----
 include/linux/fb.h                 | 2 +-
 include/uapi/linux/fb.h            | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

drivers/video/fbdev/Kconfig +1 -1

index 591a13a59787..146ab2c347f8 100644
@@ -198,7 +198,7 @@ config FB_MACMODES
        default n
 
 config FB_BACKLIGHT
-	bool
+	tristate
 	depends on FB
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE

drivers/video/fbdev/core/fbsysfs.c +4 -4

index e31a182b42bf..44cca39f2b51 100644
@@ -60,7 +60,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
 	info->device = dev;
 	info->fbcon_rotate_hint = -1;
 
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 	mutex_init(&info->bl_curve_mutex);
 #endif

@@ -429,7 +429,7 @@ static ssize_t show_fbstate(struct device *device,
 	return snprintf(buf, PAGE_SIZE, "%d\n", fb_info->state);
 }
 
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 static ssize_t store_bl_curve(struct device *device,
 			      struct device_attribute *attr,
 			      const char *buf, size_t count)

@@ -510,7 +510,7 @@ static struct device_attribute device_attrs[] = {
 	__ATTR(stride, S_IRUGO, show_stride, NULL),
 	__ATTR(rotate, S_IRUGO|S_IWUSR, show_rotate, store_rotate),
 	__ATTR(state, S_IRUGO|S_IWUSR, show_fbstate, store_fbstate),
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 	__ATTR(bl_curve, S_IRUGO|S_IWUSR, show_bl_curve, store_bl_curve),
 #endif
 };

@@ -551,7 +551,7 @@ void fb_cleanup_device(struct fb_info *fb_info)
 	}
 }
 
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 /* This function generates a linear backlight curve
  *
  *     0: off

include/linux/fb.h +1 -1

index a3cab6dc9b44..7cdd31a69719 100644
@@ -485,7 +485,7 @@ struct fb_info {
 	struct list_head modelist;      /* mode list */
 	struct fb_videomode *mode;	/* current mode */
 
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 	/* assigned backlight device */
 	/* set before framebuffer registration, 
 	   remove after unregister */

include/uapi/linux/fb.h +1 -1

index 6cd9b198b7c6..07f14cd6791a 100644
@@ -393,7 +393,7 @@ struct fb_cursor {
 	struct fb_image	image;	/* Cursor image */
 };
 
-#ifdef CONFIG_FB_BACKLIGHT
+#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
 /* Settings for the generic backlight code */
 #define FB_BACKLIGHT_LEVELS	128
 #define FB_BACKLIGHT_MAX	0xFF

Signature · 1 line

2.17.1

Arnd Bergmann

2018-10-10 15:35 UTC #1 lore raw

Quoted 6 lines · Rob Clark

BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.
Signed-off-by Rob Clark <[email protected]>

I don't see anything immediately wrong, but anything related to BACKLIGHT_CLASS_DEVICE, BACKLIGHT_LCD_SUPPORT
and FB_BACKLIGHT is really fragile in Kconfig, because of the way those interact with other options.

I've applied your patch to my randconfig build tree for testing, let's see what happens there before you apply it.

Arnd

Rob Clark

author

2018-10-11 01:16 UTC #2 lore raw

Quoted 9 lines · Arnd Bergmann

Quoted 6 lines · Rob Clark
BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.
Signed-off-by Rob Clark <[email protected]>

Quoted 8 lines

I don't see anything immediately wrong, but anything related to BACKLIGHT_CLASS_DEVICE, BACKLIGHT_LCD_SUPPORT
and FB_BACKLIGHT is really fragile in Kconfig, because of the way those interact with other options.
I've applied your patch to my randconfig build tree for testing, let's see what happens there before you apply it.

thanks.. tbh the fragility of backlight vs kconfig is why I've procrastinated on fixing this for a while.. in the end the solution seems not as bad as I feared (and after a iteration or two makes rhel kernel builds for various archs happy, at least).. but defn some randconfig build testing is in order.

PS. discovering that the thing you need to fix (a) never really worked as intended, and (b) involves backlight + fbdev.. is never a good way to start your day ;-)

BR,
-R

Arnd Bergmann

Tested-by

2018-10-11 07:02 UTC #3 lore raw

Quoted 20 lines · Rob Clark

Quoted 9 lines · Arnd Bergmann
Quoted 6 lines · Rob Clark
BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.
Signed-off-by Rob Clark <[email protected]>
Quoted 8 lines
I don't see anything immediately wrong, but anything related to BACKLIGHT_CLASS_DEVICE, BACKLIGHT_LCD_SUPPORT
and FB_BACKLIGHT is really fragile in Kconfig, because of the way those interact with other options.
I've applied your patch to my randconfig build tree for testing, let's see what happens there before you apply it.

Quoted 5 lines

thanks.. tbh the fragility of backlight vs kconfig is why I've procrastinated on fixing this for a while.. in the end the solution seems not as bad as I feared (and after a iteration or two makes rhel kernel builds for various archs happy, at least).. but defn some randconfig build testing is in order.

Ok, my overnight testing shows no regression, so please add

Tested-by Arnd Bergmann <[email protected]>

Quoted 3 lines

PS. discovering that the thing you need to fix (a) never really worked as intended, and (b) involves backlight + fbdev.. is never a good way to start your day ;-)

I still have a bunch of related fixes in my tree that address randconfig builds that never worked. I think at one point I got a few 'Reviewed-by' replies from DRM folks, but then nobody picked it up and subsequently it stopped applying. I need to go back and dig out all the dependent patches from my randconfig tree and resend that.

Arnd

Rob Clark

author

2018-10-11 12:31 UTC #4 lore raw

Quoted 28 lines · Arnd Bergmann

Quoted 20 lines · Rob Clark
Quoted 9 lines · Arnd Bergmann
Quoted 6 lines · Rob Clark
BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.
Signed-off-by Rob Clark <[email protected]>
Quoted 8 lines
I don't see anything immediately wrong, but anything related to BACKLIGHT_CLASS_DEVICE, BACKLIGHT_LCD_SUPPORT
and FB_BACKLIGHT is really fragile in Kconfig, because of the way those interact with other options.
I've applied your patch to my randconfig build tree for testing, let's see what happens there before you apply it.
Quoted 5 lines
thanks.. tbh the fragility of backlight vs kconfig is why I've procrastinated on fixing this for a while.. in the end the solution seems not as bad as I feared (and after a iteration or two makes rhel kernel builds for various archs happy, at least).. but defn some randconfig build testing is in order.

Quoted 3 lines

Ok, my overnight testing shows no regression, so please add
Tested-by Arnd Bergmann <[email protected]>

Thanks

Quoted 11 lines

Quoted 4 lines
PS. discovering that the thing you need to fix (a) never really worked as intended, and (b) involves backlight + fbdev.. is never a good way to start your day ;-)
I still have a bunch of related fixes in my tree that address randconfig builds that never worked. I think at one point I got a few 'Reviewed-by' replies from DRM folks, but then nobody picked it up and subsequently it stopped applying. I need to go back and dig out all the dependent patches from my randconfig tree and resend that.

I assume those patches (and this one) would be picked up via fbdev tree?

BR,
-R

Arnd Bergmann

2018-10-11 12:52 UTC #5 lore raw

Quoted 40 lines · Rob Clark

Quoted 28 lines · Arnd Bergmann
Quoted 20 lines · Rob Clark
Quoted 9 lines · Arnd Bergmann
Quoted 6 lines · Rob Clark
BACKLIGHT_CLASS_DEVICE is already tristate, but a dependency FB_BACKLIGHT prevents it from being built as a module. There doesn't seem to be any particularly good reason for this, so switch FB_BACKLIGHT over to tristate.
Signed-off-by Rob Clark <[email protected]>
Quoted 8 lines
I don't see anything immediately wrong, but anything related to BACKLIGHT_CLASS_DEVICE, BACKLIGHT_LCD_SUPPORT
and FB_BACKLIGHT is really fragile in Kconfig, because of the way those interact with other options.
I've applied your patch to my randconfig build tree for testing, let's see what happens there before you apply it.
Quoted 5 lines
thanks.. tbh the fragility of backlight vs kconfig is why I've procrastinated on fixing this for a while.. in the end the solution seems not as bad as I feared (and after a iteration or two makes rhel kernel builds for various archs happy, at least).. but defn some randconfig build testing is in order.
Quoted 3 lines
Ok, my overnight testing shows no regression, so please add
Tested-by Arnd Bergmann <[email protected]>
Thanks
Quoted 5 lines
Quoted 4 lines
PS. discovering that the thing you need to fix (a) never really worked as intended, and (b) involves backlight + fbdev.. is never a good way to start your day ;-)

Quoted 8 lines

Quoted 6 lines
I still have a bunch of related fixes in my tree that address randconfig builds that never worked. I think at one point I got a few 'Reviewed-by' replies from DRM folks, but then nobody picked it up and subsequently it stopped applying. I need to go back and dig out all the dependent patches from my randconfig tree and resend that.
I assume those patches (and this one) would be picked up via fbdev tree?

I think the others should go through the DRM tree, but I don't think I'll have time to revisit them before the merge window, so don't wait for that.

Arnd