Re: drm/nouveau: crash regression in 3.5

[Marcin Slusarz <marcin.slusarz@gmail.com>]

On Thu, Jul 26, 2012 at 02:56:22PM +0200, Ortwin Glück wrote:
> On 25.07.2012 20:42, Marcin Slusarz wrote:
> > Good, below patch should fix this panic.
> >
> > Note that you can hit an oops in drm_handle_vblank because patch from
> > http://lists.freedesktop.org/archives/dri-devel/2012-May/023498.html
> > has not been applied (yet?).
> 
> After applying your patch, it still crashes, although with a slightly 
> different stack trace. I then also applied the second patch, but that 
> doesn't make any difference. New log attached.
> 
> Looks like interrupt occurs before nouveau_software_context_new() is 
> called? Shouldn't the initialization be done from 
> nouveau_irq_preinstall() so it is available when the irq occurs? Again, 
> I am not an expert here. Just guessing...

No, the real problem is: with "noaccel" we don't register "software engine",
but vblank ISR relies on its existance and happily derefences NULL pointer.

Now, this patch should fix it for real...

---
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Subject: [PATCH] drm/nouveau: disable vblank interrupts before registering PDISPLAY ISR

Currently, we register vblank IRQ handler and later we disable vblank
interrupts. So, for the short amount of time, we rely on vblank ISR
to operate correctly, even if vblank interrupts are never going to be
used later.

In "noaccel" case, software engine - which is used by vblank ISR - is not
registered, so if vblank interrupt triggers in a wrong moment, we can hit
NULL pointer dereference in nouveau_software_vblank.

To fix it, disable vblank interrupts before registering PDISPLAY ISR.

Reported-by: Ortwin Glück <odi@odi.ch>
Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: stable@vger.kernel.org [3.5]
---
 drivers/gpu/drm/nouveau/nv04_crtc.c    |    1 +
 drivers/gpu/drm/nouveau/nv50_crtc.c    |    1 +
 drivers/gpu/drm/nouveau/nvd0_display.c |    2 ++
 3 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index 4c31c63..38bfe8d 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -1057,6 +1057,7 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
 	}
 
 	nv04_cursor_init(nv_crtc);
+	nouveau_vblank_disable(dev, crtc_num);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/nouveau/nv50_crtc.c b/drivers/gpu/drm/nouveau/nv50_crtc.c
index 97a477b..7648f52 100644
--- a/drivers/gpu/drm/nouveau/nv50_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv50_crtc.c
@@ -792,6 +792,7 @@ nv50_crtc_create(struct drm_device *dev, int index)
 		goto out;
 
 	nv50_cursor_init(nv_crtc);
+	nouveau_vblank_disable(dev, index);
 out:
 	if (ret)
 		nv50_crtc_destroy(&nv_crtc->base);
diff --git a/drivers/gpu/drm/nouveau/nvd0_display.c b/drivers/gpu/drm/nouveau/nvd0_display.c
index c486d3c..32f8a86 100644
--- a/drivers/gpu/drm/nouveau/nvd0_display.c
+++ b/drivers/gpu/drm/nouveau/nvd0_display.c
@@ -908,6 +908,8 @@ nvd0_crtc_create(struct drm_device *dev, int index)
 		goto out;
 
 	nvd0_crtc_lut_load(crtc);
+	/* uncomment once nvd0 vblank lands */
+	/* nouveau_vblank_disable(dev, index); */
 
 out:
 	if (ret)
--