Re: [PATCH v2 1/3] drm: Add support for panic message output

From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Michel Dänzer" <michel@daenzer.net>
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, darwish.07@gmail.com
Subject: Re: [PATCH v2 1/3] drm: Add support for panic message output
Date: Wed, 13 Mar 2019 14:37:54 +0100	[thread overview]
Message-ID: <0baabe1f-8924-2c9a-5cd4-59084a37dbb2@gmail.com> (raw)
In-Reply-To: <20190313133108.GL3888@intel.com>

Am 13.03.19 um 14:31 schrieb Ville Syrjälä:
> On Wed, Mar 13, 2019 at 10:35:08AM +0100, Michel Dänzer wrote:
>> On 2019-03-12 6:15 p.m., 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'm afraid that won't help, at least not without porting big chunks of
>> https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/amd/addrlib
>> into the kernel, none of which will be used for anything else.
>>
>>
>>>>> 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.
>> Copy how? Using a GPU engine?
> CPU maybe? Though I suppose that won't work if the buffer isn't CPU
> accesible :/

Well we do have a debug path for accessing invisible memory with the CPU.

E.g. three registers: DATA and auto increment OFFSET_LO/HI. So you can 
just read/write DATA over and over again if you want to access some memory.

But turning of tilling etc is still extremely tricky when the system is 
already unstable.

I mean from two miles high it looks like a nice to have feature, but up 
close is a different picture...

Christian.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx