Re: HEADS UP: Merging drm update

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

Can you try the attached patch?  Also, if you haven't already, can you
try running a DIAGNOSTIC/DEBUG/LOCKDEBUG kernel?

From 025745d6f43aff21c88bc9e3b393ce146fc9af04 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 26 Dec 2021 16:05:20 +0000
Subject: [PATCH] drm: Fix locking around accurate vblank counts.

- Make drm_crtc_accurate_vblank_count require the caller to hold the
  event lock, rather than take it internally.

- Fix locking around drm_crtc_accurate_vblank_count and related
  operations in amdgpu and nouveau interrupt handlers.

- Use drm_crtc_vblank_put_locked, not drm_crtc_vblank_put, when we
  already hold the event lock.
---
 .../dist/drm/amd/display/amdgpu_dm/amdgpu_dm.c    |  2 +-
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c     |  2 ++
 sys/external/bsd/drm2/dist/drm/drm_vblank.c       | 15 ++-------------
 .../drm/nouveau/dispnv50/nouveau_dispnv50_disp.c  |  2 +-
 4 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c8cbf7a6f68e..d0386f514457 100644
--- a/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -361,7 +361,7 @@ static void dm_pflip_high_irq(void *interrupt_params)
 			drm_crtc_send_vblank_event(&amdgpu_crtc->base, e);
 
 			/* Event sent, so done with vblank for this flip */
-			drm_crtc_vblank_put(&amdgpu_crtc->base);
+			drm_crtc_vblank_put_locked(&amdgpu_crtc->base);
 		}
 	} else if (e) {
 		/* VRR active and inside front-porch: vblank count and
diff --git a/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
index 79cb94ea67dc..5a154037c524 100644
--- a/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
+++ b/sys/external/bsd/drm2/dist/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c
@@ -319,7 +319,9 @@ void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc)
 				       &crcs[0], &crcs[1], &crcs[2]))
 			return;
 
+		spin_lock(&crtc->dev->event_lock);
 		drm_crtc_add_crc_entry(crtc, true,
 				       drm_crtc_accurate_vblank_count(crtc), crcs);
+		spin_unlock(&crtc->dev->event_lock);
 	}
 }
diff --git a/sys/external/bsd/drm2/dist/drm/drm_vblank.c b/sys/external/bsd/drm2/dist/drm/drm_vblank.c
index 9ced81bd7ed8..ec91bbe9391b 100644
--- a/sys/external/bsd/drm2/dist/drm/drm_vblank.c
+++ b/sys/external/bsd/drm2/dist/drm/drm_vblank.c
@@ -337,7 +337,7 @@ static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe)
  * This is mostly useful for hardware that can obtain the scanout position, but
  * doesn't have a hardware frame counter.
  */
-static u64 drm_crtc_accurate_vblank_count_locked(struct drm_crtc *crtc)
+u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = drm_crtc_index(crtc);
@@ -358,17 +358,6 @@ static u64 drm_crtc_accurate_vblank_count_locked(struct drm_crtc *crtc)
 
 	return vblank;
 }
-
-u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc)
-{
-	u64 vblank;
-
-	spin_lock(&crtc->dev->event_lock);
-	vblank = drm_crtc_accurate_vblank_count_locked(crtc);
-	spin_unlock(&crtc->dev->event_lock);
-
-	return vblank;
-}
 EXPORT_SYMBOL(drm_crtc_accurate_vblank_count);
 
 static void __disable_vblank(struct drm_device *dev, unsigned int pipe)
@@ -972,7 +961,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
 	assert_spin_locked(&dev->event_lock);
 
 	e->pipe = pipe;
-	e->sequence = drm_crtc_accurate_vblank_count_locked(crtc) + 1;
+	e->sequence = drm_crtc_accurate_vblank_count(crtc) + 1;
 	list_add_tail(&e->base.link, &dev->vblank_event_list);
 }
 EXPORT_SYMBOL(drm_crtc_arm_vblank_event);
diff --git a/sys/external/bsd/drm2/dist/drm/nouveau/dispnv50/nouveau_dispnv50_disp.c b/sys/external/bsd/drm2/dist/drm/nouveau/dispnv50/nouveau_dispnv50_disp.c
index b9423cdc84fb..f49bb2ddf56d 100644
--- a/sys/external/bsd/drm2/dist/drm/nouveau/dispnv50/nouveau_dispnv50_disp.c
+++ b/sys/external/bsd/drm2/dist/drm/nouveau/dispnv50/nouveau_dispnv50_disp.c
@@ -2136,9 +2136,9 @@ nv50_disp_atomic_commit_tail(struct drm_atomic_state *state)
 		if (new_crtc_state->event) {
 			unsigned long flags;
 			/* Get correct count/ts if racing with vblank irq */
+			spin_lock_irqsave(&crtc->dev->event_lock, flags);
 			if (new_crtc_state->active)
 				drm_crtc_accurate_vblank_count(crtc);
-			spin_lock_irqsave(&crtc->dev->event_lock, flags);
 			drm_crtc_send_vblank_event(crtc, new_crtc_state->event);
 			spin_unlock_irqrestore(&crtc->dev->event_lock, flags);