drm/nouveau: fix vblank interrupt being called before event is setup
Subsystems (3)
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.
---
drivers/gpu/drm/nouveau/core/engine/disp/nv50.c +4 -2
@@ -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 ; }
drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c +1 -1
@@ -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 ));
PATCH 2/3 drm/nouveau: fix null pointer dereference in poll_changed
Fixes vgaswitcheroo on a card without display.
---
drivers/gpu/drm/nouveau/nouveau_fbcon.c +2 -1
@@ -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
PATCH 3/3 drm/nouveau: fix semaphore dmabuf obj
In reply to Maarten LankhorstFixes some dmabuf object errors on nv50 chipset and below.
---
Xorg still won't start with vblank enabled on nv50, I have no idea why yet.
drivers/gpu/drm/nouveau/nv17_fence.c +1 -1
@@ -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 );
drivers/gpu/drm/nouveau/nv50_fence.c +5 -4
@@ -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); }
Quoted 2 lines · Maarten Lankhorst
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.
Quoted 43 lines
Signed-off-by Maarten Lankhorst <[email protected]>---
drivers/gpu/drm/nouveau/core/engine/disp/nv50.c +4
-2@@ -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 ; }drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c +1
-1@@ -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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Op 30-07-13 04:42, Ben Skeggs schreef:
Quoted 4 lines
Quoted 2 lines · Maarten Lankhorst
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.
Quoted 9 lines
I don't know how/why you think this fixes anything. The interrupt handler can't possibly be called until after
priv->base.vblankhas 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, andnouveau_disp_create()initialisespriv->base.vblankbefore 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
Quoted 14 lines · Maarten Lankhorst
Op 30-07-13 04:42, Ben Skeggs schreef:
Quoted 4 lines
Quoted 2 lines · Maarten Lankhorst
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.
Quoted 9 lines
I don't know how/why you think this fixes anything. The interrupt handler can't possibly be called until after
priv->base.vblankhas 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, andnouveau_disp_create()initialisespriv->base.vblankbefore it returns...
Quoted 7 lines
There's also a
disp_dtorfunction right above it and without asmp_wmbthe 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.vblankwas 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.
Quoted 2 lines
~Maarten