drm/todo: Update panic handling todo

Commit Message

Daniel Vetter Feb. 24, 2022, 1:24 p.m. UTC

Some things changed, and add two useful links.

v2: Also include a link to the QR encoding work. Plus review from
Javier.

Reviewed-by: Javier Martinez Canillas <[e-mail]>
Cc: Javier Martinez Canillas <[e-mail]>
Cc: Pekka Paalanen <[e-mail]>
Cc: [e-mail]
Signed-off-by: Daniel Vetter <[e-mail]>
---
 Documentation/gpu/todo.rst | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Comments

Guilherme G. Piccoli Feb. 24, 2022, 1:53 p.m. UTC | #1

On 24/02/2022 10:24, Daniel Vetter wrote:
> Some things changed, and add two useful links.
> 
> v2: Also include a link to the QR encoding work. Plus review from
> Javier.
> 
> Reviewed-by: Javier Martinez Canillas <[e-mail]>
> Cc: Javier Martinez Canillas <[e-mail]>
> Cc: Pekka Paalanen <[e-mail]>
> Cc: [e-mail]
> Signed-off-by: Daniel Vetter <[e-mail]>
> ---

Hi Daniel, thanks for the improvement - much appreciated!

Below a comment inline; other than that, feel free to add my:
Reviewed-by: Guilherme G. Piccoli <[e-mail]>

Cheers,


Guilherme

> [...]
>  
> -* There's also proposal for a simplied DRM console instead of the full-blown
> -  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
> -  obviously work for both console, in case we ever get kmslog merged.
> +* Encoding the actual oops and preceeding dmesg in a QR might help with the

Email client complains about "preceeding" word - misspelled maybe?

> +  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops messages
> +  transfer using QR codes
> +  <https://lore.kernel.org/lkml/[e-mail]/>`_
> +  for some example code that could be reused.
>  
>  Contact: Daniel Vetter
>

Pekka Paalanen Feb. 25, 2022, 9:51 a.m. UTC | #2

On Thu, 24 Feb 2022 14:24:25 +0100
Daniel Vetter <[e-mail]> wrote:

> Some things changed, and add two useful links.
> 
> v2: Also include a link to the QR encoding work. Plus review from
> Javier.
> 
> Reviewed-by: Javier Martinez Canillas <[e-mail]>
> Cc: Javier Martinez Canillas <[e-mail]>
> Cc: Pekka Paalanen <[e-mail]>
> Cc: [e-mail]
> Signed-off-by: Daniel Vetter <[e-mail]>
> ---
>  Documentation/gpu/todo.rst | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)

Acked-by: Pekka Paalanen <[e-mail]>

Thanks,
pq


> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7bf7f2111696..2b1e7fa45603 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -475,8 +475,12 @@ This is a really varied tasks with lots of little bits and pieces:
>    achieved by using an IPI to the local processor.
>  
>  * There's a massive confusion of different panic handlers. DRM fbdev emulation
> -  helpers have one, but on top of that the fbcon code itself also has one. We
> -  need to make sure that they stop fighting over each another.
> +  helpers had their own (long removed), but on top of that the fbcon code itself
> +  also has one. We need to make sure that they stop fighting over each other.
> +  This is worked around by checking ``oops_in_progress`` at various entry points
> +  into the DRM fbdev emulation helpers. A much cleaner approach here would be to
> +  switch fbcon to the `threaded printk support
> +  <https://lwn.net/Articles/800946/>`_.
>  
>  * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and
>    isn't a full solution for panic paths. We need to make sure that it only
> @@ -488,16 +492,15 @@ This is a really varied tasks with lots of little bits and pieces:
>    even spinlocks (because NMI and hardirq can panic too). We need to either
>    make sure to not call such paths, or trylock everything. Really tricky.
>  
> -* For the above locking troubles reasons it's pretty much impossible to
> -  attempt a synchronous modeset from panic handlers. The only thing we could
> -  try to achive is an atomic ``set_base`` of the primary plane, and hope that
> -  it shows up. Everything else probably needs to be delayed to some worker or
> -  something else which happens later on. Otherwise it just kills the box
> -  harder, prevent the panic from going out on e.g. netconsole.
> +* A clean solution would be an entirely separate panic output support in KMS,
> +  bypassing the current fbcon support. See `[PATCH v2 0/3] drm: Add panic handling
> +  <https://lore.kernel.org/dri-devel/[e-mail]/>`_.
>  
> -* There's also proposal for a simplied DRM console instead of the full-blown
> -  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
> -  obviously work for both console, in case we ever get kmslog merged.
> +* Encoding the actual oops and preceeding dmesg in a QR might help with the
> +  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops messages
> +  transfer using QR codes
> +  <https://lore.kernel.org/lkml/[e-mail]/>`_
> +  for some example code that could be reused.
>  
>  Contact: Daniel Vetter
>

