Re: [PATCH v5 5/6] drm/log: Implement suspend/resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/11/2024 17:00, Petr Mladek wrote:
On Fri 2024-10-25 11:46:16, Jocelyn Falempe wrote:
On 25/10/2024 01:11, Jocelyn Falempe wrote:
On 24/10/2024 16:34, Petr Mladek wrote:
On Wed 2024-10-23 14:00:13, Jocelyn Falempe wrote:
The console is already suspended in printk.c.

Does this mean that drm_log_client_suspend() is called
after suspend_console(), please?

To be honest, I wasn't able to tell which one is called first, and if
the order is enforced (I didn't check if drivers can be suspended in
parallel, or if it's all sequential)..

I then checked if it's possible to suspend the console, but didn't found
an easy API to do so, so I went with this lazy patch, just ensuring
we're not writing to a suspended graphic driver.

I've run some tests on my hardware, and the console is suspended before the
graphic driver:

[   56.409604] printk: Suspending console(s) (use no_console_suspend to
debug)
[   56.411430] serial 00:05: disabled
[   56.421877] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   56.421954] sd 4:0:0:0: [sdb] Synchronizing SCSI cache
[   56.422545] ata1.00: Entering standby power mode
[   56.422793] DRM log suspend

But because there is the "no_console_suspend" parameter, and we should make
sure to not draw when the graphic driver is suspended, I think this patch is
needed, and good enough.
I will just rephrase the commit message, to make it clear, that some message
won't be drawn, only if "no_console_suspend" is set.

Ah, I forgot about the "no_console_suspend" parameter. The problem
with this patch is that it would quietly drop all pending messages.

drm_log_write_thread() does not have any return value.
nbcon_emit_next_record() would assume that the message was printed.
And the kthread would continue emitting next message...

In compare, CON_SUSPENDED would cause that console_is_usable()
returns false. As a result, nbcon_kthread_func() would not try
to emit any message and go into a sleep.

If we set CON_SUSPENDED then the pending messages will get printed
after the resume. If we use this patch, the messages would get lost.


This is why I am not happy with this patch. I would prefer to
block the console. I see three better solutions:

   1. Set CON_SUSPENDED from drm_log_client_suspend even when
      "no_console_suspend" is used.

      It is a bit dirty and might cause some confusion.


   2. Add a new flag, e.g. CON_BLOCKED or CON_DRIVER_BLOCKED,
      which might be used for this purpose.


   3. Allow con->write_thread() to return an error code.

      The question is how exactly the error should be handled.
      The kthread would not know when the printing might succeed
      again.


I personally prefer the 2nd variant.

I looked at what serial drivers are doing, because they can also have their clock gated in suspend.

Would calling console_stop() in the suspend and console_start() in resume work ?

https://elixir.bootlin.com/linux/v6.11.6/source/drivers/tty/serial/serial_core.c#L2462

https://elixir.bootlin.com/linux/v6.11.6/source/kernel/printk/printk.c#L3323

It looks like it should do exactly what we need ?

Best regards,

--

Jocelyn



Best Regards,
Petr

PS: I am sorry for the late reply. I had vacation...





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux