Nouveau - Jul 2013 - [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup

Sort of fixes mmiotrace for me again, I could sear I sent a similar patch before
the rework to event interface, so I guess it got reintroduced.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
---
diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
index 7e3875d..35e526b 100644
--- a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
+++ b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
@@ -1266,13 +1266,15 @@ nv50_disp_intr(struct nouveau_subdev *subdev)
 	}
 
 	if (intr1 & 0x00000004) {
-		nouveau_event_trigger(priv->base.vblank, 0);
+		if (priv->base.vblank)
+			nouveau_event_trigger(priv->base.vblank, 0);
 		nv_wr32(priv, 0x610024, 0x00000004);
 		intr1 &= ~0x00000004;
 	}
 
 	if (intr1 & 0x00000008) {
-		nouveau_event_trigger(priv->base.vblank, 1);
+		if (priv->base.vblank)
+			nouveau_event_trigger(priv->base.vblank, 1);
 		nv_wr32(priv, 0x610024, 0x00000008);
 		intr1 &= ~0x00000008;
 	}
diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
index 52dd7a1..4095f65 100644
--- a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
+++ b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
@@ -941,7 +941,7 @@ nvd0_disp_intr(struct nouveau_subdev *subdev)
 		u32 mask = 0x01000000 << i;
 		if (mask & intr) {
 			u32 stat = nv_rd32(priv, 0x6100bc + (i * 0x800));
-			if (stat & 0x00000001)
+			if ((stat & 0x00000001) && priv->base.vblank)
 				nouveau_event_trigger(priv->base.vblank, i);
 			nv_mask(priv, 0x6100bc + (i * 0x800), 0, 0);
 			nv_rd32(priv, 0x6100c0 + (i * 0x800));

Fixes vgaswitcheroo on a card without display.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
---
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 4c1bc06..8f6d63d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -398,7 +398,8 @@ void
 nouveau_fbcon_output_poll_changed(struct drm_device *dev)
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
-	drm_fb_helper_hotplug_event(&drm->fbcon->helper);
+	if (drm->fbcon)
+		drm_fb_helper_hotplug_event(&drm->fbcon->helper);
 }
 
 static int

Fixes some dmabuf object errors on nv50 chipset and below.

Cc: stable at vger.kernel.org [3.7+]
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
---
Xorg still won't start with vblank enabled on nv50, I have no idea why yet.

diff --git a/drivers/gpu/drm/nouveau/nv17_fence.c
b/drivers/gpu/drm/nouveau/nv17_fence.c
index 37a032b..260f3c5 100644
--- a/drivers/gpu/drm/nouveau/nv17_fence.c
+++ b/drivers/gpu/drm/nouveau/nv17_fence.c
@@ -76,7 +76,7 @@ nv17_fence_context_new(struct nouveau_channel *chan)
 	struct ttm_mem_reg *mem = &priv->bo->bo.mem;
 	struct nouveau_object *object;
 	u32 start = mem->start * PAGE_SIZE;
-	u32 limit = mem->start + mem->size - 1;
+	u32 limit = start + mem->size - 1;
 	int ret = 0;
 
 	fctx = chan->fence = kzalloc(sizeof(*fctx), GFP_KERNEL);
diff --git a/drivers/gpu/drm/nouveau/nv50_fence.c
b/drivers/gpu/drm/nouveau/nv50_fence.c
index ada4d6d..60afa5c 100644
--- a/drivers/gpu/drm/nouveau/nv50_fence.c
+++ b/drivers/gpu/drm/nouveau/nv50_fence.c
@@ -51,26 +51,27 @@ nv50_fence_context_new(struct nouveau_channel *chan)
 	fctx->base.sync = nv17_fence_sync;
 
 	ret = nouveau_object_new(nv_object(chan->cli), chan->handle,
-				 NvSema, 0x0002,
+				 NvSema, 0x003d,
 				 &(struct nv_dma_class) {
 					.flags = NV_DMA_TARGET_VRAM |
 						 NV_DMA_ACCESS_RDWR,
 					.start = mem->start * PAGE_SIZE,
-					.limit = mem->size - 1,
+					.limit = mem->start * PAGE_SIZE + mem->size - 1,
 				 }, sizeof(struct nv_dma_class),
 				 &object);
 
 	/* dma objects for display sync channel semaphore blocks */
 	for (i = 0; !ret && i < dev->mode_config.num_crtc; i++) {
 		struct nouveau_bo *bo = nv50_display_crtc_sema(dev, i);
+		mem = &bo->bo.mem;
 
 		ret = nouveau_object_new(nv_object(chan->cli), chan->handle,
 					 NvEvoSema0 + i, 0x003d,
 					 &(struct nv_dma_class) {
 						.flags = NV_DMA_TARGET_VRAM |
 							 NV_DMA_ACCESS_RDWR,
-						.start = bo->bo.offset,
-						.limit = bo->bo.offset + 0xfff,
+						.start = mem->start * PAGE_SIZE,
+						.limit = mem->start * PAGE_SIZE + mem->size - 1,
 					 }, sizeof(struct nv_dma_class),
 					 &object);
 	}

