diff mbox series

ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE

Message ID 20240202095044.1980696-1-m.szyprowski@samsung.com
State New
Headers
Series ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE |

Commit Message

Marek Szyprowski Feb. 2, 2024, 9:50 a.m. UTC 
  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 <m.szyprowski@samsung.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Geert Uytterhoeven Feb. 2, 2024, 11:07 a.m. UTC | #1 
Hi Marek,

On Fri, Feb 2, 2024 at 10:51 AM Marek Szyprowski
<m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>

Sounds like a good interim solution.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Arnd Bergmann Feb. 2, 2024, 11:20 a.m. UTC | #2 
On Fri, Feb 2, 2024, at 11:07, Geert Uytterhoeven wrote:
> On Fri, Feb 2, 2024 at 10:51 AM Marek Szyprowski <m.szyprowski@samsung.com> 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 <m.szyprowski@samsung.com>
>
> Sounds like a good interim solution.
>
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

     Arnd
diff mbox series

Patch

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