Re: [Nouveau] [PATCH] drm/nouveau: gk20a: Turn instmem lock into mutex

2017-02-23 Thread Alexandre Courbot

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 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?


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

2017-02-23 Thread bugzilla-daemon
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.

2017-02-23 Thread Maarten Lankhorst
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

2017-02-23 Thread Thierry Reding
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

2017-02-23 Thread bugzilla-daemon
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

2017-02-23 Thread bugzilla-daemon
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

2017-02-23 Thread bugzilla-daemon
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

2017-02-23 Thread bugzilla-daemon
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

2017-02-23 Thread bugzilla-daemon
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 Skeggs 
Date:   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

2017-02-23 Thread bugzilla-daemon
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 Skeggs 
Date:   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

2017-02-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=99913

Pekka Paalanen  changed:

   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

2017-02-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=84515

Pekka Paalanen  changed:

   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

2017-02-23 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=99913

Pekka Paalanen  changed:

   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