dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [RFC PATCH] drm/fb: drop panic handling
Date: Thu, 16 Jul 2015 10:27:52 +0200	[thread overview]
Message-ID: <20150716082752.GC6223@phenom.ffwll.local> (raw)
In-Reply-To: <20150709211443.GA3736@phenom.ffwll.local>

On Thu, Jul 09, 2015 at 11:14:43PM +0200, Daniel Vetter wrote:
> On Thu, Jul 09, 2015 at 01:15:34PM +1000, Dave Airlie wrote:
> > From: Dave Airlie <airlied@redhat.com>
> > 
> > This really doesn't seem to have much chance of working anymore,
> > 
> > esp for irq context, qxl at least tries to talk to the hw,
> > and waits for irqs, and fails.
> > 
> > with runtime pm and other stuff I think we should just
> > bail on this for now.
> > 
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> Yeah concurred that this has become hopeless. Also this would allow us to
> drop an pretension in i915 to still support this which means we can stop
> checking drm_can_sleep in our wait_for macros. Which has papered over some
> pretty serious bugs.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Well I applied this to drm-misc.

> There's one more though, we can get into the fbdev callbacks through a
> pretty impressive chain:
> 
> panic() -> bust_spinlock() -> console_unblank() -> pass the trylock ->
> 	c->unblank() -> unblank_screen() (now in vt/vt.c) ->
> 		vc_sw->con_blank() -> fbcon_blank()
> 
> To make this really complete I think we also need to sprinkle
> 
> 	if (oops_in_progress)
> 		return;
> 
> over all the fbdev entry points we have in drm_fbdev_helper.c plus all the
> ones in drivers which have their own (qxl, udl, i915 are the ones I know
> of).

I'll do a patch for this. Just realized that we have some cargo-culted
checks already, but they don't work everywhere so mostly about unifying
everything.
-Daniel

> 
> Cheers, Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c | 26 --------------------------
> >  1 file changed, 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index cac4229..eaf652b 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -429,24 +429,6 @@ static bool drm_fb_helper_force_kernel_mode(void)
> >  	return error;
> >  }
> >  
> > -static int drm_fb_helper_panic(struct notifier_block *n, unsigned long ununsed,
> > -			void *panic_str)
> > -{
> > -	/*
> > -	 * It's a waste of time and effort to switch back to text console
> > -	 * if the kernel should reboot before panic messages can be seen.
> > -	 */
> > -	if (panic_timeout < 0)
> > -		return 0;
> > -
> > -	pr_err("panic occurred, switching back to text console\n");
> > -	return drm_fb_helper_force_kernel_mode();
> > -}
> > -
> > -static struct notifier_block paniced = {
> > -	.notifier_call = drm_fb_helper_panic,
> > -};
> > -
> >  static bool drm_fb_helper_is_bound(struct drm_fb_helper *fb_helper)
> >  {
> >  	struct drm_device *dev = fb_helper->dev;
> > @@ -672,9 +654,6 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
> >  	if (!list_empty(&fb_helper->kernel_fb_list)) {
> >  		list_del(&fb_helper->kernel_fb_list);
> >  		if (list_empty(&kernel_fb_helper_list)) {
> > -			pr_info("drm: unregistered panic notifier\n");
> > -			atomic_notifier_chain_unregister(&panic_notifier_list,
> > -							 &paniced);
> >  			unregister_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> >  		}
> >  	}
> > @@ -1109,12 +1088,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
> >  	dev_info(fb_helper->dev->dev, "fb%d: %s frame buffer device\n",
> >  			info->node, info->fix.id);
> >  
> > -	/* Switch back to kernel console on panic */
> > -	/* multi card linked list maybe */
> >  	if (list_empty(&kernel_fb_helper_list)) {
> > -		dev_info(fb_helper->dev->dev, "registered panic notifier\n");
> > -		atomic_notifier_chain_register(&panic_notifier_list,
> > -					       &paniced);
> >  		register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op);
> >  	}
> >  
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2015-07-16  8:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-09  3:15 [RFC PATCH] drm/fb: drop panic handling Dave Airlie
2015-07-09 21:14 ` Daniel Vetter
2015-07-16  8:27   ` Daniel Vetter [this message]
2015-08-20  5:27     ` Jesse Barnes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150716082752.GC6223@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox