Re: [PATCH 0/2] video: fix build with VIDEO && !VIDEO_LOGO

Re: [PATCH 0/2] video: fix build with VIDEO && !VIDEO_LOGO

[Julien Stephan]

I messed up the u-boot list, removing the old one, adding the new one.
Sorry for the noise

Le mar. 1 sept. 2026 à 10:52, Julien Stephan <jstephan@baylibre.com> a écrit :
>
> While enabling a splash screen I enabled SPLASH_SCREEN which
> automatically disable CONFIG_VIDEO_LOGO (while keeping CONFIG_VIDEO))
> and hit a link failure:
>
>   video-uclass.o: in function `video_get_u_boot_logo':
>   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
>
> The __splash_u_boot_logo_begin/_end symbols come from u_boot_logo.bmp.o,
> which is only built when CONFIG_VIDEO_LOGO is set, but the splash helpers
> reference them unconditionally.
>
> Downstream just worked around this by unsetting EXPO, which is the only
> caller of video_get_u_boot_logo, but that's not a real fix, so patch 2
> guards the splash code (and adds a NULL-returning stub for
> video_get_u_boot_logo()) so VIDEO without VIDEO_LOGO builds.
>
> While at it, patch 1 makes show_splash() actually return the
> video_bmp_display() result instead of discarding it and returning 0.
> This has the side effect of actually making the whole video device fail
> to probe (no display), where before the console came up fine minus the
> logo.
>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
> Julien Stephan (2):
>       video: propagate show_splash() return value
>       video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO
>
>  drivers/video/video-uclass.c | 12 ++++++------
>  include/video.h              |  9 ++++++++-
>  2 files changed, 14 insertions(+), 7 deletions(-)
> ---
> base-commit: a18265f1ccb7a272721ed4286ed3b5a6182ff424
> change-id: 20260901-fix-build-with-config-video-but-config-video-logo-disabled-f56fffc664eb
>
> Best regards,
> --
> Julien Stephan <jstephan@baylibre.com>
>

Re: [PATCH 1/2] video: propagate show_splash() return value

[Julien Stephan]

I messed up the u-boot list, removing the old one, adding the new one.
Sorry for the noise

Le mar. 1 sept. 2026 à 10:52, Julien Stephan <jstephan@baylibre.com> a écrit :
>
> show_splash() computed the result of video_bmp_display() but discarded
> it and always returned 0, so a failing splash display was silently
> ignored by video_post_probe(). Return the value directly instead.
>
> This has the side effect of actually making the whole video device fail
> to probe (no display), where before the console came up fine minus the
> logo.
>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>  drivers/video/video-uclass.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 228d6bacc58..de161054d52 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -595,11 +595,8 @@ void *video_get_u_boot_logo(void)
>  static int show_splash(struct udevice *dev)
>  {
>         u8 *data = SPLASH_START(u_boot_logo);
> -       int ret;
> -
> -       ret = video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
>
> -       return 0;
> +       return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
>  }
>
>  int video_default_font_height(struct udevice *dev)
>
> --
> 2.55.0
>

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Julien Stephan]

I messed up the u-boot list, removing the old one, adding the new one.
Sorry for the noise

Le mar. 1 sept. 2026 à 10:52, Julien Stephan <jstephan@baylibre.com> a écrit :
>
> Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> at link time:
>
>   video-uclass.o: in function `video_get_u_boot_logo':
>   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
>
> The __splash_u_boot_logo_begin/_end symbols are provided by
> u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
>
>   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
>
> video_get_u_boot_logo() and show_splash() reference those symbols
> unconditionally, so with the logo disabled the reference is left
> dangling. show_splash() alone would be dead-code eliminated (it is
> static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> but video_get_u_boot_logo() is an exported function and is always
> emitted.
>
> Guard the splash helpers and their symbol references with
> CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> video_get_u_boot_logo() stub returning NULL for the disabled case in
> video.h. Callers already handle a NULL logo pointer (e.g.
> bootflow_menu.c), so no caller changes are needed.
>
> Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
>
> Fixes: 0d3890188d6b ("video: Add function to obtain the U-Boot logo")
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>  drivers/video/video-uclass.c | 7 +++++--
>  include/video.h              | 9 ++++++++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index de161054d52..4c959a57619 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
>         return priv->ysize;
>  }
>
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
>  #define SPLASH_DECL(_name) \
>         extern u8 __splash_ ## _name ## _begin[]; \
>         extern u8 __splash_ ## _name ## _end[]
> @@ -598,6 +599,7 @@ static int show_splash(struct udevice *dev)
>
>         return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
>  }
> +#endif
>
>  int video_default_font_height(struct udevice *dev)
>  {
> @@ -716,14 +718,15 @@ static int video_post_probe(struct udevice *dev)
>                 return ret;
>         }
>
> -       if (CONFIG_IS_ENABLED(VIDEO_LOGO) &&
> -           !CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> +       if (!CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
>                 ret = show_splash(dev);
>                 if (ret) {
>                         log_debug("Cannot show splash screen\n");
>                         return ret;
>                 }
>         }
> +#endif
>
>         /* register cyclic as soon as the first video device is probed */
>         if (CONFIG_IS_ENABLED(CYCLIC) && (gd->flags && GD_FLG_RELOC) &&
> diff --git a/include/video.h b/include/video.h
> index 9ea6b676463..8e4c1544e56 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -418,9 +418,16 @@ bool video_is_active(void);
>  /**
>   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
>   *
> - * Returns: Pointer to logo
> + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
>   */
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
>  void *video_get_u_boot_logo(void);
> +#else
> +static inline void *video_get_u_boot_logo(void)
> +{
> +       return NULL;
> +}
> +#endif
>
>  /*
>   * bmp_display() - Display BMP (bitmap) data located in memory
>
> --
> 2.55.0
>

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

On Tue, Sep 01, 2026 at 10:52:43AM +0200, Julien Stephan wrote:
> Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> at link time:
> 
>   video-uclass.o: in function `video_get_u_boot_logo':
>   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
> 
> The __splash_u_boot_logo_begin/_end symbols are provided by
> u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
> 
>   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> 
> video_get_u_boot_logo() and show_splash() reference those symbols
> unconditionally, so with the logo disabled the reference is left
> dangling. show_splash() alone would be dead-code eliminated (it is
> static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> but video_get_u_boot_logo() is an exported function and is always
> emitted.
> 
> Guard the splash helpers and their symbol references with
> CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> video_get_u_boot_logo() stub returning NULL for the disabled case in
> video.h. Callers already handle a NULL logo pointer (e.g.
> bootflow_menu.c), so no caller changes are needed.
> 
> Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
> 
> Fixes: 0d3890188d6b ("video: Add function to obtain the U-Boot logo")
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>  drivers/video/video-uclass.c | 7 +++++--
>  include/video.h              | 9 ++++++++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index de161054d52..4c959a57619 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
>  	return priv->ysize;
>  }
>  
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
>  #define SPLASH_DECL(_name) \
>  	extern u8 __splash_ ## _name ## _begin[]; \
>  	extern u8 __splash_ ## _name ## _end[]
> @@ -598,6 +599,7 @@ static int show_splash(struct udevice *dev)
>  
>  	return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
>  }
> +#endif
>  
>  int video_default_font_height(struct udevice *dev)
>  {
> @@ -716,14 +718,15 @@ static int video_post_probe(struct udevice *dev)
>  		return ret;
>  	}
>  
> -	if (CONFIG_IS_ENABLED(VIDEO_LOGO) &&
> -	    !CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> +	if (!CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
>  		ret = show_splash(dev);
>  		if (ret) {
>  			log_debug("Cannot show splash screen\n");
>  			return ret;
>  		}
>  	}
> +#endif

Is this hunk really needed? I can see getting here as part of debugging
the problem, but before the change it should evaluate to 'if (0 && ...)'
and be link-time eliminated.

>  
>  	/* register cyclic as soon as the first video device is probed */
>  	if (CONFIG_IS_ENABLED(CYCLIC) && (gd->flags && GD_FLG_RELOC) &&
> diff --git a/include/video.h b/include/video.h
> index 9ea6b676463..8e4c1544e56 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -418,9 +418,16 @@ bool video_is_active(void);
>  /**
>   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
>   *
> - * Returns: Pointer to logo
> + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
>   */
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
>  void *video_get_u_boot_logo(void);
> +#else
> +static inline void *video_get_u_boot_logo(void)
> +{
> +	return NULL;
> +}
> +#endif

This part does make sense.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Julien Stephan]

Le mar. 1 sept. 2026 à 18:42, Tom Rini <trini@konsulko.com> a écrit :
>
> On Tue, Sep 01, 2026 at 10:52:43AM +0200, Julien Stephan wrote:
> > Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> > at link time:
> >
> >   video-uclass.o: in function `video_get_u_boot_logo':
> >   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
> >
> > The __splash_u_boot_logo_begin/_end symbols are provided by
> > u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
> >
> >   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> >
> > video_get_u_boot_logo() and show_splash() reference those symbols
> > unconditionally, so with the logo disabled the reference is left
> > dangling. show_splash() alone would be dead-code eliminated (it is
> > static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> > but video_get_u_boot_logo() is an exported function and is always
> > emitted.
> >
> > Guard the splash helpers and their symbol references with
> > CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> > video_get_u_boot_logo() stub returning NULL for the disabled case in
> > video.h. Callers already handle a NULL logo pointer (e.g.
> > bootflow_menu.c), so no caller changes are needed.
> >
> > Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> > enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
> >
> > Fixes: 0d3890188d6b ("video: Add function to obtain the U-Boot logo")
> > Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> > ---
> >  drivers/video/video-uclass.c | 7 +++++--
> >  include/video.h              | 9 ++++++++-
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> > index de161054d52..4c959a57619 100644
> > --- a/drivers/video/video-uclass.c
> > +++ b/drivers/video/video-uclass.c
> > @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
> >       return priv->ysize;
> >  }
> >
> > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> >  #define SPLASH_DECL(_name) \
> >       extern u8 __splash_ ## _name ## _begin[]; \
> >       extern u8 __splash_ ## _name ## _end[]
> > @@ -598,6 +599,7 @@ static int show_splash(struct udevice *dev)
> >
> >       return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
> >  }
> > +#endif
> >
> >  int video_default_font_height(struct udevice *dev)
> >  {
> > @@ -716,14 +718,15 @@ static int video_post_probe(struct udevice *dev)
> >               return ret;
> >       }
> >
> > -     if (CONFIG_IS_ENABLED(VIDEO_LOGO) &&
> > -         !CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> > +     if (!CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> >               ret = show_splash(dev);
> >               if (ret) {
> >                       log_debug("Cannot show splash screen\n");
> >                       return ret;
> >               }
> >       }
> > +#endif
>
> Is this hunk really needed? I can see getting here as part of debugging
> the problem, but before the change it should evaluate to 'if (0 && ...)'
> and be link-time eliminated.
>

Hi Tom,

Yes it is needed since I moved show_splash() inside the #if
CONFIG_IS_ENABLED(VIDEO_LOGO) guard above.
I can go back to the runtime check, and keep show_splash() outside of
the guard, but I'll have to use video_get_u_boot_logo() instead of
relying on SPLASH_START(u_boot_logo);

What do you prefer?

Cheers
Julien
> >
> >       /* register cyclic as soon as the first video device is probed */
> >       if (CONFIG_IS_ENABLED(CYCLIC) && (gd->flags && GD_FLG_RELOC) &&
> > diff --git a/include/video.h b/include/video.h
> > index 9ea6b676463..8e4c1544e56 100644
> > --- a/include/video.h
> > +++ b/include/video.h
> > @@ -418,9 +418,16 @@ bool video_is_active(void);
> >  /**
> >   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
> >   *
> > - * Returns: Pointer to logo
> > + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
> >   */
> > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> >  void *video_get_u_boot_logo(void);
> > +#else
> > +static inline void *video_get_u_boot_logo(void)
> > +{
> > +     return NULL;
> > +}
> > +#endif
>
> This part does make sense.
>
> --
> Tom

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4315 bytes --]

On Wed, Sep 02, 2026 at 10:48:38AM +0200, Julien Stephan wrote:
> Le mar. 1 sept. 2026 à 18:42, Tom Rini <trini@konsulko.com> a écrit :
> >
> > On Tue, Sep 01, 2026 at 10:52:43AM +0200, Julien Stephan wrote:
> > > Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> > > at link time:
> > >
> > >   video-uclass.o: in function `video_get_u_boot_logo':
> > >   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
> > >
> > > The __splash_u_boot_logo_begin/_end symbols are provided by
> > > u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
> > >
> > >   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> > >
> > > video_get_u_boot_logo() and show_splash() reference those symbols
> > > unconditionally, so with the logo disabled the reference is left
> > > dangling. show_splash() alone would be dead-code eliminated (it is
> > > static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> > > but video_get_u_boot_logo() is an exported function and is always
> > > emitted.
> > >
> > > Guard the splash helpers and their symbol references with
> > > CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> > > video_get_u_boot_logo() stub returning NULL for the disabled case in
> > > video.h. Callers already handle a NULL logo pointer (e.g.
> > > bootflow_menu.c), so no caller changes are needed.
> > >
> > > Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> > > enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
> > >
> > > Fixes: 0d3890188d6b ("video: Add function to obtain the U-Boot logo")
> > > Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> > > ---
> > >  drivers/video/video-uclass.c | 7 +++++--
> > >  include/video.h              | 9 ++++++++-
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> > > index de161054d52..4c959a57619 100644
> > > --- a/drivers/video/video-uclass.c
> > > +++ b/drivers/video/video-uclass.c
> > > @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
> > >       return priv->ysize;
> > >  }
> > >
> > > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> > >  #define SPLASH_DECL(_name) \
> > >       extern u8 __splash_ ## _name ## _begin[]; \
> > >       extern u8 __splash_ ## _name ## _end[]
> > > @@ -598,6 +599,7 @@ static int show_splash(struct udevice *dev)
> > >
> > >       return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);
> > >  }
> > > +#endif
> > >
> > >  int video_default_font_height(struct udevice *dev)
> > >  {
> > > @@ -716,14 +718,15 @@ static int video_post_probe(struct udevice *dev)
> > >               return ret;
> > >       }
> > >
> > > -     if (CONFIG_IS_ENABLED(VIDEO_LOGO) &&
> > > -         !CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> > > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> > > +     if (!CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) {
> > >               ret = show_splash(dev);
> > >               if (ret) {
> > >                       log_debug("Cannot show splash screen\n");
> > >                       return ret;
> > >               }
> > >       }
> > > +#endif
> >
> > Is this hunk really needed? I can see getting here as part of debugging
> > the problem, but before the change it should evaluate to 'if (0 && ...)'
> > and be link-time eliminated.
> >
> 
> Hi Tom,
> 
> Yes it is needed since I moved show_splash() inside the #if
> CONFIG_IS_ENABLED(VIDEO_LOGO) guard above.
> I can go back to the runtime check, and keep show_splash() outside of
> the guard, but I'll have to use video_get_u_boot_logo() instead of
> relying on SPLASH_START(u_boot_logo);
> 
> What do you prefer?

Ah, I see now, that wasn't clear to me from the context. Looking at 1/2
and then 2/2 now, can we just remove show_splash() and call
video_bmp_display directly? That means not guarding SPLASH_START but
again it should optimize away. I do complain about how if
(CONFIG_IS_ENABLED(...)) isn't always great, but I think mixing that and
#if together just reads awkwardly too. So if we can't restructure the
code to keep if (CONFIG_IS_ENABLED(...)) then we should #if the whole
block there. Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Simon Glass]

