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

[Petr Mladek <pmladek@suse.com>]

On Mon 2024-11-04 11:52:33, John Ogness wrote:
> On 2024-11-04, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> > 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 ?
> 
> Yes. That is what it is for.

It seems that you are right. I have never really investigated the purpose
of this API /o\

One problem with this API is that it does not check whether the
console is registered. I wonder whether it might cause problems.

For example, we should not set the CON_ENABLE flag when the console is not
registered. Doing so would cause register_console() to always enable
the console, even when it is not preferred.

Additionally, nbcon_kthread_wake() uses con->rcuwait, which is initialized
by nbcon_alloc() called from register_console(). Fortunately, nbcon_alloc()
is always called even if the console is not enabled in the end, but this
might change in the future and cause subtle errors.

[ After even more thinking ]

I wonder whether console_start()/console_stop() should really
manipulate CON_ENABLE flag. It might be historical solution when
@console_suspended was a global variable.

But it has changed with the commit 9e70a5e109a4a2336 ("printk: Add
per-console suspended state").

It might make more sense when console_start()/console_stop()
manipulates CON_SUSPENDED flag. Then it would make sense
to rename them suspend_this_console()/resume_this_console().

What do you think?

Best Regards,
Petr