On Tue, Mar 12, 2019 at 08:02:56PM +0200, Ville Syrjälä wrote:
On Tue, Mar 12, 2019 at 06:37:57PM +0100, Noralf Trønnes wrote:
Den 12.03.2019 18.25, skrev Ville Syrjälä:
On Tue, Mar 12, 2019 at 06:15:24PM +0100, Noralf Trønnes wrote:
Den 12.03.2019 17.17, skrev Ville Syrjälä:
On Tue, Mar 12, 2019 at 11:47:04AM +0100, Michel Dänzer wrote:
On 2019-03-11 6:42 p.m., Noralf Trønnes wrote: > This adds support for outputting kernel messages on panic(). > A kernel message dumper is used to dump the log. The dumper iterates > over each DRM device and it's crtc's to find suitable framebuffers. > > All the other dumpers are run before this one except mtdoops. > Only atomic drivers are supported. > > Signed-off-by: Noralf Trønnes noralf@tronnes.org > --- > [...] > > diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h > index f0b34c977ec5..f3274798ecfe 100644 > --- a/include/drm/drm_framebuffer.h > +++ b/include/drm/drm_framebuffer.h > @@ -94,6 +94,44 @@ struct drm_framebuffer_funcs { > struct drm_file *file_priv, unsigned flags, > unsigned color, struct drm_clip_rect *clips, > unsigned num_clips); > + > + /** > + * @panic_vmap: > + * > + * Optional callback for panic handling. > + * > + * For vmapping the selected framebuffer in a panic context. Must > + * be super careful about locking (only trylocking allowed). > + * > + * RETURNS: > + * > + * NULL if it didn't work out, otherwise an opaque cookie which is > + * passed to @panic_draw_xy. It can be anything: vmap area, structure > + * with more details, just a few flags, ... > + */ > + void *(*panic_vmap)(struct drm_framebuffer *fb);
FWIW, the panic_vmap hook cannot work in general with the amdgpu/radeon drivers:
Framebuffers are normally tiled, writing to them with the CPU results in garbled output.
In which case the driver needs to support the ->panic_draw_xy callback, or maybe it's possible to make a generic helper for tiled buffers.
I've proposed somewhere else that we rename panic_vmap to panic_prepare, and the vmap pointer to an abstract cookie. Then the driver can do whatever it wants too, e.g. in ->panic_prepare it does a few trylocks to get at the buffer and make sure it can set up a temporary pte to write into it page-by-page. ->panic_draw_xy can then do whatever it needs to do, using the opaque void *cookie.
And if the trylock fails you just return NULL from ->panic_prepare
And ->panic_cleanup would be just to clean up the mess for the validation use-case when running this from debugfs.
With a discrete GPU having a large amount of VRAM, the framebuffer may not be directly CPU accessible at all.
I would have been nice to know how Windows works around this.
There would need to be a mechanism for switching scanout to a linear, CPU accessible framebuffer.
I suppose panic_vmap() could just provide a linear temp buffer to the panic handler, and panic_unmap() could copy the contents over to the real fb.
That said, this approach of scribbling over the primary plane's framebuffer has some clear limitations:
- something may overwrite the oops message before the user can even read it
When the dumper drm_panic_kmsg_dump() runs, the other CPU's should have been stopped. See panic().
GPUs etc. may still be executing away.
Would it be safe to stop it in a panic situation? It would ofc be bad to crash the box even harder.
Some drivers/devices may have working (and hopefully even reliable) gpu reset, some may not.
I don't think touching the gpu is a good idea. Even disabling planes and all that feels risky. And there's really not much working anymore in panic context, we can't even schedule a worker/timer to redraw the panic output a bit later. -Daniel