[PATCH 0/2] drm/nouveau: Fix cursor-related display hang

[PATCH 0/2] drm/nouveau: Fix cursor-related display hang

[Lyude Paul]

This is a fix for one of the many issues I've been finding on my brand
new desktop. This one in particular is easy to hit if you have a cursor
with a continously updating surface between two displays, what fun!

Reminds me of a another bug from long ago…

Lyude Paul (2):
  drm/nouveau/disp/nv50-: Set lock_core in curs507a_prepare
  drm/nouveau/kms/nv50-: Assert we hold nv50_disp->lock in
    nv50_head_flush_*

 drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 1 +
 drivers/gpu/drm/nouveau/dispnv50/head.c     | 5 +++++
 2 files changed, 6 insertions(+)


base-commit: 8e7460eac786c72f48c4e04ce9be692b939428ce
-- 
2.52.0

[PATCH 1/2] drm/nouveau/disp/nv50-: Set lock_core in curs507a_prepare

[Lyude Paul]

For a while, I've been seeing a strange issue where some (usually not all)
of the display DMA channels will suddenly hang, particularly when there is
a visible cursor on the screen that is being frequently updated, and
especially when said cursor happens to go between two screens. While this
brings back lovely memories of fixing Intel Skylake bugs, I would quite
like to fix it :).

It turns out the problem that's happening here is that we're managing to
reach nv50_head_flush_set() in our atomic commit path without actually
holding nv50_disp->mutex. This means that cursor updates happening in
parallel (along with any other atomic updates that need to use the core
channel) will race with eachother, which eventually causes us to corrupt
the pushbuffer - leading to a plethora of various GSP errors, usually:

  nouveau 0000:c1:00.0: gsp: Xid:56 CMDre 00000000 00000218 00102680 00000004 00800003
  nouveau 0000:c1:00.0: gsp: Xid:56 CMDre 00000000 0000021c 00040509 00000004 00000001
  nouveau 0000:c1:00.0: gsp: Xid:56 CMDre 00000000 00000000 00000000 00000001 00000001

The reason this is happening is because generally we check whether we need
to set nv50_atom->lock_core at the end of nv50_head_atomic_check().
However, curs507a_prepare is called from the fb_prepare callback, which
happens after the atomic check phase. As a result, this can lead to commits
that both touch the core channel but also don't grab nv50_disp->mutex.

So, fix this by making sure that we set nv50_atom->lock_core in
cus507a_prepare().

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 1590700d94ac ("drm/nouveau/kms/nv50-: split each resource type into their own source files")
Cc: <stable@vger.kernel.org> # v4.18+
---
 drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
index a95ee5dcc2e39..1a889139cb053 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/curs507a.c
@@ -84,6 +84,7 @@ curs507a_prepare(struct nv50_wndw *wndw, struct nv50_head_atom *asyh,
 		asyh->curs.handle = handle;
 		asyh->curs.offset = offset;
 		asyh->set.curs = asyh->curs.visible;
+		nv50_atom(asyh->state.state)->lock_core = true;
 	}
 }
 
-- 
2.52.0

[PATCH 2/2] drm/nouveau/kms/nv50-: Assert we hold nv50_disp->lock in nv50_head_flush_*

[Lyude Paul]

Now that we've had one bug that occurred in nouveau as the result of
nv50_head_flush_* being called without the appropriate locks, let's add
some lockdep asserts to make sure this doesn't happen in the future.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nouveau/dispnv50/head.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 3dd742b4f8232..e32ed1db6c566 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -43,6 +43,9 @@ nv50_head_flush_clr(struct nv50_head *head,
 	union nv50_head_atom_mask clr = {
 		.mask = asyh->clr.mask & ~(flush ? 0 : asyh->set.mask),
 	};
+
+	lockdep_assert_held(&head->disp->mutex);
+
 	if (clr.crc)  nv50_crc_atomic_clr(head);
 	if (clr.olut) head->func->olut_clr(head);
 	if (clr.core) head->func->core_clr(head);
@@ -65,6 +68,8 @@ nv50_head_flush_set_wndw(struct nv50_head *head, struct nv50_head_atom *asyh)
 void
 nv50_head_flush_set(struct nv50_head *head, struct nv50_head_atom *asyh)
 {
+	lockdep_assert_held(&head->disp->mutex);
+
 	if (asyh->set.view   ) head->func->view    (head, asyh);
 	if (asyh->set.mode   ) head->func->mode    (head, asyh);
 	if (asyh->set.core   ) head->func->core_set(head, asyh);
-- 
2.52.0

Re: [PATCH 0/2] drm/nouveau: Fix cursor-related display hang

[Dave Airlie]

Both look good to me,

Reviewed-by: Dave Airlie <airlied@redhat.com>

On Sat, 20 Dec 2025 at 07:53, Lyude Paul <lyude@redhat.com> wrote:
>
> This is a fix for one of the many issues I've been finding on my brand
> new desktop. This one in particular is easy to hit if you have a cursor
> with a continously updating surface between two displays, what fun!
>
> Reminds me of a another bug from long ago…
>
> Lyude Paul (2):
>   drm/nouveau/disp/nv50-: Set lock_core in curs507a_prepare
>   drm/nouveau/kms/nv50-: Assert we hold nv50_disp->lock in
>     nv50_head_flush_*
>
>  drivers/gpu/drm/nouveau/dispnv50/curs507a.c | 1 +
>  drivers/gpu/drm/nouveau/dispnv50/head.c     | 5 +++++
>  2 files changed, 6 insertions(+)
>
>
> base-commit: 8e7460eac786c72f48c4e04ce9be692b939428ce
> --
> 2.52.0
>