Re: [PATCH RFC v2 1/3] drm: client: add splash client

Hi Geert,

On Tue, Jan 06, 2026 at 03:47:49PM +0100, Geert Uytterhoeven wrote:
> Hi Franceso,
> 
> On Tue, 6 Jan 2026 at 15:26, Francesco Valla <[email protected]> wrote:
> > Add a DRM client that draws a simple splash, with possibility to show:
> >
> >   - a colored background;
> >   - a static BMP image (loaded as firmware);
> >   - the logo provided by EFI BGRT.
> >
> > The client is not meant to replace a full-featured bootsplash, but
> > rather to remove some complexity (and hopefully boot time) on small
> > embedded platforms or on systems with a limited scope (e.g: recovery
> > or manufacturing images).
> >
> > The background color can be set either at build time from a dedicated
> > config option or at runtime through the drm_client_lib.splash_color
> > command line parameter. Any color in RGB888 format can be used.
> >
> > If enabled, the static BMP image is loaded using the kernel firmware
> > infrastructure; a valid BMP image with 24bpp color and no compression
> > is expected. The name of the image can be set through the
> > drm_client_lib.splash_bmp kernel command line parameter, with the
> > default being 'drm_splash.bmp'.
> >
> > Just like the existing DRM clients, the splash can be enabled from the
> > kernel command line using drm_client_lib.active=splash.
> >
> > Signed-off-by: Francesco Valla <[email protected]>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/clients/drm_splash.c
> 
> > +#if IS_ENABLED(CONFIG_DRM_CLIENT_SPLASH_BMP_SUPPORT)
> 
> There is no need to protect this block with #if, as it does not generate
> any code.
> 

This was left here when I moved some code along - I'll remove the
guards.

> > +#define BMP_FILE_MAGIC_ID 0x4d42
> > +
> > +/* BMP header structures copied from drivers/video/fbdev/efifb.c */
> > +struct bmp_file_header {
> > +       u16 id;
> > +       u32 file_size;
> > +       u32 reserved;
> > +       u32 bitmap_offset;
> > +} __packed;
> > +
> > +struct bmp_dib_header {
> > +       u32 dib_header_size;
> > +       s32 width;
> > +       s32 height;
> > +       u16 planes;
> > +       u16 bpp;
> > +       u32 compression;
> > +       u32 bitmap_size;
> > +       u32 horz_resolution;
> > +       u32 vert_resolution;
> > +       u32 colors_used;
> > +       u32 colors_important;
> > +} __packed;
> > +#endif // CONFIG_DRM_CLIENT_SPLASH_BMP_SUPPORT
> 
> As per [1], all these values are little-endian.  Hence they should
> be declared as such using le16 or le32, and accessed using
> get_unalined_le{16,32}().
> 
> [1] https://en.wikipedia.org/wiki/BMP_file_format#File_structure
> 

This is obviously right, I'll use proper accessors in next version.

I wonder why this is done precisely this way inside the efifb driver,
but that's maybe because all EFI platforms are little-endian? 

> Gr{oetje,eeting}s,
> 
>                         Geert
>

Thank you!

Reagrds,
Francesco