Hi Jani,
On Fri, 02 Oct 2026, Jani Nikula wrote:
> Data is not an interface.
>
> If these will be used more, perhaps it would be better to wrap access to
> them in functions? I think most of the time it will help with
> maintenance.
>
> And you can add stubs for CONFIG_ACPI_BGRT=n where they belong,
> i.e. include/linux/efi-bgrt.h instead of drm_splash.c like in patch 11
> of this series.
Agreed, and the same goes for your comment on patch 3. For v5 I would
replace the export with accessors in efi-bgrt.c, stubbed in
efi-bgrt.h for CONFIG_ACPI_BGRT=n:
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 splash client then drops its own wrappers and stubs, and the
"static inline" in drm_splash.c go as well.
drivers/acpi/bgrt.c is the only other user of bgrt_tab and
bgrt_image_size. I can move it to the accessors too, so that both
variables become static, if that is wanted; otherwise they stay
global for it and are no longer exported.
Ard, this changes the patch you acked, so I will not carry your
Acked-by unless the above works for you. Both variables become
__ro_after_init either way, as you asked.
Thanks,
Màxim
El vie, 2 oct 2026 a las 11:37, Jani Nikula
(<[email protected]>) escribió:
>
> On Thu, 01 Oct 2026, Màxim Pedraza Padilla <[email protected]> wrote:
> > The DRM splash client reads the boot graphics the firmware left behind
> > through bgrt_tab and bgrt_image_size. Built as part of drm_client_lib,
> > which can be a module, it then fails to link:
> >
> > 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!
> >
> > Export both. They are filled in once, early in boot, and only read
> > after that.
> >
> > Reported-by: Mario Limonciello <[email protected]>
> > Closes:
> > https://lore.kernel.org/dri-devel/[email protected]/
> > Assisted-by: Claude:claude-opus-5-5
> > Signed-off-by: Màxim Pedraza Padilla <[email protected]>
> > ---
> > drivers/firmware/efi/efi-bgrt.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/efi-bgrt.c
> > b/drivers/firmware/efi/efi-bgrt.c
> > index 1da451582812..746a8fb3a337 100644
> > --- a/drivers/firmware/efi/efi-bgrt.c
> > +++ b/drivers/firmware/efi/efi-bgrt.c
> > @@ -17,7 +17,10 @@
> > #include <linux/efi-bgrt.h>
> >
> > struct acpi_table_bgrt bgrt_tab;
> > +EXPORT_SYMBOL_GPL(bgrt_tab);
> > +
> > size_t bgrt_image_size;
> > +EXPORT_SYMBOL_GPL(bgrt_image_size);
>
> Data is not an interface.
>
> If these will be used more, perhaps it would be better to wrap access to
> them in functions? I think most of the time it will help with
> maintenance.
>
> And you can add stubs for CONFIG_ACPI_BGRT=n where they belong,
> i.e. include/linux/efi-bgrt.h instead of drm_splash.c like in patch 11
> of this series.
>
>
> BR,
> Jani.
>
>
> >
> > struct bmp_header {
> > u16 id;
>
> --
> Jani Nikula, Intel
