[Intel-gfx] [PATCH v2 1/3] drm: Add support for panic message output

[Noralf Trønnes]

Den 12.03.2019 11.58, skrev Daniel Vetter:
> On Mon, Mar 11, 2019 at 11:33:15PM +0100, Noralf Trønnes wrote:
>>
>>
>> Den 11.03.2019 20.23, skrev Daniel Vetter:
>>> On Mon, Mar 11, 2019 at 06:42:16PM +0100, 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 at tronnes.org>
>>>

<snip>

>>>> +static void drm_panic_kmsg_render_screen(struct drm_plane *plane,
>>>> +					 struct kmsg_dumper *dumper)
>>>> +{
>>>> +	struct drm_framebuffer *fb = plane->state->fb;
>>>> +	bool first_iteration = true;
>>>> +	struct drm_panic_ctx ctx;
>>>> +	static char text[1024];
>>>> +	size_t len;
>>>> +
>>>> +	ctx.vmap = fb->funcs->panic_vmap(fb);
>>>> +
>>>> +	/* Print some info when testing */
>>>> +	if (dumper->max_reason == KMSG_DUMP_OOPS) {
>>>> +		struct drm_format_name_buf format_name;
>>>> +
>>>> +		pr_info("%s: [FB:%d] %ux%u format=%s vmap=%p\n",
>>>> +			__func__, fb->base.id, fb->width, fb->height,
>>>> +			drm_get_format_name(fb->format->format, &format_name),
>>>> +			ctx.vmap);
>>>> +	}
>>>> +
>>>> +	if (!ctx.vmap)
>>>> +		return;
>>>
>>> For some framebuffers it might be useful to not require vmap, but instead
>>> have some page-by-page kind of access helper. Since preemptively setting
>>> up a vmap for all the non-continous buffers is a bit much, and we really
>>> can't set up the vmap in the panic handler.
>>>
>>> Maybe we should call this panic_prepare/panic_finish and
>>> s/vmap/cookie/, to make it entirely opaque?
>>>
>>> But a bit overengineering perhaps :-)
>>>
>>
>> I guess i915 wants something else than vmap, but I have no idea how a
>> page-by-page solution is to be implemented.
> 
> i915 should be mostly fine, since we have a GTT for remapping and can make
> it look continuous. It might not be in the part of the GTT accessible by
> the cpu though.
> 
> Wrt implementation: My idea would be to extract the pixel writing function
> and export it to drivers. The driver would then implement a
> ->panic_draw_xy function which looks up the right page, computes it
> address, computes the x/y offset within (taking into account tiling and
> stuff like that), and then uses the helper function to draw the right
> pixel value for the format at the given address.
> 
> That's why I suggested that drivers need to either implement
> ->panic_prepare or ->panic_draw_xy. Since for this approach you might not
> need a ->panic_prepare/cleanup implementation, since it's all done in
> ->panic_draw_xy on a pixel-by-pixel basis.
> 
>> When we get bootsplash, we will at least have a kernel address during
>> that phase for all drivers supporting it.
> 
> Hm, not following what you mean here. Why is bootsplash special?
> 

Special in the sense that it's framebuffer will be linear and already
have a virtual address for its backing buffer since that's a
prerequisite for it to render the splash image.

Noralf.