Hi Jocelyn,
On Mon, Jun 17, 2024 at 11:59 AM Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
> On 13/06/2024 21:18, Geert Uytterhoeven wrote:
> > Re-use the existing support for boot-up logos to draw a monochrome
> > graphical logo in the DRM panic handler. When no suitable graphical
> > logo is available, the code falls back to the ASCII art penguin logo.
> >
> > Note that all graphical boot-up logos are freed during late kernel
> > initialization, hence a copy must be made for later use.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > --- a/drivers/gpu/drm/drm_panic.c
> > +++ b/drivers/gpu/drm/drm_panic.c
> > PANIC_LINE(" \\___)=(___/"),
> > };
> >
> > +#ifdef CONFIG_LOGO
> > +static const struct linux_logo *logo_mono;
> > +
> > +static int drm_panic_setup_logo(void)
> > +{
> > + const struct linux_logo *logo = fb_find_logo(1);
> > + const unsigned char *logo_data;
> > + struct linux_logo *logo_dup;
> > +
> > + if (!logo || logo->type != LINUX_LOGO_MONO)
> > + return 0;
> > +
> > + /* The logo is __init, so we must make a copy for later use */
> > + logo_data = kmemdup(logo->data,
> > + size_mul(DIV_ROUND_UP(logo->width, BITS_PER_BYTE), logo->height),
> > + GFP_KERNEL);
> > + if (!logo_data)
> > + return -ENOMEM;
> > +
> > + logo_dup = kmemdup(logo, sizeof(*logo), GFP_KERNEL);
> > + if (!logo_dup) {
> > + kfree(logo_data);
> > + return -ENOMEM;
> > + }
> > +
> > + logo_dup->data = logo_data;
> > + logo_mono = logo_dup;
> > +
> > + return 0;
> > +}
> > +
> > +device_initcall(drm_panic_setup_logo);
> > +#else
> > +#define logo_mono ((const struct linux_logo *)NULL)
> > +#endif
>
> I didn't notice that the first time, but the core drm can be built as a
> module.
> That means this will leak memory each time the module is removed.
While I hadn't considered a modular DRM core, there is no memory leak:
after the logos are freed (from late_initcall_sync()), fb_find_logo()
returns NULL. This does mean there won't be a graphical logo on panic,
though.
> But to solve the circular dependency between drm_kms_helper and
> drm_panic, one solution is to depends on drm core to be built-in.
> In this case there won't be a leak.
> So depending on how we solve the circular dependency, it can be acceptable.
So far there is no reason to select DRM_KMS_HELPER, right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
