2024-02-02 09:51:20

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH] ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE

Commit 72fee6b0a3a4 ("fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH")
disabled availablity of the SH_MOBILE_LCDC driver on the RENESAS arch.
This innocent change has a significant side-effect on the ARM's
multi_v7_defconfig, because FB_BACKLIGHT symbol is no longer selected,
what in turn leaves BACKLIGHT_CLASS_DEVICE symbol selected only as
a module. The latter disables some backlight related code in the DRM
core, because the DRM core is set to be compiled-in in this defconfig.
This leaves all DRM display panels without integrated backlight control,
even if the needed modules have been properly loaded and probed.

Fix this by selecting BACKLIGHT_CLASS_DEVICE to be compiled-in in
multi_v7_defconfig.

Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 8f7445729cd0..b2955dcb5a53 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -777,6 +777,7 @@ CONFIG_FB_EFI=y
CONFIG_FB_WM8505=y
CONFIG_FB_SH_MOBILE_LCDC=y
CONFIG_FB_SIMPLE=y
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
CONFIG_BACKLIGHT_PWM=y
CONFIG_BACKLIGHT_AS3711=y
CONFIG_BACKLIGHT_GPIO=y
--
2.34.1



2024-02-02 11:08:07

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE

Hi Marek,

On Fri, Feb 2, 2024 at 10:51 AM Marek Szyprowski
<[email protected]> wrote:
> Commit 72fee6b0a3a4 ("fbdev: Restrict FB_SH_MOBILE_LCDC to SuperH")
> disabled availablity of the SH_MOBILE_LCDC driver on the RENESAS arch.
> This innocent change has a significant side-effect on the ARM's
> multi_v7_defconfig, because FB_BACKLIGHT symbol is no longer selected,
> what in turn leaves BACKLIGHT_CLASS_DEVICE symbol selected only as
> a module. The latter disables some backlight related code in the DRM

Oops, sorry for that.

> core, because the DRM core is set to be compiled-in in this defconfig.
> This leaves all DRM display panels without integrated backlight control,
> even if the needed modules have been properly loaded and probed.

Hmm, that's bad.

Is there any way to fix this in DRM?
A quick grep shows that DRM is using the full monty of
IS_{BUILTIN,ENABLED,MODULE,REACHABLE}(CONFIG_BACKLIGHT_CLASS_DEVICE).
Probably not all of them are in perfect sync?

Several DRM drivers do select BACKLIGHT_CLASS_DEVICE, but if that
does not work in the modular case, it should be fixed.

> Fix this by selecting BACKLIGHT_CLASS_DEVICE to be compiled-in in
> multi_v7_defconfig.
>
> Signed-off-by: Marek Szyprowski <[email protected]>

Sounds like a good interim solution.

Acked-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-02-02 11:20:52

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE

On Fri, Feb 2, 2024, at 11:07, Geert Uytterhoeven wrote:
> On Fri, Feb 2, 2024 at 10:51 AM Marek Szyprowski <[email protected]> wrote:
>> core, because the DRM core is set to be compiled-in in this defconfig.
>> This leaves all DRM display panels without integrated backlight control,
>> even if the needed modules have been properly loaded and probed.
>
> Hmm, that's bad.
>
> Is there any way to fix this in DRM?
> A quick grep shows that DRM is using the full monty of
> IS_{BUILTIN,ENABLED,MODULE,REACHABLE}(CONFIG_BACKLIGHT_CLASS_DEVICE).
> Probably not all of them are in perfect sync?

The IS_REACHABLE() ones are almost certainly bugs, as are the
'select BACKLIGHT_CLASS_DEVICE' ones we have in drivers/gpu.

> Several DRM drivers do select BACKLIGHT_CLASS_DEVICE, but if that
> does not work in the modular case, it should be fixed.

The select should do the right thing in principle, but mixing
it with depends is what causes circular dependencies. Unfortunately
trying to fix it likely also causes those, but I think it's worth
revisiting.

It should be possible to change it like this:

- change all DRM drivers that require the class to 'depends on
BACKLIGHT_CLASS_DEVICE'

- change all those drivers that can optionally use it to
'depends on BACKLIGHT_CLASS_DEVICE || !BACKLIGHT_CLASS_DEVICE'
to avoid the dependency on a loadable module

- Make BACKLIGHT_CLASS_DEVICE itself default to 'DRM' in order
to avoid regressions in defconfig files but still make it
possible to turn it off.

>> Fix this by selecting BACKLIGHT_CLASS_DEVICE to be compiled-in in
>> multi_v7_defconfig.
>>
>> Signed-off-by: Marek Szyprowski <[email protected]>
>
> Sounds like a good interim solution.
>
> Acked-by: Geert Uytterhoeven <[email protected]>

Thanks, I've applied it to the soc/defconfig branch now.

Arnd