* [RFC PATCH] drm/fb: drop panic handling
@ 2015-07-09 3:15 Dave Airlie
2015-07-09 21:14 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Dave Airlie @ 2015-07-09 3:15 UTC (permalink / raw)
To: dri-devel
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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] drm/fb: drop panic handling
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
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2015-07-09 21:14 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel
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>
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).
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] drm/fb: drop panic handling
2015-07-09 21:14 ` Daniel Vetter
@ 2015-07-16 8:27 ` Daniel Vetter
2015-08-20 5:27 ` Jesse Barnes
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2015-07-16 8:27 UTC (permalink / raw)
To: Dave Airlie; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] drm/fb: drop panic handling
2015-07-16 8:27 ` Daniel Vetter
@ 2015-08-20 5:27 ` Jesse Barnes
0 siblings, 0 replies; 4+ messages in thread
From: Jesse Barnes @ 2015-08-20 5:27 UTC (permalink / raw)
To: Daniel Vetter, Dave Airlie; +Cc: dri-devel
On 07/16/2015 01:27 AM, Daniel Vetter wrote:
> 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);
>>> }
I missed this; Daniel just told me about it tonight. It's too bad we
have to drop it; I think PPC has had it since forever, so losing it from
KMS is too bad. Hopefully we can bring it back somehow with David's new
kernel console stuff at some point...
Jesse
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-08-20 5:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2015-08-20 5:27 ` Jesse Barnes
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.