Re: [PATCH v2 6/6] video/logo: move logo selection logic to Kconfig

Hi Vincent,

On Tue, 6 Jan 2026 at 21:10, Vincent Mailhol <[email protected]> wrote:
> On 06/01/2026 at 12:48, Geert Uytterhoeven wrote:
> > Thanks for your patch, which is now commit bd710b3da7308cb1
> > ("video/logo: move logo selection logic to Kconfig") in fbdev/for-next.
> >
> > On Thu, 1 Jan 2026 at 16:26, Vincent Mailhol <[email protected]> wrote:
> >> Now that the path to the logo file can be directly entered in Kbuild,
> >> there is no more need to handle all the logo file selection in the
> >> Makefile and the C files.
> >
> > This may do the wrong thing when booting a multi-platform kernel.
> >
> >>
> >> The only exception is the logo_spe_clut224 which is only used by the
> >> Cell processor (found for example in the Playstation 3) [1]. This
> >> extra logo uses its own different image which shows up on a separate
> >> line just below the normal logo. Because the extra logo uses a
> >> different image, it can not be factorized under the custom logo logic.
> >>
> >> Move all the logo file selection logic to Kbuild (except from the
> >> logo_spe_clut224.ppm), this done, clean-up the C code to only leave
> >> one entry for each logo type (monochrome, 16-colors and 224-colors).
> >>
> >> [1] Cell SPE logos
> >> Link: https://lore.kernel.org/all/[email protected]/
> >>
> >> Signed-off-by: Vincent Mailhol <[email protected]>
> >
> >> --- a/drivers/video/logo/Kconfig
> >> +++ b/drivers/video/logo/Kconfig
> >
> >> @@ -61,6 +63,12 @@ config LOGO_LINUX_CLUT224
> >>  config LOGO_LINUX_CLUT224_FILE
> >>         string "224-color logo .ppm file"
> >>         depends on LOGO_LINUX_CLUT224
> >> +       default "drivers/video/logo/logo_dec_clut224.ppm" if MACH_DECSTATION || ALPHA
> >> +       default "drivers/video/logo/logo_mac_clut224.ppm" if MAC
> >
> > E.g. an m68k multi-platform kernel including Mac support will scare
> > non-Mac users into thinking their machine was assimilated by the
> > Apple Empire...
> >
> >> +       default "drivers/video/logo/logo_parisc_clut224.ppm" if PARISC
> >> +       default "drivers/video/logo/logo_sgi_clut224.ppm" if SGI_IP22 || SGI_IP27 || SGI_IP32
> >> +       default "drivers/video/logo/logo_sun_clut224.ppm" if SPARC
> >> +       default "drivers/video/logo/logo_superh_clut224.ppm" if SUPERH
> >>         default "drivers/video/logo/logo_linux_clut224.ppm"
> >>         help
> >>           Takes a path to a 224-color logo in the portable pixmap file
> >
> >> --- a/drivers/video/logo/logo.c
> >> +++ b/drivers/video/logo/logo.c
> >> @@ -48,59 +48,21 @@ const struct linux_logo * __ref fb_find_logo(int depth)
> >>         if (nologo || logos_freed)
> >>                 return NULL;
> >>
> >> -       if (depth >= 1) {
> >>  #ifdef CONFIG_LOGO_LINUX_MONO
> >> -               /* Generic Linux logo */
> >> +       if (depth >= 1)
> >>                 logo = &logo_linux_mono;
> >>  #endif
> >> -#ifdef CONFIG_LOGO_SUPERH_MONO
> >> -               /* SuperH Linux logo */
> >> -               logo = &logo_superh_mono;
> >> -#endif
> >> -       }
> >>
> >> -       if (depth >= 4) {
> >>  #ifdef CONFIG_LOGO_LINUX_VGA16
> >> -               /* Generic Linux logo */
> >> +       if (depth >= 4)
> >>                 logo = &logo_linux_vga16;
> >>  #endif
> >> -#ifdef CONFIG_LOGO_SUPERH_VGA16
> >> -               /* SuperH Linux logo */
> >> -               logo = &logo_superh_vga16;
> >> -#endif
> >> -       }
> >>
> >> -       if (depth >= 8) {
> >>  #ifdef CONFIG_LOGO_LINUX_CLUT224
> >> -               /* Generic Linux logo */
> >> +       if (depth >= 8)
> >>                 logo = &logo_linux_clut224;
> >>  #endif
> >> -#ifdef CONFIG_LOGO_DEC_CLUT224
> >> -               /* DEC Linux logo on MIPS/MIPS64 or ALPHA */
> >> -               logo = &logo_dec_clut224;
> >> -#endif
> >> -#ifdef CONFIG_LOGO_MAC_CLUT224
> >> -               /* Macintosh Linux logo on m68k */
> >> -               if (MACH_IS_MAC)
> >
> > MACH_IS_MAC can be a runtime check.
>
> OK. I missed this.
>
> I think there are two options to fix this:
>
>   1. Keep CONFIG_LOGO_MAC_CLUT224 untouched
>   2. Remove logo_mac_clut224.ppm
>
> The first option is less controversial but I would like to ask you what
> you think about removing the logo_mac_clut224 file.
>
> Here, we are speaking of the Macintosh 68k which ended sales in 1995,
> right? So the user base should be rather small, I guess.

Yes, the user base is small.

BTW, the only reason you don't have this issue with MACH_DECSTATION and
the various SGI_IP* options is that MIPS does not support multi-platform
kernels.

> And people who still want the custom MAC logo would still be able to add
>
>   CONFIG_LOGO_MAC_CLUT224="path/to/logo_mac_clut224.ppm"

LOGO_LINUX_CLUT224_FILE ;-)

> to their config to restore the old behaviour anyway.
>
> My choice would go more toward the removal option but what do you think?

I am not too attached to keeping the dynamic behavior for the Mac logo,
I just wanted to point out the impact.
I expect most people who care about logos (in products) just have their
own custom out-of-tree code.  As fb_find_logo() and the underlying
infrastructure still exists, I don't expect them to have too much
trouble forward porting that to newer kernels.

What do other people think?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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