Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 06/09/2024 10:47, John Ogness wrote: On 2024-09-06, John Ogness wrote: Your device_lock()/device_unlock() callbacks probably just need to lock/unlock your mutex @drm_log_lock. Sorry, forgot to mention that the device_lock() callback must also disable migration. Since you are using a mutex, you will need to manually do that as well... mutex_lock(&drm_log_lock); migrate_disable(); Thanks a lot, I've done most of the conversion, and it's already working great. I will send a new series next week. Best regards, -- Jocelyn John
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 2024-09-06, John Ogness wrote: > Your device_lock()/device_unlock() callbacks probably just need to > lock/unlock your mutex @drm_log_lock. Sorry, forgot to mention that the device_lock() callback must also disable migration. Since you are using a mutex, you will need to manually do that as well... mutex_lock(&drm_log_lock); migrate_disable(); John
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 2024-09-06, Jocelyn Falempe wrote: >> It would be great to see a version of drm_log that only implements >> write_thread() and does not do any of its own buffering with workqueue >> and also does not need to track multiple graphic loggers at the same >> time. > > Thanks for the head-up. > I will rebase it on top of Linux-next, and adapt to the new > write_thread() API, it should be much simpler. For drm_log, your write_atomic() callback should be NULL. You only need to implement the write_thead(), device_lock(), and device_unlock() callbacks. Your device_lock()/device_unlock() callbacks probably just need to lock/unlock your mutex @drm_log_lock. device_lock() is already called when the write_thread() callback is called. So your write_thread callback really only needs to call your drm_log_draw_kmsg_record(&dclient->scanout, wctxt->outbuf, wctxt->len). Please let me know if you run into any issues. We probably should write a document "HOWTO write an NBCON console driver". John Ogness
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 05/09/2024 17:22, John Ogness wrote: On 2024-08-01, John Ogness wrote: On 2024-08-01, Jocelyn Falempe wrote: I think I can also register one console for each drm driver, which will simplify drm_log even further. (currently it would mean having a circular buffer and work function for each driver which is a bit too much). Indeed. Do you know if there is a chance to have write_thread() in 6.12 or 6.13 ? FYI: The full NBCON API (with write_thread()) is now available in linux-next. It would be great to see a version of drm_log that only implements write_thread() and does not do any of its own buffering with workqueue and also does not need to track multiple graphic loggers at the same time. Thanks for the head-up. I will rebase it on top of Linux-next, and adapt to the new write_thread() API, it should be much simpler. I also need to adapt to the drm client rework (https://patchwork.freedesktop.org/series/137391/) so that it will be possible to choose on kernel command line between drm_log and fbdev emulation/fbcon. Best regards, -- Jocelyn John Ogness
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 2024-08-01, John Ogness wrote: > On 2024-08-01, Jocelyn Falempe wrote: >> I think I can also register one console for each drm driver, which >> will simplify drm_log even further. (currently it would mean having a >> circular buffer and work function for each driver which is a bit too >> much). > > Indeed. > >> Do you know if there is a chance to have write_thread() in 6.12 or >> 6.13 ? FYI: The full NBCON API (with write_thread()) is now available in linux-next. It would be great to see a version of drm_log that only implements write_thread() and does not do any of its own buffering with workqueue and also does not need to track multiple graphic loggers at the same time. John Ogness
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 2024-08-01, Jocelyn Falempe wrote: > I think I can also register one console for each drm driver, which > will simplify drm_log even further. (currently it would mean having a > circular buffer and work function for each driver which is a bit too > much). Indeed. > Do you know if there is a chance to have write_thread() in 6.12 or > 6.13 ? Sorry, I have no crystal ball. Petr Mladek and I are working really hard to get things in shape for mainline. I do think there is a chance for the 6.12 merge window. But it would also need to get past Linus. Our recent atomic console efforts were rejected [0] for the 6.11 merge window. We are working to get that series in shape for 6.12 in parallel. John [0] https://lore.kernel.org/lkml/CAHk-=whU_woFnFN-3Jv2hNCmwLg_fkrT42AWwxm-=ha5bmn...@mail.gmail.com
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 01/08/2024 12:51, John Ogness wrote:
On 2024-08-01, Jocelyn Falempe wrote:
* It uses a circular buffer so the console->write() callback is very
quick, and will never stall.
* Drawing is done asynchronously using a workqueue.
For CON_NBCON, neither of the above points are necessary. You can draw
directly from the write_thread() callback. See below:
+static bool drm_log_work_draw(void)
+{
+ unsigned int len;
+ char buf[512];
+
+ len = drm_log_buf_read(buf, sizeof(buf));
+ if (len)
+ drm_log_draw_all(buf, len);
+ return len != 0;
+}
For CON_NBCON, this is essentially your write_thread() callback:
void drm_log_write_thread(struct console *con,
struct nbcon_write_context *wctxt)
{
drm_log_draw_all(wctxt->outbuf, wctxt->len);
}
You cannot implement a write_atomic() callback because the console must
be able to print directly in NMI context and must not defer. But
write_atomic() is optional, so you should be fine there.
Disclaimer: Only in PREEMPT_RT patchset at the moment.
Thanks, so that means the circular buffer and workqueue are only
necessary until write_thread() is merged in mainline. It will be a nice
simplification.
I think I can also register one console for each drm driver, which will
simplify drm_log even further. (currently it would mean having a
circular buffer and work function for each driver which is a bit too much).
Do you know if there is a chance to have write_thread() in 6.12 or 6.13 ?
John Ogness >
Best regards
--
Jocelyn
Re: [RFC PATCH 3/3] drm/log: Introduce a new boot logger to draw the kmsg on the screen
On 2024-08-01, Jocelyn Falempe wrote:
> * It uses a circular buffer so the console->write() callback is very
>quick, and will never stall.
> * Drawing is done asynchronously using a workqueue.
For CON_NBCON, neither of the above points are necessary. You can draw
directly from the write_thread() callback. See below:
> +static bool drm_log_work_draw(void)
> +{
> + unsigned int len;
> + char buf[512];
> +
> + len = drm_log_buf_read(buf, sizeof(buf));
> + if (len)
> + drm_log_draw_all(buf, len);
> + return len != 0;
> +}
For CON_NBCON, this is essentially your write_thread() callback:
void drm_log_write_thread(struct console *con,
struct nbcon_write_context *wctxt)
{
drm_log_draw_all(wctxt->outbuf, wctxt->len);
}
You cannot implement a write_atomic() callback because the console must
be able to print directly in NMI context and must not defer. But
write_atomic() is optional, so you should be fine there.
Disclaimer: Only in PREEMPT_RT patchset at the moment.
John Ogness
