* [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup
@ 2013-07-23 13:43 Maarten Lankhorst
2013-07-23 13:45 ` [PATCH 2/3] drm/nouveau: fix null pointer dereference in poll_changed Maarten Lankhorst
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2013-07-23 13:43 UTC (permalink / raw)
To: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
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@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));
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drm/nouveau: fix null pointer dereference in poll_changed
2013-07-23 13:43 [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Maarten Lankhorst
@ 2013-07-23 13:45 ` Maarten Lankhorst
2013-07-23 13:49 ` [PATCH 3/3] drm/nouveau: fix semaphore dmabuf obj Maarten Lankhorst
2013-07-30 2:42 ` [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Ben Skeggs
2 siblings, 0 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2013-07-23 13:45 UTC (permalink / raw)
To: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Fixes vgaswitcheroo on a card without display.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm/nouveau: fix semaphore dmabuf obj
2013-07-23 13:43 [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Maarten Lankhorst
2013-07-23 13:45 ` [PATCH 2/3] drm/nouveau: fix null pointer dereference in poll_changed Maarten Lankhorst
@ 2013-07-23 13:49 ` Maarten Lankhorst
2013-07-30 2:42 ` [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Ben Skeggs
2 siblings, 0 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2013-07-23 13:49 UTC (permalink / raw)
To: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Fixes some dmabuf object errors on nv50 chipset and below.
Cc: stable@vger.kernel.org [3.7+]
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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);
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup
2013-07-23 13:43 [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Maarten Lankhorst
2013-07-23 13:45 ` [PATCH 2/3] drm/nouveau: fix null pointer dereference in poll_changed Maarten Lankhorst
2013-07-23 13:49 ` [PATCH 3/3] drm/nouveau: fix semaphore dmabuf obj Maarten Lankhorst
@ 2013-07-30 2:42 ` Ben Skeggs
2013-07-30 6:10 ` Maarten Lankhorst
2 siblings, 1 reply; 6+ messages in thread
From: Ben Skeggs @ 2013-07-30 2:42 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
On Tue, Jul 23, 2013 at 11:43 PM, Maarten Lankhorst
<maarten.lankhorst@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup
2013-07-30 2:42 ` [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Ben Skeggs
@ 2013-07-30 6:10 ` Maarten Lankhorst
2013-07-30 6:13 ` Ben Skeggs
0 siblings, 1 reply; 6+ messages in thread
From: Maarten Lankhorst @ 2013-07-30 6:10 UTC (permalink / raw)
To: Ben Skeggs; +Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Op 30-07-13 04:42, Ben Skeggs schreef:
> On Tue, Jul 23, 2013 at 11:43 PM, Maarten Lankhorst
> <maarten.lankhorst@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup
2013-07-30 6:10 ` Maarten Lankhorst
@ 2013-07-30 6:13 ` Ben Skeggs
0 siblings, 0 replies; 6+ messages in thread
From: Ben Skeggs @ 2013-07-30 6:13 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
On Tue, Jul 30, 2013 at 4:10 PM, Maarten Lankhorst
<maarten.lankhorst@canonical.com> wrote:
> Op 30-07-13 04:42, Ben Skeggs schreef:
>> On Tue, Jul 23, 2013 at 11:43 PM, Maarten Lankhorst
>> <maarten.lankhorst@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.
It would've had to have happened during a module unload in that case,
I can't see how else it could've possibly happened.
The destructor case is valid though, but not really a critical issue,
so I'd rather find a better solution than these hacked checks :) I'll
add it to the list...
Ben.
>
> ~Maarten
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-30 6:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-23 13:43 [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Maarten Lankhorst
2013-07-23 13:45 ` [PATCH 2/3] drm/nouveau: fix null pointer dereference in poll_changed Maarten Lankhorst
2013-07-23 13:49 ` [PATCH 3/3] drm/nouveau: fix semaphore dmabuf obj Maarten Lankhorst
2013-07-30 2:42 ` [PATCH 1/3] drm/nouveau: fix vblank interrupt being called before event is setup Ben Skeggs
2013-07-30 6:10 ` Maarten Lankhorst
2013-07-30 6:13 ` Ben Skeggs
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.