Re: [Nouveau] [PATCH] drm/nouveau: gk20a: Turn instmem lock into mutex
On 02/24/2017 01:20 AM, Thierry Reding wrote: * PGP Signed by an unknown key On Mon, Jan 30, 2017 at 09:03:07PM +0100, Thierry Reding wrote: From: Thierry RedingThe gk20a implementation of instance memory uses vmap()/vunmap() to map memory regions into the kernel's virtual address space. These functions may sleep, so protecting them by a spin lock is not safe. This triggers a warning if the DEBUG_ATOMIC_SLEEP Kconfig option is enabled. Fix this by using a mutex instead. Signed-off-by: Thierry Reding --- drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) Alex, could you take a look at this? Sorry! Yes, using a mutex here should be safe since vmap() can sleep anyway. And I don't think this code can ever be reached in atomic context (Ben can confirm that last point). Tested this patch and it seems to work like a charm. Reviewed-by: Alexandre Courbot Tested-by: Alexandre Courbot Thanks, Thierry diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c index a6a7fa0d7679..7f5244d57d2f 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c @@ -94,7 +94,7 @@ struct gk20a_instmem { struct nvkm_instmem base; /* protects vaddr_* and gk20a_instobj::vaddr* */ - spinlock_t lock; + struct mutex lock; /* CPU mappings LRU */ unsigned int vaddr_use; @@ -184,11 +184,10 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory) struct gk20a_instmem *imem = node->base.imem; struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; const u64 size = nvkm_memory_size(memory); - unsigned long flags; nvkm_ltc_flush(ltc); - spin_lock_irqsave(>lock, flags); + mutex_lock(>lock); if (node->base.vaddr) { if (!node->use_cpt) { @@ -216,7 +215,7 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory) out: node->use_cpt++; - spin_unlock_irqrestore(>lock, flags); + mutex_unlock(>lock); return node->base.vaddr; } @@ -239,9 +238,8 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory) struct gk20a_instobj_iommu *node = gk20a_instobj_iommu(memory); struct gk20a_instmem *imem = node->base.imem; struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; - unsigned long flags; - spin_lock_irqsave(>lock, flags); + mutex_lock(>lock); /* we should at least have one user to release... */ if (WARN_ON(node->use_cpt == 0)) @@ -252,7 +250,7 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory) list_add_tail(>vaddr_node, >vaddr_lru); out: - spin_unlock_irqrestore(>lock, flags); + mutex_unlock(>lock); wmb(); nvkm_ltc_invalidate(ltc); @@ -306,19 +304,18 @@ gk20a_instobj_dtor_iommu(struct nvkm_memory *memory) struct gk20a_instmem *imem = node->base.imem; struct device *dev = imem->base.subdev.device->dev; struct nvkm_mm_node *r; - unsigned long flags; int i; if (unlikely(list_empty(>base.mem.regions))) goto out; - spin_lock_irqsave(>lock, flags); + mutex_lock(>lock); /* vaddr has already been recycled */ if (node->base.vaddr) gk20a_instobj_iommu_recycle_vaddr(node); - spin_unlock_irqrestore(>lock, flags); + mutex_unlock(>lock); r = list_first_entry(>base.mem.regions, struct nvkm_mm_node, rl_entry); @@ -580,7 +577,7 @@ gk20a_instmem_new(struct nvkm_device *device, int index, if (!(imem = kzalloc(sizeof(*imem), GFP_KERNEL))) return -ENOMEM; nvkm_instmem_ctor(_instmem, device, index, >base); - spin_lock_init(>lock); + mutex_init(>lock); *pimem = >base; /* do not allow more than 1MB of CPU-mapped instmem */ -- 2.11.0 * Unknown Key * 0x7F3EB3A1 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99922] [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99922 --- Comment #4 from Ben Skeggs--- Are you able to try with xf86-video-modesetting instead of the nouveau 2d driver and report if the issue still occurs? -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [Intel-gfx] [PATCH 1/3] drm/atomic: Make disable_all helper fully disable the crtc.
Op 23-02-17 om 16:03 schreef Sean Paul: > On Tue, Feb 21, 2017 at 02:51:40PM +0100, Maarten Lankhorst wrote: >> It seems that nouveau requires this, so best to do this in the helper. >> This allows nouveau to use the atomic suspend helper. > Pardon the stupid question, but why can't nouveau just do the right thing when > crtc_state->active becomes false? This patch is also required to clean up a connector leak in i915 driver unload as well, and also to clean up plane state destruction there. Nouveau needs it to unpin some framebuffers during suspend, but even if none of this was true, this is the right way to handle disable_all. Looking at DPMS is fragile. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/nouveau: gk20a: Turn instmem lock into mutex
On Mon, Jan 30, 2017 at 09:03:07PM +0100, Thierry Reding wrote: > From: Thierry Reding> > The gk20a implementation of instance memory uses vmap()/vunmap() to map > memory regions into the kernel's virtual address space. These functions > may sleep, so protecting them by a spin lock is not safe. This triggers > a warning if the DEBUG_ATOMIC_SLEEP Kconfig option is enabled. Fix this > by using a mutex instead. > > Signed-off-by: Thierry Reding > --- > drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c | 19 --- > 1 file changed, 8 insertions(+), 11 deletions(-) Alex, could you take a look at this? Thanks, Thierry > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c > index a6a7fa0d7679..7f5244d57d2f 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/instmem/gk20a.c > @@ -94,7 +94,7 @@ struct gk20a_instmem { > struct nvkm_instmem base; > > /* protects vaddr_* and gk20a_instobj::vaddr* */ > - spinlock_t lock; > + struct mutex lock; > > /* CPU mappings LRU */ > unsigned int vaddr_use; > @@ -184,11 +184,10 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory) > struct gk20a_instmem *imem = node->base.imem; > struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; > const u64 size = nvkm_memory_size(memory); > - unsigned long flags; > > nvkm_ltc_flush(ltc); > > - spin_lock_irqsave(>lock, flags); > + mutex_lock(>lock); > > if (node->base.vaddr) { > if (!node->use_cpt) { > @@ -216,7 +215,7 @@ gk20a_instobj_acquire_iommu(struct nvkm_memory *memory) > > out: > node->use_cpt++; > - spin_unlock_irqrestore(>lock, flags); > + mutex_unlock(>lock); > > return node->base.vaddr; > } > @@ -239,9 +238,8 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory) > struct gk20a_instobj_iommu *node = gk20a_instobj_iommu(memory); > struct gk20a_instmem *imem = node->base.imem; > struct nvkm_ltc *ltc = imem->base.subdev.device->ltc; > - unsigned long flags; > > - spin_lock_irqsave(>lock, flags); > + mutex_lock(>lock); > > /* we should at least have one user to release... */ > if (WARN_ON(node->use_cpt == 0)) > @@ -252,7 +250,7 @@ gk20a_instobj_release_iommu(struct nvkm_memory *memory) > list_add_tail(>vaddr_node, >vaddr_lru); > > out: > - spin_unlock_irqrestore(>lock, flags); > + mutex_unlock(>lock); > > wmb(); > nvkm_ltc_invalidate(ltc); > @@ -306,19 +304,18 @@ gk20a_instobj_dtor_iommu(struct nvkm_memory *memory) > struct gk20a_instmem *imem = node->base.imem; > struct device *dev = imem->base.subdev.device->dev; > struct nvkm_mm_node *r; > - unsigned long flags; > int i; > > if (unlikely(list_empty(>base.mem.regions))) > goto out; > > - spin_lock_irqsave(>lock, flags); > + mutex_lock(>lock); > > /* vaddr has already been recycled */ > if (node->base.vaddr) > gk20a_instobj_iommu_recycle_vaddr(node); > > - spin_unlock_irqrestore(>lock, flags); > + mutex_unlock(>lock); > > r = list_first_entry(>base.mem.regions, struct nvkm_mm_node, >rl_entry); > @@ -580,7 +577,7 @@ gk20a_instmem_new(struct nvkm_device *device, int index, > if (!(imem = kzalloc(sizeof(*imem), GFP_KERNEL))) > return -ENOMEM; > nvkm_instmem_ctor(_instmem, device, index, >base); > - spin_lock_init(>lock); > + mutex_init(>lock); > *pimem = >base; > > /* do not allow more than 1MB of CPU-mapped instmem */ > -- > 2.11.0 > signature.asc Description: PGP signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99922] [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99922 --- Comment #3 from kevin@potatofrom.space --- Created attachment 129872 --> https://bugs.freedesktop.org/attachment.cgi?id=129872=edit problematic xorg log on 4.10 with three monitors (Apologies for the email spam; I'm new to bugzilla. I'll refrain from posting any more attachments to avoid more spam. If you need the vbios/other dmesgs or Xorg logs, just ask.) -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99921] [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99921 kevin@potatofrom.space changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #1 from kevin@potatofrom.space --- *** This bug has been marked as a duplicate of bug 99922 *** -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99922] [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99922 --- Comment #2 from kevin@potatofrom.space --- *** Bug 99921 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99922] [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99922 --- Comment #1 from kevin@potatofrom.space --- Created attachment 129871 --> https://bugs.freedesktop.org/attachment.cgi?id=129871=edit problematic dmesg with three monitors on Linux 4.10 -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99922] New: [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99922 Bug ID: 99922 Summary: [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off Product: xorg Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Keywords: bisected Severity: normal Priority: medium Component: Driver/nouveau Assignee: nouveau@lists.freedesktop.org Reporter: kevin@potatofrom.space QA Contact: xorg-t...@lists.x.org On Linux 4.10, after waking monitors up from powersave mode, X displays only a black screen with a movable cursor on all three monitors. Linux 4.9.11 does not exhibit this issue. I initially noticed the issue with a triple-monitor setup, but it still occurs even if only one monitor is plugged in via HDMI. I've bisected the issue to the following atomic-mode-setting commit (full git bisect log attached): commit 839ca903f12ef8f09374e0b655456482536a733e Author: Ben SkeggsDate: Fri Nov 4 17:20:36 2016 +1000 drm/nouveau/kms/nv50: transition to atomic interfaces internally This commit implements the atomic commit interfaces, and implements the legacy modeset and page flipping interfaces on top of them. There's two major changes in behavior from before: - We're now making use of interlocks between core and satellite EVO channels, which greatly improves our ability to keep their states synchronised. - DPMS is now implemented as a full modeset to either tear down the entire pipe (or bring it back up). This choice was made mostly to ease the initial implementation, but I'm also not sure what we gain by bring backing the old behaviour. We shall see. This does NOT currently expose the atomic ioctl by default, due to limited testing having been performed. Signed-off-by: Ben Skeggs Dmesgs and Xorg logs will be provided for the following configurations that exhibit the issue: - 4.10, with one monitor attached - 4.10, with three monitors attached - A bisected revision that displays the issue (seems to have a bit more information) I'm using an MSI GTX 770 2GB; vbios will be attached. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99921] New: [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off
https://bugs.freedesktop.org/show_bug.cgi?id=99921 Bug ID: 99921 Summary: [NVE4] [regression, bisected] 4.10 (atomic): X shows black screen with cursor after monitors turn off Product: xorg Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Driver/nouveau Assignee: nouveau@lists.freedesktop.org Reporter: kevin@potatofrom.space QA Contact: xorg-t...@lists.x.org Created attachment 129870 --> https://bugs.freedesktop.org/attachment.cgi?id=129870=edit git bisect log On Linux 4.10, after waking monitors up from powersave mode, X displays only a black screen with a movable cursor on all three monitors. Linux 4.9.11 does not exhibit this issue. I initially noticed the issue with a triple-monitor setup, but it still occurs even if only one monitor is plugged in via HDMI. I've bisected the issue to the following atomic-mode-setting commit (full git bisect log attached): commit 839ca903f12ef8f09374e0b655456482536a733e Author: Ben SkeggsDate: Fri Nov 4 17:20:36 2016 +1000 drm/nouveau/kms/nv50: transition to atomic interfaces internally This commit implements the atomic commit interfaces, and implements the legacy modeset and page flipping interfaces on top of them. There's two major changes in behavior from before: - We're now making use of interlocks between core and satellite EVO channels, which greatly improves our ability to keep their states synchronised. - DPMS is now implemented as a full modeset to either tear down the entire pipe (or bring it back up). This choice was made mostly to ease the initial implementation, but I'm also not sure what we gain by bring backing the old behaviour. We shall see. This does NOT currently expose the atomic ioctl by default, due to limited testing having been performed. Signed-off-by: Ben Skeggs Dmesgs and Xorg logs are provided for the following configurations that exhibit the issue: - 4.10, with one monitor attached - 4.10, with three monitors attached - A bisected revision that displays the issue (seems to have a bit more information) I'm using an MSI GTX 770 2GB; vbios is attached. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99913] [G96] Reclocking fails on voltage
https://bugs.freedesktop.org/show_bug.cgi?id=99913 Pekka Paalanenchanged: What|Removed |Added Status|NEEDINFO|NEW --- Comment #7 from Pekka Paalanen --- Actually, I found out that there is an old dump from this laptop referred to in bug #60680: http://people.freedesktop.org/~pq/mmiotrace-fdo-bug-60680.tar.xz Does that help? -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 84515] [NV96] HDMI audio works, but has audio gaps or pauses
https://bugs.freedesktop.org/show_bug.cgi?id=84515 Pekka Paalanenchanged: What|Removed |Added Status|NEEDINFO|RESOLVED Resolution|--- |WORKSFORME --- Comment #6 from Pekka Paalanen --- After two and a half years, I have still not produced an mmiotrace. I also haven't done things that would have suffered from the gaps. So I suppose I can just close this report. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 99913] [G96] Reclocking fails on voltage
https://bugs.freedesktop.org/show_bug.cgi?id=99913 Pekka Paalanenchanged: What|Removed |Added Status|NEW |NEEDINFO --- Comment #6 from Pekka Paalanen --- Thanks, I'll have a look, but don't hold your breath. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau