Re: [PATCH v5 02/13] efi: bgrt: add accessors for the boot graphics

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 13.10.26 um 11:32 schrieb Màxim Pedraza Padilla:
> The DRM splash client shows the boot graphics the firmware left behind,
> and needs the image address and size, its offsets and the status bits.
> Reading them from bgrt_tab and bgrt_image_size works with drm_client_lib
> built in, but it can be a module, and neither is exported:
>
>    ERROR: modpost: drivers/gpu/drm/clients/drm_client_lib.ko: symbol 'bgrt_tab' undefined!
>    ERROR: modpost: drivers/gpu/drm/clients/drm_client_lib.ko: symbol 'bgrt_image_size' undefined!
>
> Rather than export the data, add an accessor for each of them, exported
> GPL only, with stubs in efi-bgrt.h for CONFIG_ACPI_BGRT=n so that users
> need no #ifdef of their own. The variables stay global for
> drivers/acpi/bgrt.c, and are only written by efi_bgrt_init(): mark them
> __ro_after_init.
>
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Closes: https://lore.kernel.org/dri-devel/5d7067de-97b7-4232-9cf6-e4b978696482@amd.com/
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Màxim Pedraza Padilla <maximpedraza@gmail.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/firmware/efi/efi-bgrt.c | 74 ++++++++++++++++++++++++++++++++-
>   include/linux/efi-bgrt.h        | 39 ++++++++++++++++-
>   2 files changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
> index 1da451582812..01425c4d2e70 100644
> --- a/drivers/firmware/efi/efi-bgrt.c
> +++ b/drivers/firmware/efi/efi-bgrt.c
> @@ -16,8 +16,8 @@
>   #include <linux/efi.h>
>   #include <linux/efi-bgrt.h>
>   
> -struct acpi_table_bgrt bgrt_tab;
> -size_t bgrt_image_size;
> +struct acpi_table_bgrt bgrt_tab __ro_after_init;
> +size_t bgrt_image_size __ro_after_init;
>   
>   struct bmp_header {
>   	u16 id;
> @@ -89,3 +89,73 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
>   out:
>   	memset(bgrt, 0, sizeof(bgrt_tab));
>   }
> +
> +/**
> + * efi_bgrt_has_image() - Whether the firmware left a boot graphic
> + *
> + * Return: true if the BGRT points at a BMP image, which efi_bgrt_init()
> + * checked and reserved.
> + */
> +bool efi_bgrt_has_image(void)
> +{
> +	return bgrt_tab.image_address != 0;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_has_image);
> +
> +/**
> + * efi_bgrt_image_address() - Physical address of the boot graphic
> + *
> + * Return: the address of the BMP image, or 0 if there is none.
> + */
> +phys_addr_t efi_bgrt_image_address(void)
> +{
> +	return bgrt_tab.image_address;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_image_address);
> +
> +/**
> + * efi_bgrt_image_size() - Size of the boot graphic
> + *
> + * Return: the size of the BMP image, as its header gives it, or 0 if there
> + * is none.
> + */
> +size_t efi_bgrt_image_size(void)
> +{
> +	return bgrt_image_size;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_image_size);
> +
> +/**
> + * efi_bgrt_status() - Status field of the BGRT
> + *
> + * Return: the status byte. Bit 0 tells whether the image is still on
> + * screen and, since ACPI 6.2, bits 1-2 how far it has to be turned to show
> + * upright (ACPI_BGRT_ORIENTATION_OFFSET).
> + */
> +u8 efi_bgrt_status(void)
> +{
> +	return bgrt_tab.status;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_status);
> +
> +/**
> + * efi_bgrt_image_offset_x() - Horizontal position of the boot graphic
> + *
> + * Return: the X offset, in pixels, at which the firmware drew the image.
> + */
> +u32 efi_bgrt_image_offset_x(void)
> +{
> +	return bgrt_tab.image_offset_x;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_image_offset_x);
> +
> +/**
> + * efi_bgrt_image_offset_y() - Vertical position of the boot graphic
> + *
> + * Return: the Y offset, in pixels, at which the firmware drew the image.
> + */
> +u32 efi_bgrt_image_offset_y(void)
> +{
> +	return bgrt_tab.image_offset_y;
> +}
> +EXPORT_SYMBOL_GPL(efi_bgrt_image_offset_y);
> diff --git a/include/linux/efi-bgrt.h b/include/linux/efi-bgrt.h
> index e6cd51005633..1c287e5fd388 100644
> --- a/include/linux/efi-bgrt.h
> +++ b/include/linux/efi-bgrt.h
> @@ -9,7 +9,14 @@
>   void efi_bgrt_init(struct acpi_table_header *table);
>   int __init acpi_parse_bgrt(struct acpi_table_header *table);
>   
> -/* The BGRT data itself; only valid if bgrt_image != NULL. */
> +bool efi_bgrt_has_image(void);
> +phys_addr_t efi_bgrt_image_address(void);
> +size_t efi_bgrt_image_size(void);
> +u8 efi_bgrt_status(void);
> +u32 efi_bgrt_image_offset_x(void);
> +u32 efi_bgrt_image_offset_y(void);
> +
> +/* The BGRT data itself; only valid if efi_bgrt_has_image(). */
>   extern size_t bgrt_image_size;
>   extern struct acpi_table_bgrt bgrt_tab;
>   
> @@ -21,6 +28,36 @@ static inline int __init acpi_parse_bgrt(struct acpi_table_header *table)
>   	return 0;
>   }
>   
> +static inline bool efi_bgrt_has_image(void)
> +{
> +	return false;
> +}
> +
> +static inline phys_addr_t efi_bgrt_image_address(void)
> +{
> +	return 0;
> +}
> +
> +static inline size_t efi_bgrt_image_size(void)
> +{
> +	return 0;
> +}
> +
> +static inline u8 efi_bgrt_status(void)
> +{
> +	return 0;
> +}
> +
> +static inline u32 efi_bgrt_image_offset_x(void)
> +{
> +	return 0;
> +}
> +
> +static inline u32 efi_bgrt_image_offset_y(void)
> +{
> +	return 0;
> +}
> +
>   #endif /* !CONFIG_ACPI_BGRT */
>   
>   #endif /* _LINUX_EFI_BGRT_H */

-- 
--
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)