Daniel Vetter Feb. 28, 2022, 8:58 a.m. UTC | #3

On Thu, Feb 24, 2022 at 10:53:01AM -0300, Guilherme G. Piccoli wrote:
> On 24/02/2022 10:24, Daniel Vetter wrote:
> > Some things changed, and add two useful links.
> > 
> > v2: Also include a link to the QR encoding work. Plus review from
> > Javier.
> > 
> > Reviewed-by: Javier Martinez Canillas <[e-mail]>
> > Cc: Javier Martinez Canillas <[e-mail]>
> > Cc: Pekka Paalanen <[e-mail]>
> > Cc: [e-mail]
> > Signed-off-by: Daniel Vetter <[e-mail]>
> > ---
> 
> Hi Daniel, thanks for the improvement - much appreciated!
> 
> Below a comment inline; other than that, feel free to add my:
> Reviewed-by: Guilherme G. Piccoli <[e-mail]>
> 
> Cheers,
> 
> 
> Guilherme
> 
> > [...]
> >  
> > -* There's also proposal for a simplied DRM console instead of the full-blown
> > -  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
> > -  obviously work for both console, in case we ever get kmslog merged.
> > +* Encoding the actual oops and preceeding dmesg in a QR might help with the
> 
> Email client complains about "preceeding" word - misspelled maybe?

Indeed, I've fixed this while applying.
-Daniel

> 
> > +  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops messages
> > +  transfer using QR codes
> > +  <https://lore.kernel.org/lkml/[e-mail]/>`_
> > +  for some example code that could be reused.
> >  
> >  Contact: Daniel Vetter
> >

Patch

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 7bf7f2111696..2b1e7fa45603 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -475,8 +475,12 @@  This is a really varied tasks with lots of little bits and pieces:
   achieved by using an IPI to the local processor.
 
 * There's a massive confusion of different panic handlers. DRM fbdev emulation
-  helpers have one, but on top of that the fbcon code itself also has one. We
-  need to make sure that they stop fighting over each another.
+  helpers had their own (long removed), but on top of that the fbcon code itself
+  also has one. We need to make sure that they stop fighting over each other.
+  This is worked around by checking ``oops_in_progress`` at various entry points
+  into the DRM fbdev emulation helpers. A much cleaner approach here would be to
+  switch fbcon to the `threaded printk support
+  <https://lwn.net/Articles/800946/>`_.
 
 * ``drm_can_sleep()`` is a mess. It hides real bugs in normal operations and
   isn't a full solution for panic paths. We need to make sure that it only
@@ -488,16 +492,15 @@  This is a really varied tasks with lots of little bits and pieces:
   even spinlocks (because NMI and hardirq can panic too). We need to either
   make sure to not call such paths, or trylock everything. Really tricky.
 
-* For the above locking troubles reasons it's pretty much impossible to
-  attempt a synchronous modeset from panic handlers. The only thing we could
-  try to achive is an atomic ``set_base`` of the primary plane, and hope that
-  it shows up. Everything else probably needs to be delayed to some worker or
-  something else which happens later on. Otherwise it just kills the box
-  harder, prevent the panic from going out on e.g. netconsole.
+* A clean solution would be an entirely separate panic output support in KMS,
+  bypassing the current fbcon support. See `[PATCH v2 0/3] drm: Add panic handling
+  <https://lore.kernel.org/dri-devel/[e-mail]/>`_.
 
-* There's also proposal for a simplied DRM console instead of the full-blown
-  fbcon and DRM fbdev emulation. Any kind of panic handling tricks should
-  obviously work for both console, in case we ever get kmslog merged.
+* Encoding the actual oops and preceeding dmesg in a QR might help with the
+  dread "important stuff scrolled away" problem. See `[RFC][PATCH] Oops messages
+  transfer using QR codes
+  <https://lore.kernel.org/lkml/[e-mail]/>`_
+  for some example code that could be reused.
 
 Contact: Daniel Vetter