diff mbox series

drm/todo: Update panic handling todo

Message ID 20220224125934.3461478-1-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series drm/todo: Update panic handling todo | expand

Commit Message

Daniel Vetter Feb. 24, 2022, 12:59 p.m. UTC
Some things changed, and add two useful links.

Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: gpiccoli@igalia.com
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 Documentation/gpu/todo.rst | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

Javier Martinez Canillas Feb. 24, 2022, 1:08 p.m. UTC | #1
Hello Daniel,

On 2/24/22 13:59, Daniel Vetter wrote:
> Some things changed, and add two useful links.
> 
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: gpiccoli@igalia.com
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  Documentation/gpu/todo.rst | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7bf7f2111696..283d35a500bd 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 another.

The "over each another" sounds a little bit off, shouldn't be "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,9 @@ 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.
> -
> -* 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.
> +* 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/20190311174218.51899-1-noralf@tronnes.org/>`_.
>  

Having these two links in the docs is very useful. Thanks a lot for adding that.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Best regards,
diff mbox series

Patch

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 7bf7f2111696..283d35a500bd 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 another.
+  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,9 @@  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.
-
-* 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.
+* 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/20190311174218.51899-1-noralf@tronnes.org/>`_.
 
 Contact: Daniel Vetter