Re: [PATCH v6 08/15] drm/client: splash: pick the image source in one place

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 27.10.26 um 14:53 schrieb Màxim Pedraza Padilla:
> The client chooses between two sources with a boolean: the BGRT if the
> firmware left one, otherwise the BMP loaded as firmware. A third one is
> coming, and with it an order of preference that a boolean cannot hold.
>
> Name the source in use with an enum, picked once by
> drm_splash_pick_source(), and have drm_splash_image_load() switch on
> it. No functional change.
>
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Màxim Pedraza Padilla <maximpedraza@gmail.com>
> ---
>   drivers/gpu/drm/clients/drm_splash.c | 45 +++++++++++++++++++++++-----
>   1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/clients/drm_splash.c b/drivers/gpu/drm/clients/drm_splash.c
> index 337c7160c4a1..911149b92a52 100644
> --- a/drivers/gpu/drm/clients/drm_splash.c
> +++ b/drivers/gpu/drm/clients/drm_splash.c
> @@ -39,6 +39,16 @@
>    * a static image.
>    */
>   
> +/*
> + * Where the image comes from. drm_splash_pick_source() sets the order of
> + * preference; NONE is zero, so that a new splash starts without a source.
> + */
> +enum drm_splash_source {
> +	DRM_SPLASH_SRC_NONE,
> +	DRM_SPLASH_SRC_BGRT,
> +	DRM_SPLASH_SRC_BMP,

I'd call this DRM_SPLASH_SRC_FW, because that's what you're loading 
from.  Although the format is a BMP, BMP is also the format of BGRT.

Patch 4 already mixed this up. Renaming the BMP names there to FW seems 
sensible.

The rest of the patch looks good.

> +};
> +
>   struct drm_splash_scanout {
>   	int id;
>   	/* Modesets scanning this buffer out, by position in the list */
> @@ -87,7 +97,7 @@ struct drm_splash {
>   	const struct firmware *fw;
>   	void *map_data;
>   
> -	bool use_bgrt;
> +	enum drm_splash_source src;
>   };
>   
>   static struct drm_splash *client_to_drm_splash(struct drm_client_dev *client)
> @@ -429,7 +439,7 @@ static int drm_splash_bmp_to_scanout(struct drm_splash *splash,
>   		return -EINVAL;
>   	}
>   
> -	if (splash->use_bgrt) {
> +	if (splash->src == DRM_SPLASH_SRC_BGRT) {
>   		x_pad = efi_bgrt_image_offset_x();
>   		y_pad = efi_bgrt_image_offset_y();
>   
> @@ -498,10 +508,31 @@ static int drm_splash_bmp_to_scanout(struct drm_splash *splash,
>   static int drm_splash_image_load(struct drm_splash *splash, const u8 **img_data,
>   				 size_t *img_data_len)
>   {
> -	if (splash->use_bgrt)
> +	switch (splash->src) {
> +	case DRM_SPLASH_SRC_BGRT:
>   		return drm_splash_bgrt_load(splash, img_data, img_data_len);
> +	case DRM_SPLASH_SRC_BMP:
> +		return drm_splash_fw_load(splash, img_data, img_data_len);
> +	case DRM_SPLASH_SRC_NONE:
> +		break;
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +/*
> + * Pick the one source the image will come from: the BGRT is whatever the
> + * firmware shows, and the BMP loaded as firmware is the fallback. With
> + * neither, only the background colour is drawn.
> + */
> +static enum drm_splash_source drm_splash_pick_source(struct drm_splash *splash)
> +{
> +	if (drm_splash_bgrt_available(splash))
> +		return DRM_SPLASH_SRC_BGRT;
> +	if (IS_ENABLED(CONFIG_DRM_CLIENT_SPLASH_SRC_BMP))
> +		return DRM_SPLASH_SRC_BMP;
>   
> -	return drm_splash_fw_load(splash, img_data, img_data_len);
> +	return DRM_SPLASH_SRC_NONE;
>   }
>   
>   /* At unregister, once nothing else can use the image */
> @@ -818,10 +849,10 @@ static int drm_splash_client_hotplug(struct drm_client_dev *client)
>   
>   	if (!splash->source_picked) {
>   		splash->source_picked = true;
> -		splash->use_bgrt = drm_splash_bgrt_available(splash);
> +		splash->src = drm_splash_pick_source(splash);
>   
> -		/* If no other image has been loaded, try to load a BMP as firmware */
> -		if (IS_ENABLED(CONFIG_DRM_CLIENT_SPLASH_SRC_BMP) && !splash->use_bgrt) {
> +		/* The BMP loaded as firmware is the only source that arrives later */
> +		if (splash->src == DRM_SPLASH_SRC_BMP) {
>   			ret = drm_splash_fw_request_bmp(splash);
>   			splash->fw_requested = !ret;
>   			if (ret)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)