Hi Julien,

On 2026-09-01T08:52:42, Julien Stephan <jstephan@baylibre.com> wrote:
> video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO
>
> Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> at link time:
>
>   video-uclass.o: in function `video_get_u_boot_logo':
>   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
>
> The __splash_u_boot_logo_begin/_end symbols are provided by
> u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
>
>   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
>
> video_get_u_boot_logo() and show_splash() reference those symbols
> unconditionally, so with the logo disabled the reference is left
> dangling. show_splash() alone would be dead-code eliminated (it is
> static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> but video_get_u_boot_logo() is an exported function and is always
> emitted.
>
> Guard the splash helpers and their symbol references with
> CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> video_get_u_boot_logo() stub returning NULL for the disabled case in
> video.h. Callers already handle a NULL logo pointer (e.g.
> bootflow_menu.c), so no caller changes are needed.
>
> Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).

I won't comment on the word order / mixed tenses.

> [...]
>
> drivers/video/video-uclass.c | 7 +++++--
>  include/video.h              | 9 ++++++++-
>  2 files changed, 13 insertions(+), 3 deletions(-)

> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
>       return priv->ysize;
>  }
>
> +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
>  #define SPLASH_DECL(_name) \
>       extern u8 __splash_ ## _name ## _begin[]; \
>       extern u8 __splash_ ## _name ## _end[]

We try to avoid #ifdef in .c files and prefer if
(CONFIG_IS_ENABLED(...)) so both branches are compile-checked. The
SPLASH_DECL externs do need the preprocessor guard since the linker
symbols don't exist, but the guard around show_splash() and its call
site in video_post_probe() could be avoided by keeping show_splash()
defined and providing an #else stub that returns 0. The existing if
(CONFIG_IS_ENABLED(VIDEO_LOGO) && ...) at the caller can then stay.

> diff --git a/include/video.h b/include/video.h
> @@ -418,9 +418,16 @@ bool video_is_active(void);
>  /**
>   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
>   *
> - * Returns: Pointer to logo
> + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
>   */

The stub is guarded with CONFIG_IS_ENABLED(VIDEO_LOGO), so the doc
should refer to VIDEO_LOGO in general terms - SPL may have it disabled
while proper enables it. Something like 'Returns: Pointer to logo, or
NULL if VIDEO_LOGO is not enabled in this phase'.

Regards,
Simon

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3379 bytes --]

On Fri, Sep 18, 2026 at 03:49:44PM -0500, Simon Glass wrote:
> Hi Julien,
> 
> On 2026-09-01T08:52:42, Julien Stephan <jstephan@baylibre.com> wrote:
> > video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO
> >
> > Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> > at link time:
> >
> >   video-uclass.o: in function `video_get_u_boot_logo':
> >   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
> >
> > The __splash_u_boot_logo_begin/_end symbols are provided by
> > u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
> >
> >   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> >
> > video_get_u_boot_logo() and show_splash() reference those symbols
> > unconditionally, so with the logo disabled the reference is left
> > dangling. show_splash() alone would be dead-code eliminated (it is
> > static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> > but video_get_u_boot_logo() is an exported function and is always
> > emitted.
> >
> > Guard the splash helpers and their symbol references with
> > CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> > video_get_u_boot_logo() stub returning NULL for the disabled case in
> > video.h. Callers already handle a NULL logo pointer (e.g.
> > bootflow_menu.c), so no caller changes are needed.
> >
> > Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> > enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
> 
> I won't comment on the word order / mixed tenses.
> 
> > [...]
> >
> > drivers/video/video-uclass.c | 7 +++++--
> >  include/video.h              | 9 ++++++++-
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> > diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> > @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
> >       return priv->ysize;
> >  }
> >
> > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> >  #define SPLASH_DECL(_name) \
> >       extern u8 __splash_ ## _name ## _begin[]; \
> >       extern u8 __splash_ ## _name ## _end[]
> 
> We try to avoid #ifdef in .c files and prefer if
> (CONFIG_IS_ENABLED(...)) so both branches are compile-checked. The
> SPLASH_DECL externs do need the preprocessor guard since the linker
> symbols don't exist, but the guard around show_splash() and its call
> site in video_post_probe() could be avoided by keeping show_splash()
> defined and providing an #else stub that returns 0. The existing if
> (CONFIG_IS_ENABLED(VIDEO_LOGO) && ...) at the caller can then stay.
> 
> > diff --git a/include/video.h b/include/video.h
> > @@ -418,9 +418,16 @@ bool video_is_active(void);
> >  /**
> >   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
> >   *
> > - * Returns: Pointer to logo
> > + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
> >   */
> 
> The stub is guarded with CONFIG_IS_ENABLED(VIDEO_LOGO), so the doc
> should refer to VIDEO_LOGO in general terms - SPL may have it disabled
> while proper enables it. Something like 'Returns: Pointer to logo, or
> NULL if VIDEO_LOGO is not enabled in this phase'.

