| Message ID | 1369117423-7772-1-git-send-email-chenhc@lemote.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Die, 2013-05-21 at 14:23 +0800, Huacai Chen wrote: > When GPU acceleration is disabled, drm_vblank_cleanup() will free the > vblank-related data, such as vblank_refcount, vblank_inmodeset, etc. > But we found that drm_vblank_post_modeset() may be called after the > cleanup, which use vblank_refcount and vblank_inmodeset. And this will > cause a kernel panic. > > Fix this by return immediately if dev->num_crtcs is zero. This is the > same thing that drm_vblank_pre_modeset() does. > > Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup(): > [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4 > [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174 > [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38 > [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc > [ 62.628906] [<ffffffff8047f92c>] drm_crtc_helper_set_config+0x6d8/0x988 > [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104 > [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c > [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118 > [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384 > [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc > [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94 > [ 62.628906] [<ffffffff8024fcbc>] __blocking_notifier_call_chain+0x48/0x68 > [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260 > [ 62.628906] [<ffffffff8047e010>] drm_fb_helper_single_fb_probe+0x260/0x314 > [ 62.628906] [<ffffffff8047e2c4>] drm_fb_helper_initial_config+0x200/0x234 > [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4 > [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18 > [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c > [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238 > [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0 > [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30 > [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc > [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44 > [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4 > [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90 > [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18 > > Signed-off-by: Huacai Chen <chenhc@lemote.com> > Signed-off-by: Binbin Zhou <zhoubb@lemote.com> > Cc: <stable@vger.kernel.org> > --- > drivers/gpu/drm/drm_irq.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index c798eea..c7a1348 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -981,7 +981,7 @@ EXPORT_SYMBOL(drm_vblank_off); > */ > void drm_vblank_pre_modeset(struct drm_device *dev, int crtc) > { > - /* vblank is not initialized (IRQ not installed ?) */ > + /* vblank is not initialized (IRQ not installed ?), or has been freed */ > if (!dev->num_crtcs) > return; > /* > @@ -1003,6 +1003,10 @@ void drm_vblank_post_modeset(struct drm_device *dev, int crtc) > { > unsigned long irqflags; > > + /* vblank is not initialized (IRQ not installed ?), or has been freed */ > + if (!dev->num_crtcs) > + return; > + > if (dev->vblank_inmodeset[crtc]) { > spin_lock_irqsave(&dev->vbl_lock, irqflags); > dev->vblank_disable_allowed = 1; The comment changes seem superfluous to me, but either way: Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Am Dienstag, den 21.05.2013, 14:23 +0800 schrieb Huacai Chen: > When GPU acceleration is disabled, drm_vblank_cleanup() will free the > vblank-related data, such as vblank_refcount, vblank_inmodeset, etc. > But we found that drm_vblank_post_modeset() may be called after the > cleanup, which use vblank_refcount and vblank_inmodeset. And this will > cause a kernel panic. > > Fix this by return immediately if dev->num_crtcs is zero. This is the > same thing that drm_vblank_pre_modeset() does. > > Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup(): > [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4 > [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174 > [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38 > [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc > [ 62.628906] [<ffffffff8047f92c>] drm_crtc_helper_set_config+0x6d8/0x988 > [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104 > [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c > [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118 > [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384 > [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc > [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94 > [ 62.628906] [<ffffffff8024fcbc>] __blocking_notifier_call_chain+0x48/0x68 > [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260 > [ 62.628906] [<ffffffff8047e010>] drm_fb_helper_single_fb_probe+0x260/0x314 > [ 62.628906] [<ffffffff8047e2c4>] drm_fb_helper_initial_config+0x200/0x234 > [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4 > [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18 > [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c > [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238 > [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0 > [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30 > [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc > [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44 > [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4 > [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90 > [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18 Nice commit message! > Signed-off-by: Huacai Chen <chenhc@lemote.com> > Signed-off-by: Binbin Zhou <zhoubb@lemote.com> > Cc: <stable@vger.kernel.org> > --- > drivers/gpu/drm/drm_irq.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) […] Acked-by: Paul Menzel <paulepanter@users.sourceforge.net> Thanks, Paul
Hi, Dave Why this patch hasn't been applied in your tree? I think you are the right maintainer. Huacai Chen On Tue, May 21, 2013 at 11:30 PM, Paul Menzel < paulepanter@users.sourceforge.net> wrote: > Am Dienstag, den 21.05.2013, 14:23 +0800 schrieb Huacai Chen: > > When GPU acceleration is disabled, drm_vblank_cleanup() will free the > > vblank-related data, such as vblank_refcount, vblank_inmodeset, etc. > > But we found that drm_vblank_post_modeset() may be called after the > > cleanup, which use vblank_refcount and vblank_inmodeset. And this will > > cause a kernel panic. > > > > Fix this by return immediately if dev->num_crtcs is zero. This is the > > same thing that drm_vblank_pre_modeset() does. > > > > Call trace of a drm_vblank_post_modeset() after drm_vblank_cleanup(): > > [ 62.628906] [<ffffffff804868d0>] drm_vblank_post_modeset+0x34/0xb4 > > [ 62.628906] [<ffffffff804c7008>] atombios_crtc_dpms+0xb4/0x174 > > [ 62.628906] [<ffffffff804c70e0>] atombios_crtc_commit+0x18/0x38 > > [ 62.628906] [<ffffffff8047f038>] drm_crtc_helper_set_mode+0x304/0x3cc > > [ 62.628906] [<ffffffff8047f92c>] > drm_crtc_helper_set_config+0x6d8/0x988 > > [ 62.628906] [<ffffffff8047dd40>] drm_fb_helper_set_par+0x94/0x104 > > [ 62.628906] [<ffffffff80439d14>] fbcon_init+0x424/0x57c > > [ 62.628906] [<ffffffff8046a638>] visual_init+0xb8/0x118 > > [ 62.628906] [<ffffffff8046b9f8>] take_over_console+0x238/0x384 > > [ 62.628906] [<ffffffff80436df8>] fbcon_takeover+0x7c/0xdc > > [ 62.628906] [<ffffffff8024fa20>] notifier_call_chain+0x44/0x94 > > [ 62.628906] [<ffffffff8024fcbc>] > __blocking_notifier_call_chain+0x48/0x68 > > [ 62.628906] [<ffffffff8042d990>] register_framebuffer+0x228/0x260 > > [ 62.628906] [<ffffffff8047e010>] > drm_fb_helper_single_fb_probe+0x260/0x314 > > [ 62.628906] [<ffffffff8047e2c4>] > drm_fb_helper_initial_config+0x200/0x234 > > [ 62.628906] [<ffffffff804e5560>] radeon_fbdev_init+0xd4/0xf4 > > [ 62.628906] [<ffffffff804e0e08>] radeon_modeset_init+0x9bc/0xa18 > > [ 62.628906] [<ffffffff804bfc14>] radeon_driver_load_kms+0xdc/0x12c > > [ 62.628906] [<ffffffff8048b548>] drm_get_pci_dev+0x148/0x238 > > [ 62.628906] [<ffffffff80423564>] local_pci_probe+0x5c/0xd0 > > [ 62.628906] [<ffffffff80241ac4>] work_for_cpu_fn+0x1c/0x30 > > [ 62.628906] [<ffffffff802427c8>] process_one_work+0x274/0x3bc > > [ 62.628906] [<ffffffff80242934>] process_scheduled_works+0x24/0x44 > > [ 62.628906] [<ffffffff8024515c>] worker_thread+0x31c/0x3f4 > > [ 62.628906] [<ffffffff802497a8>] kthread+0x88/0x90 > > [ 62.628906] [<ffffffff80206794>] kernel_thread_helper+0x10/0x18 > > Nice commit message! > > > Signed-off-by: Huacai Chen <chenhc@lemote.com> > > Signed-off-by: Binbin Zhou <zhoubb@lemote.com> > > Cc: <stable@vger.kernel.org> > > --- > > drivers/gpu/drm/drm_irq.c | 6 +++++- > > 1 files changed, 5 insertions(+), 1 deletions(-) > > […] > > Acked-by: Paul Menzel <paulepanter@users.sourceforge.net> > > > Thanks, > > Paul > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel > >
On Mon, Jun 3, 2013 at 6:29 PM, Huacai Chen <chenhc@lemote.com> wrote: > Hi, Dave > > Why this patch hasn't been applied in your tree? I think you are the right > maintainer. > Because I missed it, i've stuck in my fixes queue now. thanks for pinging. Dave.
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index c798eea..c7a1348 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -981,7 +981,7 @@ EXPORT_SYMBOL(drm_vblank_off); */ void drm_vblank_pre_modeset(struct drm_device *dev, int crtc) { - /* vblank is not initialized (IRQ not installed ?) */ + /* vblank is not initialized (IRQ not installed ?), or has been freed */ if (!dev->num_crtcs) return; /* @@ -1003,6 +1003,10 @@ void drm_vblank_post_modeset(struct drm_device *dev, int crtc) { unsigned long irqflags; + /* vblank is not initialized (IRQ not installed ?), or has been freed */ + if (!dev->num_crtcs) + return; + if (dev->vblank_inmodeset[crtc]) { spin_lock_irqsave(&dev->vbl_lock, irqflags); dev->vblank_disable_allowed = 1;