[PATCH v2 7/7] drm/panic: Add support for drawing a monochrome graphical logo

[Geert Uytterhoeven]

Hi Jocelyn,

On Mon, Jun 17, 2024 at 11:59 AM Jocelyn Falempe <jfalempe at redhat.com> 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 at glider.be>

> > --- 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 at linux-m68k.org

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