On Tue, Jul 23, 2013 at 11:43 PM, Maarten Lankhorst
<maarten.lankhorst at canonical.com> wrote:
> Sort of fixes mmiotrace for me again, I could sear I sent a similar patch
before
> the rework to event interface, so I guess it got reintroduced.
I don't know how/why you think this fixes anything.  The interrupt
handler can't possibly be called until after priv->base.vblank has
been initialised...

Seriously, go look, the subdev interrupt handler pointer isn't filled
in (and can't be) until after nouveau_disp_create() has been called,
and nouveau_disp_create() initialises priv->base.vblank before it
returns...

Ben.

>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
> ---
> diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
> index 7e3875d..35e526b 100644
> --- a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
> +++ b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c
> @@ -1266,13 +1266,15 @@ nv50_disp_intr(struct nouveau_subdev *subdev)
>         }
>
>         if (intr1 & 0x00000004) {
> -               nouveau_event_trigger(priv->base.vblank, 0);
> +               if (priv->base.vblank)
> +                       nouveau_event_trigger(priv->base.vblank, 0);
>                 nv_wr32(priv, 0x610024, 0x00000004);
>                 intr1 &= ~0x00000004;
>         }
>
>         if (intr1 & 0x00000008) {
> -               nouveau_event_trigger(priv->base.vblank, 1);
> +               if (priv->base.vblank)
> +                       nouveau_event_trigger(priv->base.vblank, 1);
>                 nv_wr32(priv, 0x610024, 0x00000008);
>                 intr1 &= ~0x00000008;
>         }
> diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
> index 52dd7a1..4095f65 100644
> --- a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
> +++ b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c
> @@ -941,7 +941,7 @@ nvd0_disp_intr(struct nouveau_subdev *subdev)
>                 u32 mask = 0x01000000 << i;
>                 if (mask & intr) {
>                         u32 stat = nv_rd32(priv, 0x6100bc + (i * 0x800));
> -                       if (stat & 0x00000001)
> +                       if ((stat & 0x00000001) &&
priv->base.vblank)
>                                 nouveau_event_trigger(priv->base.vblank,
i);
>                         nv_mask(priv, 0x6100bc + (i * 0x800), 0, 0);
>                         nv_rd32(priv, 0x6100c0 + (i * 0x800));
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Op 30-07-13 04:42, Ben Skeggs schreef:
> On Tue, Jul 23, 2013 at 11:43 PM, Maarten Lankhorst
> <maarten.lankhorst at canonical.com> wrote:
>> Sort of fixes mmiotrace for me again, I could sear I sent a similar
patch before
>> the rework to event interface, so I guess it got reintroduced.
> I don't know how/why you think this fixes anything.  The interrupt
> handler can't possibly be called until after priv->base.vblank has
> been initialised...
>
> Seriously, go look, the subdev interrupt handler pointer isn't filled
> in (and can't be) until after nouveau_disp_create() has been called,
> and nouveau_disp_create() initialises priv->base.vblank before it
> returns...
>
There's also a disp_dtor function right above it and without a smp_wmb
the writes could still be reordered in the constructor. O:-) A spinlock would
be needed to make sure that no interrupt is in process if you set intr to null
before destroying vblank event, though.

I can probably try to get the oops again indicating that priv->base.vblank
was null in the interrupt handler if you want,
iirc it happened reliably during mmiotracing.

~Maarten