Simon, can you please make sure you're looking at both the current
version, and previous feedback, when commenting on older patches?
There's been a v2 for about 2 weeks. Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Simon Glass]

Hi Tom,

On Fri, 18 Sept 2026 at 16:41, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 18, 2026 at 03:49:44PM -0500, Simon Glass wrote:
> > Hi Julien,
> >
> > On 2026-09-01T08:52:42, Julien Stephan <jstephan@baylibre.com> wrote:
> > > video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO
> > >
> > > Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails
> > > at link time:
> > >
> > >   video-uclass.o: in function `video_get_u_boot_logo':
> > >   video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin'
> > >
> > > The __splash_u_boot_logo_begin/_end symbols are provided by
> > > u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set:
> > >
> > >   obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o
> > >
> > > video_get_u_boot_logo() and show_splash() reference those symbols
> > > unconditionally, so with the logo disabled the reference is left
> > > dangling. show_splash() alone would be dead-code eliminated (it is
> > > static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard),
> > > but video_get_u_boot_logo() is an exported function and is always
> > > emitted.
> > >
> > > Guard the splash helpers and their symbol references with
> > > CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline
> > > video_get_u_boot_logo() stub returning NULL for the disabled case in
> > > video.h. Callers already handle a NULL logo pointer (e.g.
> > > bootflow_menu.c), so no caller changes are needed.
> > >
> > > Reproduce with any board that enables VIDEO without VIDEO_LOGO or
> > > enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO).
> >
> > I won't comment on the word order / mixed tenses.
> >
> > > [...]
> > >
> > > drivers/video/video-uclass.c | 7 +++++--
> > >  include/video.h              | 9 ++++++++-
> > >  2 files changed, 13 insertions(+), 3 deletions(-)
> >
> > > diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> > > @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev)
> > >       return priv->ysize;
> > >  }
> > >
> > > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> > >  #define SPLASH_DECL(_name) \
> > >       extern u8 __splash_ ## _name ## _begin[]; \
> > >       extern u8 __splash_ ## _name ## _end[]
> >
> > We try to avoid #ifdef in .c files and prefer if
> > (CONFIG_IS_ENABLED(...)) so both branches are compile-checked. The
> > SPLASH_DECL externs do need the preprocessor guard since the linker
> > symbols don't exist, but the guard around show_splash() and its call
> > site in video_post_probe() could be avoided by keeping show_splash()
> > defined and providing an #else stub that returns 0. The existing if
> > (CONFIG_IS_ENABLED(VIDEO_LOGO) && ...) at the caller can then stay.
> >
> > > diff --git a/include/video.h b/include/video.h
> > > @@ -418,9 +418,16 @@ bool video_is_active(void);
> > >  /**
> > >   * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
> > >   *
> > > - * Returns: Pointer to logo
> > > + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
> > >   */
> >
> > The stub is guarded with CONFIG_IS_ENABLED(VIDEO_LOGO), so the doc
> > should refer to VIDEO_LOGO in general terms - SPL may have it disabled
> > while proper enables it. Something like 'Returns: Pointer to logo, or
> > NULL if VIDEO_LOGO is not enabled in this phase'.
>
> Simon, can you please make sure you're looking at both the current
> version, and previous feedback, when commenting on older patches?
> There's been a v2 for about 2 weeks. Thanks.

Ooops, yes will do.

Regards,
Simon