[Intel-gfx] [PATCH] drm/fbdev-helper: Explain how to debug console_lock fun

[Daniel Vetter]

On Sun, Jan 24, 2016 at 10:33:57PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> Thank you for the patch.
> 
> On Friday 22 January 2016 08:53:45 Daniel Vetter wrote:
> > Every new KMS driver writer seems to run into this and wonder how
> > exactly drm_fb_helper_initial_config can die doing nothing at all.
> > Set up some big warnings signs around this newbie trap to avoid future
> > frustration and wasting everyone's time.
> > 
> > Cc: Carlos Palminha <CARLOS.PALMINHA at synopsys.com>
> > Cc: Xinliang Liu <xinliang.liu at linaro.org>
> > Cc: laurent.pinchart at ideasonboard.com
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c index 1e103c4c6ee0..9bf84b227613 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2091,6 +2091,28 @@ out:
> >   * drm_fb_helper_fill_fix() are provided as helpers to setup simple default
> > * values for the fbdev info structure.
> >   *
> > + * HANG DEBUGGING:
> > + *
> > + * When you have fbcon support built-in or already loaded, this function
> > will do
> > + * a full modeset to setup the fbdev console. And due to locking misdesign
> 
> No need to start the sentence with "And", s/And due/Due/ will do.
> 
> > in
> > + * the VT/fbdev subsystem that entire modeset sequence has to be done while
> > + * holding console_lock. And until console_unlock is called no dmesg lines
> 
> Same here.
> 
> > will
> > + * be sent out to consoles, not even serial console. Which means when your
> 
> Similarly, s/Which means when/This means that if/
> 
> > + * driver crashes, you will see absolutely nothing else but a system stuck
> > in
> > + * this function, with no further output. And any kind of printk() you
> 
> s/And any/Any/
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks for the review, added your changes and merged the patch.

> Very useful addition to the documentation, I got bitten by this more than 
> once. I wonder whether we could make this information even easier to find.

I figured most likely when debugging hangs you will notice eventually that
the hang is in this function and then stumble over the kerneldoc.
Everywhere else it will probably get ignored. That's why I placed it here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch