Message-ID: <34979a3b-e968-4ca6-867e-a90e6cb35b83@suse.de>
Date: Tue, 3 Dec 2024 10:35:45 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Jocelyn Falempe <jfalempe@...hat.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
 Daniel Vetter <daniel@...ll.ch>, John Ogness <john.ogness@...utronix.de>,
 Javier Martinez Canillas <javierm@...hat.com>,
 "Guilherme G . Piccoli" <gpiccoli@...lia.com>,
 bluescreen_avenger@...izon.net, Caleb Connolly <caleb.connolly@...aro.org>,
 Petr Mladek <pmladek@...e.com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 2/6] drm/log: Introduce a new boot logger to draw the
 kmsg on the screen

Hi


Am 03.12.24 um 10:22 schrieb Jocelyn Falempe:
> On 03/12/2024 09:48, Thomas Zimmermann wrote:
>> Hi
>>
>>
>> Am 15.11.24 um 14:40 schrieb Jocelyn Falempe:
>>> drm_log is a simple logger that uses the drm_client API to print the
>>> kmsg boot log on the screen. This is not a full replacement to fbcon,
>>> as it will only print the kmsg. It will never handle user input, or a
>>> terminal because this is better done in userspace.
>>>
>>> Design decisions:
>>>   * It uses the drm_client API, so it should work on all drm drivers
>>>     from the start.
>>>   * It doesn't scroll the message, that way it doesn't need to redraw
>>>     the whole screen for each new message.
>>>     It also means it doesn't have to keep drawn messages in memory, to
>>>     redraw them when scrolling.
>>>   * It uses the new non-blocking console API, so it should work well
>>>     with PREEMPT_RT.
>>>
>>> This patch also adds a Kconfig menu to select the drm client to use.
>>> It can be overwritten on the kernel command line with:
>>> drm_client_lib.default=log or drm_client_lib.default=fbdev
>>>
>>> Signed-off-by: Jocelyn Falempe <jfalempe@...hat.com>
>>> Reviewed-by: John Ogness <john.ogness@...utronix.de> # console API
>>> ---
>>>
>>> v2:
>>>   * Use vmap_local() api, with that change, I've tested it 
>>> successfully on simpledrm, virtio-gpu, amdgpu, and nouveau.
>>>   * Stop drawing when the drm_master is taken. This avoid wasting 
>>> CPU cycle if the buffer is not visible.
>>>   * Use deferred probe. Only do the probe the first time there is a 
>>> log to draw. With this, if you boot with quiet, drm_log won't do any 
>>> modeset.
>>>   * Add color support for the timestamp prefix, like what dmesg does.
>>>   * Add build dependency on  disabling the fbdev emulation, as they 
>>> are both drm_client, and there is no way to choose which one gets 
>>> the focus.
>>>
>>> v3:
>>>   * Remove the work thread and circular buffer, and use the new 
>>> write_thread() console API.
>>>   * Register a console for each drm driver.
>>>
>>> v4:
>>>   * Can be built as a module, even if that's not really useful.
>>>   * Rebased on top of "drm: Introduce DRM client library" series 
>>> from Thomas Zimmermann.
>>>   * Add a Kconfig menu to choose between drm client.
>>> v5:
>>>   * Build drm_log in drm_client_lib module, to avoid circular 
>>> dependency.
>>>
>>> v8:
>>>   * Rebased after drm client moved to drivers/gpu/drm/clients/
>>>   * Rename DRM_LOG to DRM_CLIENT_LOG (Thomas Zimmermann)
>>>   * Add an info message if no clients are initialized in 
>>> drm_client_setup()
>>>
>>>   drivers/gpu/drm/clients/Kconfig               |  48 +++
>>>   drivers/gpu/drm/clients/Makefile              |   1 +
>>>   drivers/gpu/drm/clients/drm_client_internal.h |   6 +
>>>   drivers/gpu/drm/clients/drm_client_setup.c    |  29 +-
>>>   drivers/gpu/drm/clients/drm_log.c             | 370 
>>> ++++++++++++++++++
>>>   5 files changed, 450 insertions(+), 4 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/clients/drm_log.c
>>>
>>> diff --git a/drivers/gpu/drm/clients/Kconfig b/drivers/gpu/drm/ 
>>> clients/Kconfig
>>> index 01ad3b0001303..7cbdb1b881b45 100644
>>> --- a/drivers/gpu/drm/clients/Kconfig
>>> +++ b/drivers/gpu/drm/clients/Kconfig
>>> @@ -13,6 +13,7 @@ config DRM_CLIENT_SELECTION
>>>       tristate
>>>       depends on DRM
>>>       select DRM_CLIENT_LIB if DRM_FBDEV_EMULATION
>>> +    select DRM_CLIENT_LIB if DRM_CLIENT_LOG
>>
>> 'if <>' sorted alphabetically please.
>
> Yes, sure.
>>
>>>       help
>>>         Drivers that support in-kernel DRM clients have to select this
>>>         option.
>>> @@ -70,4 +71,51 @@ config DRM_FBDEV_LEAK_PHYS_SMEM
>>>         If in doubt, say "N" or spread the word to your closed source
>>>         library vendor.
>>> +config DRM_CLIENT_LOG
>>> +    bool "Print the kernel boot message on the screen"
>>> +    depends on DRM_CLIENT_SELECTION
>>> +    select DRM_DRAW
>>> +    select DRM_CLIENT
>>> +    select DRM_CLIENT_SETUP
>>
>> Select sorted alphabetically please.
>
> Agreed too
>>
>>> +    help
>>> +      This enable a drm logger, that will print the kernel messages 
>>> to the
>>> +      screen until the userspace is ready to take over.
>>> +
>>> +      If you only need logs, but no terminal, or if you prefer 
>>> userspace
>>> +      terminal, say "Y".
>>> +
>>> +choice
>>> +    prompt "Default DRM Client"
>>> +    depends on DRM_CLIENT_SELECTION
>>> +    default DRM_CLIENT_DEFAULT_FBDEV
>>> +    help
>>> +      Selects the default drm client.
>>> +
>>> +      The selection made here can be overridden by using the kernel
>>> +      command line 'drm_client_lib.default=fbdev' option.
>>> +
>>> +config DRM_CLIENT_DEFAULT_FBDEV
>>> +    bool "fbdev"
>>> +    depends on DRM_FBDEV_EMULATION
>>> +    help
>>> +      Use fbdev emulation as default drm client. This is needed to 
>>> have
>>> +      fbcon on top of a drm driver.
>>> +
>>> +config DRM_CLIENT_DEFAULT_LOG
>>> +    bool "log"
>>> +    depends on DRM_CLIENT_LOG
>>> +    help
>>> +      Use drm log as default drm client. This will display boot 
>>> logs on the
>>> +      screen, but doesn't implement a full terminal. For that you 
>>> will need
>>> +      a userspace terminal using drm/kms.
>>> +
>>> +endchoice
>>> +
>>> +config DRM_CLIENT_DEFAULT
>>> +       string
>>> +       depends on DRM_CLIENT
>>> +       default "fbdev" if DRM_CLIENT_DEFAULT_FBDEV
>>> +       default "log" if DRM_CLIENT_DEFAULT_LOG
>>> +       default ""
>>> +
>>>   endmenu
>>> diff --git a/drivers/gpu/drm/clients/Makefile b/drivers/gpu/drm/ 
>>> clients/Makefile
>>> index 1d004ec92e1ea..9fb776590efe7 100644
>>> --- a/drivers/gpu/drm/clients/Makefile
>>> +++ b/drivers/gpu/drm/clients/Makefile
>>> @@ -2,4 +2,5 @@
>>>   drm_client_lib-y := drm_client_setup.o
>>>   drm_client_lib-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fbdev_client.o
>>> +drm_client_lib-$(CONFIG_DRM_CLIENT_LOG) += drm_log.o
>>>   obj-$(CONFIG_DRM_CLIENT_LIB) += drm_client_lib.o
>>> diff --git a/drivers/gpu/drm/clients/drm_client_internal.h 
>>> b/drivers/ gpu/drm/clients/drm_client_internal.h
>>> index 23258934956aa..6dc078bf6503b 100644
>>> --- a/drivers/gpu/drm/clients/drm_client_internal.h
>>> +++ b/drivers/gpu/drm/clients/drm_client_internal.h
>>> @@ -16,4 +16,10 @@ static inline int drm_fbdev_client_setup(struct 
>>> drm_device *dev,
>>>   }
>>>   #endif
>>> +#ifdef CONFIG_DRM_CLIENT_LOG
>>> +void drm_log_register(struct drm_device *dev);
>>> +#else
>>> +static inline void drm_log_register(struct drm_device *dev) {}
>>> +#endif
>>> +
>>>   #endif
>>> diff --git a/drivers/gpu/drm/clients/drm_client_setup.c 
>>> b/drivers/gpu/ drm/clients/drm_client_setup.c
>>> index 4b211a4812b5b..92a7b3c180d19 100644
>>> --- a/drivers/gpu/drm/clients/drm_client_setup.c
>>> +++ b/drivers/gpu/drm/clients/drm_client_setup.c
>>> @@ -7,6 +7,12 @@
>>>   #include "drm_client_internal.h"
>>> +static char drm_client_default[16] = CONFIG_DRM_CLIENT_DEFAULT;
>>> +module_param_string(client, drm_client_default, 
>>> sizeof(drm_client_default), 0444);
>>> +MODULE_PARM_DESC(client,
>>> +         "Choose which drm client to start, default is"
>>> +         CONFIG_DRM_CLIENT_DEFAULT "]");
>>
>> CONFIG_DRM_CLIENT_DEFAULT is good for the config option. But using 
>> 'drm_client_lib.client=log' seems imprecise to me. The module 
>> parameter itself should maybe called something like 'setup' or 'active'.
>
> Ok, I can rename it to drm_client_lib.active=log
>
>>
>> (As a sidenote, if we ever have more clients, we could then modify 
>> the parameter to be separated by comma. The user would then be able 
>> to specify a set of clients from the ones available.)
>
> I'm a bit confused about this.
> The goal is to allow to build any number of drm clients in the kernel, 
> but only one can run. There are no way to switch from one client to 
> another at runtime. So what would be the purpose of specifying 
> multiple clients?

