Den 11.03.2019 20.29, skrev Daniel Vetter:
> On Mon, Mar 11, 2019 at 08:23:38PM +0100, Daniel Vetter wrote:
>> 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 <nor...@tronnes.org>

<snip>

>>> +static void drm_panic_try_dev(struct drm_device *dev, struct kmsg_dumper 
>>> *dumper)
>>> +{
>>> +   struct drm_framebuffer *fb;
>>> +   struct drm_plane *plane;
>>> +   struct drm_crtc *crtc;
>>> +
>>> +   if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>>> +           return;
>>> +
>>> +   if (dumper->max_reason == KMSG_DUMP_OOPS)
>>> +           pr_info("%s: %s on minor %d\n", __func__, dev->driver->name,
>>> +                   dev->primary->index);
>>> +
>>> +   drm_for_each_crtc(crtc, dev) {
>>> +           if (!ww_mutex_trylock(&crtc->mutex.mutex))
>>> +                   continue;
>>> +
>>> +           if (!crtc->enabled || !crtc->primary)
>>> +                   goto crtc_unlock;
>>> +
>>> +           if (!crtc->state || !crtc->state->active)
>>> +                   goto crtc_unlock;
>>> +
>>> +           plane = crtc->primary;
>>> +           if (!ww_mutex_trylock(&plane->mutex.mutex))
>>> +                   goto crtc_unlock;
>>> +
>>> +           /*
>>> +            * TODO: Should we check plane_state->visible?
>>> +            * It is not set on vc4
>>
>> I think we should. We should also check whether that primary is connected
>> to the crtc (some hw you can reassign planes freely between crtc, and the
>> crtc->primary pointer is only used for compat with legacy ioctl).
>>
>>> +           if (!plane->state || !plane->state->visible)
>>> +            */
>>> +           if (!plane->state)
>>> +                   goto plane_unlock;
>>> +
>>> +           fb = plane->state->fb;
>>> +           if (!fb || !fb->funcs->panic_vmap)
>>
>> Docs says this is optional, doesn't seem to be all that optional. I'd
>> check for this or a driver-specific ->panic_draw_xy instead.
>>> +                   goto plane_unlock;
>>> +
>>> +           /*
>>> +            * fbcon puts out panic messages so stay away to avoid jumbled
>>> +            * output. If vc->vc_mode != KD_TEXT fbcon won't put out
>>> +            * messages (see vt_console_print).
>>> +            */
>>> +           if (dev->fb_helper && dev->fb_helper->fb == fb)
> 
> This is a bit a layering violation. Could we instead tell fbcon that it
> shouldn't do panic handling for drm drivers? I think that would resolve
> this overlap here in a much cleaner way. drm fbdev helpers already have
> lots of oops_in_progress checks all over to make sure fbcon doesn't do
> anything bad. That only leaves the actual rendering, which I think we can
> stop too with a simple flag.
> 
> Ofc only for atomic drivers which have this panic handling mode here
> implemented.

There used to be a fbdev flag FBINFO_CAN_FORCE_OUTPUT that controlled
vc->vc_panic_force_write, but it's gone now I see.

Noralf.

> -Daniel
> 
>>> +                   goto plane_unlock;
>>> +
>>> +           drm_panic_kmsg_render_screen(plane, dumper);
>>> +plane_unlock:
>>> +           ww_mutex_unlock(&plane->mutex.mutex);
>>> +crtc_unlock:
>>> +           ww_mutex_unlock(&crtc->mutex.mutex);
>>> +   }
>>> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to