The user could then specify the order of trying them, like in 
'active=log, fdev, bootsplash,foo, bar'. And the kernel would try them 
in this precise order. Only make sense if there are more than two 
clients available of course.

>
>>
>>
>>> +
>>>   /**
>>>    * drm_client_setup() - Setup in-kernel DRM clients
>>>    * @dev: DRM device
>>> @@ -25,11 +31,26 @@
>>>    */
>>>   void drm_client_setup(struct drm_device *dev, const struct 
>>> drm_format_info *format)
>>>   {
>>> -    int ret;
>>> -    ret = drm_fbdev_client_setup(dev, format);
>>> -    if (ret)
>>> -        drm_warn(dev, "Failed to set up DRM client; error %d\n", ret);
>>> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>>> +    if (!strcmp(drm_client_default, "fbdev")) {
>>> +        int ret;
>>> +
>>> +        ret = drm_fbdev_client_setup(dev, format);
>>> +        if (ret)
>>> +            drm_warn(dev, "Failed to set up DRM client; error 
>>> %d\n", ret);
>>> +        return;
>>> +    }
>>> +#endif
>>> +
>>> +#ifdef CONFIG_DRM_CLIENT_LOG
>>> +    if (!strcmp(drm_client_default, "log")) {
>>> +        drm_log_register(dev);
>>> +        return;
>>> +    }
>>> +#endif
>>> +
>>> +    drm_info(dev, "No drm client registered\n");
>>
>> Let's remain silent here. If anything, debug please.
>
> If you get there, it's either a typo, or a misconfiguration, but yes I 
> can move it as debug.

It should be a warning then. Something like

   if (strcmp(drm_client_log, ""))
       drm_warn("Unknown DRM client %s\n", drm_client_log)

>
>>
>>>   }
>>>   EXPORT_SYMBOL(drm_client_setup);
>>> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/ 
>>> clients/drm_log.c
>>> new file mode 100644
>>> index 0000000000000..cb793a348f1f9
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/clients/drm_log.c
>>> @@ -0,0 +1,370 @@
>>> +// SPDX-License-Identifier: GPL-2.0 or MIT
>>> +/*
>>> + * Copyright (c) 2024 Red Hat.
>>> + * Author: Jocelyn Falempe <jfalempe@...hat.com>
>>> + */
>>> +
>>> +#include <linux/console.h>
>>> +#include <linux/font.h>
>>> +#include <linux/init.h>
>>> +#include <linux/iosys-map.h>
>>> +#include <linux/module.h>
>>> +#include <linux/types.h>
>>> +
>>> +#include <drm/drm_client.h>
>>> +#include <drm/drm_drv.h>
>>> +#include <drm/drm_fourcc.h>
>>> +#include <drm/drm_framebuffer.h>
>>> +#include <drm/drm_print.h>
>>> +
>>> +#include "drm_client_internal.h"
>>> +#include "../drm_draw.h"
>>> +
>>> +MODULE_AUTHOR("Jocelyn Falempe");
>>> +MODULE_DESCRIPTION("DRM boot logger");
>>> +MODULE_LICENSE("GPL");
>>> +
>>> +/**
>>> + * DOC: overview
>>> + *
>>> + * This is a simple graphic logger, to print the kernel message on 
>>> screen, until
>>> + * a userspace application is able to take over.
>>> + * It is only for debugging purpose.
>>> + */
>>> +
>>> +struct drm_log_scanout {
>>> +    struct drm_client_buffer *buffer;
>>> +    const struct font_desc *font;
>>> +    u32 rows;
>>> +    u32 columns;
>>> +    u32 line;
>>> +    u32 format;
>>> +    u32 px_width;
>>> +    u32 front_color;
>>> +};
>>> +
>>> +struct drm_log {
>>> +    struct mutex lock;
>>> +    struct drm_client_dev client;
>>> +    struct console con;
>>> +    bool probed;
>>> +    u32 n_scanout;
>>> +    struct drm_log_scanout *scanout;
>>> +};
>>> +
>>> +static struct drm_log *client_to_drm_log(struct drm_client_dev 
>>> *client)
>>> +{
>>> +    return container_of(client, struct drm_log, client);
>>> +}
>>> +
>>> +static struct drm_log *console_to_drm_log(struct console *con)
>>> +{
>>> +    return container_of(con, struct drm_log, con);
>>> +}
>>> +
>>> +static void drm_log_blit(struct iosys_map *dst, unsigned int 
>>> dst_pitch,
>>> +             const u8 *src, unsigned int src_pitch,
>>> +             u32 height, u32 width, u32 scale, u32 px_width, u32 
>>> color)
>>> +{
>>> +    switch (px_width) {
>>> +    case 2:
>>> +        drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, 
>>> width, scale, color);
>>> +        break;
>>> +    case 3:
>>> +        drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, 
>>> width, scale, color);
>>> +        break;
>>> +    case 4:
>>> +        drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, 
>>> width, scale, color);
>>> +        break;
>>> +    default:
>>> +        WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
>>> +    }
>>> +}
>>> +
>>> +static void drm_log_clear_line(struct drm_log_scanout *scanout, u32 
>>> line)
>>> +{
>>> +    struct drm_framebuffer *fb = scanout->buffer->fb;
>>> +    unsigned long height = scanout->font->height;
>>> +    struct iosys_map map;
>>> +    struct drm_rect r = DRM_RECT_INIT(0, line * height, fb->width, 
>>> height);
>>> +
>>> +    if (drm_client_buffer_vmap_local(scanout->buffer, &map))
>>> +        return;
>>> +    iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb- 
>>> >pitches[0]);
>>> +    drm_client_buffer_vunmap_local(scanout->buffer);
>>> +    drm_client_framebuffer_flush(scanout->buffer, &r);
>>> +}
>>> +
>>> +static void drm_log_draw_line(struct drm_log_scanout *scanout, 
>>> const char *s,
>>> +                  unsigned int len)
>>> +{
>>> +    struct drm_framebuffer *fb = scanout->buffer->fb;
>>> +    struct iosys_map map;
>>> +    const struct font_desc *font = scanout->font;
>>> +    size_t font_pitch = DIV_ROUND_UP(font->width, 8);
>>> +    const u8 *src;
>>> +    u32 px_width = fb->format->cpp[0];
>>> +    struct drm_rect r = DRM_RECT_INIT(0, scanout->line * font->height,
>>> +                      fb->width, (scanout->line + 1) * font->height);
>>> +    u32 i;
>>> +
>>> +    if (drm_client_buffer_vmap_local(scanout->buffer, &map))
>>> +        return;
>>> +
>>> +    iosys_map_incr(&map, r.y1 * fb->pitches[0]);
>>> +    for (i = 0; i < len && i < scanout->columns; i++) {
>>> +        src = drm_draw_get_char_bitmap(font, s[i], font_pitch);
>>> +        drm_log_blit(&map, fb->pitches[0], src, font_pitch, font- 
>>> >height, font->width,
>>> +                 1, px_width, scanout->front_color);
>>> +        iosys_map_incr(&map, font->width * px_width);
>>> +    }
>>> +
>>> +    scanout->line++;
>>> +    if (scanout->line >= scanout->rows)
>>> +        scanout->line = 0;
>>> +    drm_client_buffer_vunmap_local(scanout->buffer);
>>> +    drm_client_framebuffer_flush(scanout->buffer, &r);
>>> +}
>>> +
>>> +static void drm_log_draw_new_line(struct drm_log_scanout *scanout,
>>> +                  const char *s, unsigned int len)
>>> +{
>>> +    if (scanout->line == 0) {
>>> +        drm_log_clear_line(scanout, 0);
>>> +        drm_log_clear_line(scanout, 1);
>>> +        drm_log_clear_line(scanout, 2);
>>> +    } else if (scanout->line + 2 < scanout->rows)
>>> +        drm_log_clear_line(scanout, scanout->line + 2);
>>> +
>>> +    drm_log_draw_line(scanout, s, len);
>>> +}
>>> +
>>> +static void drm_log_draw_kmsg_record(struct drm_log_scanout *scanout,
>>> +                     const char *s, unsigned int len)
>>> +{
>>> +    /* do not print the ending \n character */
>>> +    if (s[len - 1] == '\n')
>>> +        len--;
>>> +
>>> +    while (len > scanout->columns) {
>>> +        drm_log_draw_new_line(scanout, s, scanout->columns);
>>> +        s += scanout->columns;
>>> +        len -= scanout->columns;
>>> +    }
>>> +    if (len)
>>> +        drm_log_draw_new_line(scanout, s, len);
>>> +}
>>> +
>>> +static u32 drm_log_find_usable_format(struct drm_plane *plane)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < plane->format_count; i++)
>>> +        if (drm_draw_color_from_xrgb8888(0xffffff, plane- 
>>> >format_types[i]) != 0)
>>> +            return plane->format_types[i];
>>> +    return DRM_FORMAT_INVALID;
>>> +}
>>> +
>>> +static int drm_log_setup_modeset(struct drm_client_dev *client,
>>> +                 struct drm_mode_set *mode_set,
>>> +                 struct drm_log_scanout *scanout)
>>> +{
>>> +    struct drm_crtc *crtc = mode_set->crtc;
>>> +    u32 width = mode_set->mode->hdisplay;
>>> +    u32 height = mode_set->mode->vdisplay;
>>> +    u32 format;
>>> +
>>> +    scanout->font = get_default_font(width, height, NULL, NULL);
>>> +    if (!scanout->font)
>>> +        return -ENOENT;
>>> +
>>> +    format = drm_log_find_usable_format(crtc->primary);
>>> +    if (format == DRM_FORMAT_INVALID)
>>> +        return -EINVAL;
>>> +
>>> +    scanout->buffer = drm_client_framebuffer_create(client, width, 
>>> height, format);
>>> +    if (IS_ERR(scanout->buffer)) {
>>> +        drm_warn(client->dev, "drm_log can't create framebuffer %d 
>>> %d %p4cc\n",
>>> +             width, height, &format);
>>> +        return -ENOMEM;
>>> +    }
>>> +    mode_set->fb = scanout->buffer->fb;
>>> +    scanout->rows = height / scanout->font->height;
>>> +    scanout->columns = width / scanout->font->width;
>>> +    scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, 
>>> format);
>>> +    return 0;
>>> +}
>>> +
>>> +static int drm_log_count_modeset(struct drm_client_dev *client)
>>> +{
>>> +    struct drm_mode_set *mode_set;
>>> +    int count = 0;
>>> +
>>> +    mutex_lock(&client->modeset_mutex);
>>> +    drm_client_for_each_modeset(mode_set, client)
>>> +        count++;
>>> +    mutex_unlock(&client->modeset_mutex);
>>> +    return count;
>>> +}
>>> +
>>> +static void drm_log_init_client(struct drm_log *dlog)
>>> +{
>>> +    struct drm_client_dev *client = &dlog->client;
>>> +    struct drm_mode_set *mode_set;
>>> +    int i, max_modeset;
>>> +    int n_modeset = 0;
>>> +
>>> +    dlog->probed = true;
>>> +
>>> +    if (drm_client_modeset_probe(client, 0, 0))
>>> +        return;
>>> +
>>> +    max_modeset = drm_log_count_modeset(client);
>>> +    if (!max_modeset)
>>> +        return;
>>> +
>>> +    dlog->scanout = kcalloc(max_modeset, sizeof(*dlog->scanout), 
>>> GFP_KERNEL);
>>> +    if (!dlog->scanout)
>>> +        return;
>>> +
>>> +    mutex_lock(&client->modeset_mutex);
>>> +    drm_client_for_each_modeset(mode_set, client) {
>>> +        if (!mode_set->mode)
>>> +            continue;
>>> +        if (drm_log_setup_modeset(client, mode_set, &dlog- 
>>> >scanout[n_modeset]))
>>> +            continue;
>>> +        n_modeset++;
>>> +    }
>>> +    mutex_unlock(&client->modeset_mutex);
>>> +    if (n_modeset == 0)
>>> +        goto err_nomodeset;
>>> +
>>> +    if (drm_client_modeset_commit(client))
>>> +        goto err_failed_commit;
>>> +
>>> +    dlog->n_scanout = n_modeset;
>>> +    return;
>>> +
>>> +err_failed_commit:
>>> +    for (i = 0; i < n_modeset; i++)
>>> + drm_client_framebuffer_delete(dlog->scanout[i].buffer);
>>> +
>>> +err_nomodeset:
>>> +    kfree(dlog->scanout);
>>> +    dlog->scanout = NULL;
>>> +}
>>> +
>>> +static void drm_log_free_scanout(struct drm_client_dev *client)
>>> +{
>>> +    struct drm_log *dlog = client_to_drm_log(client);
>>> +    int i;
>>> +
>>> +    if (dlog->n_scanout) {
>>> +        for (i = 0; i < dlog->n_scanout; i++)
>>> + drm_client_framebuffer_delete(dlog->scanout[i].buffer);
>>> +        dlog->n_scanout = 0;
>>> +        kfree(dlog->scanout);
>>> +        dlog->scanout = NULL;
>>> +    }
>>> +}
>>> +
>>> +static void drm_log_client_unregister(struct drm_client_dev *client)
>>> +{
>>> +    struct drm_log *dlog = client_to_drm_log(client);
>>> +    struct drm_device *dev = client->dev;
>>> +
>>> +    unregister_console(&dlog->con);
>>> +
>>> +    mutex_lock(&dlog->lock);
>>> +    drm_log_free_scanout(client);
>>> +    drm_client_release(client);
>>> +    mutex_unlock(&dlog->lock);
>>> +    kfree(dlog);
>>> +    drm_info(dev, "Unregistered with drm log\n");
>>
>> We don't do such messages anywhere. So if anything, debug output here.
>
> Agreed.
>>
>>> +}
>>> +
>>> +static int drm_log_client_hotplug(struct drm_client_dev *client)
>>> +{
>>> +    struct drm_log *dlog = client_to_drm_log(client);
>>> +
>>> +    mutex_lock(&dlog->lock);
>>> +    drm_log_free_scanout(client);
>>> +    dlog->probed = false;
>>> +    mutex_unlock(&dlog->lock);
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct drm_client_funcs drm_log_client_funcs = {
>>> +    .owner        = THIS_MODULE,
>>> +    .unregister    = drm_log_client_unregister,
>>> +    .hotplug    = drm_log_client_hotplug,
>>> +};
>>> +
>>> +static void drm_log_write_thread(struct console *con, struct 
>>> nbcon_write_context *wctxt)
>>> +{
>>> +    struct drm_log *dlog = console_to_drm_log(con);
>>> +    int i;
>>> +
>>> +    if (!dlog->probed)
>>> +        drm_log_init_client(dlog);
>>> +
>>> +    for (i = 0; i < dlog->n_scanout; i++)
>>> +        drm_log_draw_kmsg_record(&dlog->scanout[i], wctxt->outbuf, 
>>> wctxt->len);
>>> +}
>>> +
>>> +static void drm_log_lock(struct console *con, unsigned long *flags)
>>> +{
>>> +    struct drm_log *dlog = console_to_drm_log(con);
>>> +
>>> +    mutex_lock(&dlog->lock);
>>> +    migrate_disable();
>>> +}
>>> +
>>> +static void drm_log_unlock(struct console *con, unsigned long flags)
>>> +{
>>> +    struct drm_log *dlog = console_to_drm_log(con);
>>> +
>>> +    migrate_enable();
>>> +    mutex_unlock(&dlog->lock);
>>> +}
>>> +
>>> +static void drm_log_register_console(struct console *con)
>>> +{
>>> +    strscpy(con->name, "drm_log");
>>> +    con->write_thread = drm_log_write_thread;
>>> +    con->device_lock = drm_log_lock;
>>> +    con->device_unlock = drm_log_unlock;
>>> +    con->flags = CON_PRINTBUFFER | CON_NBCON;
>>> +    con->index = -1;
>>> +
>>> +    register_console(con);
>>> +}
>>> +
>>> +/**
>>> + * drm_log_register() - Register a drm device to drm_log
>>> + * @dev: the drm device to register.
>>> + */
>>> +void drm_log_register(struct drm_device *dev)
>>> +{
>>> +    struct drm_log *new;
>>> +
>>> +    new = kzalloc(sizeof(*new), GFP_KERNEL);
>>> +    if (!new)
>>> +        goto err_warn;
>>> +
>>> +    mutex_init(&new->lock);
>>> +    if (drm_client_init(dev, &new->client, "drm_log", 
>>> &drm_log_client_funcs))
>>> +        goto err_free;
>>> +
>>> +    drm_client_register(&new->client);
>>> +
>>> +    drm_log_register_console(&new->con);
>>> +
>>> +    drm_info(dev, "Registered with drm log as %s\n", new->con.name);
>>
>> Debug please. Don't spam the kernel log with status messages.
>
> Yes I will move that to debug, but it's still much less verbose than 
> the current situation with fbdev/fbcon.
>
> [    1.631473] fbcon: Deferring console take-over
> [    1.631475] simple-framebuffer simple-framebuffer.0: [drm] fb0: 
> simpledrmdrmfb frame buffer device
> [    7.058770] fbcon: i915drmfb (fb0) is primary device
> [    7.058772] fbcon: Deferring console take-over
> [    7.058774] i915 0000:00:02.0: [drm] fb0: i915drmfb frame buffer 
> device

I don't like this either. Such output distracts from the important 
messages. But it's been there for ages so I don't dare to remove it.

Best regards
Thomas

>
>>
>> Best regards
>> Thomas
>>
>>> +    return;
>>> +
>>> +err_free:
>>> +    kfree(new);
>>> +err_warn:
>>> +    drm_warn(dev, "Failed to register with drm log\n");
>>> +}
>>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Powered by blists - more mailing lists