[1/3] drm/nouveau: Use drm_vblank_on/off consistently
diff mbox

Message ID 1453370938-5464-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Jan. 21, 2016, 10:08 a.m. UTC
In

commit 9cba5efab5a8145ae6c52ea273553f069c294482
Author: Mario Kleiner <mario.kleiner.de@gmail.com>
Date:   Tue Jul 29 02:36:44 2014 +0200

    drm/nouveau: Dis/Enable vblank irqs during suspend/resume

drm_vblank_on/off calls where added around suspend/resume to make sure
vblank stay doesn't go boom over that transition. But nouveau already
used drm_vblank_pre/post_modeset over modesets. Instead use
drm_vblank_on/off everyhwere. The slight change here is that after
_off drm_vblank_get will refuse to work right away, but nouveau
doesn't seem to depend upon that anywhere outside of the pageflip
paths.

The longer-term plan here is to switch all kms drivers to
drm_vblank_on/off so that common code like pending event cleanup can
be done there, while drm_vblank_pre/post_modeset will be purely
drm internal for the old UMS ioctl.

Note that the drm_vblank_off still seems required in the suspend path
since nouveau doesn't explicitly disable crtcs. But on the resume side
drm_helper_resume_force_mode should end up calling drm_vblank_on
through the nouveau crtc hooks already. Hence remove the call in the
resume code.

v2: Don't forget about nv50+, reported by Mario.

Tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Tested-by: poma <pomidorabelisima@gmail.com>
Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c   | 4 ++--
 drivers/gpu/drm/nouveau/nouveau_display.c | 6 +-----
 drivers/gpu/drm/nouveau/nv50_display.c    | 8 ++++++++
 3 files changed, 11 insertions(+), 7 deletions(-)

Comments

Michel Dänzer Jan. 22, 2016, 2:14 a.m. UTC | #1
On 21.01.2016 19:08, Daniel Vetter wrote:
> In
> 
> commit 9cba5efab5a8145ae6c52ea273553f069c294482
> Author: Mario Kleiner <mario.kleiner.de@gmail.com>
> Date:   Tue Jul 29 02:36:44 2014 +0200
> 
>     drm/nouveau: Dis/Enable vblank irqs during suspend/resume
> 
> drm_vblank_on/off calls where added around suspend/resume to make sure
> vblank stay doesn't go boom over that transition. But nouveau already
> used drm_vblank_pre/post_modeset over modesets. Instead use
> drm_vblank_on/off everyhwere. The slight change here is that after
> _off drm_vblank_get will refuse to work right away, but nouveau
> doesn't seem to depend upon that anywhere outside of the pageflip
> paths.
> 
> The longer-term plan here is to switch all kms drivers to
> drm_vblank_on/off so that common code like pending event cleanup can
> be done there, while drm_vblank_pre/post_modeset will be purely
> drm internal for the old UMS ioctl.
> 
> Note that the drm_vblank_off still seems required in the suspend path
> since nouveau doesn't explicitly disable crtcs. But on the resume side
> drm_helper_resume_force_mode should end up calling drm_vblank_on
> through the nouveau crtc hooks already. Hence remove the call in the
> resume code.
> 
> v2: Don't forget about nv50+, reported by Mario.

I may be blind, but I can't see the DRI2 code in xf86-video-nouveau
dealing with DRM_IOCTL_WAIT_VBLANK returning an error during DPMS off /
modeset. Does it?

If not, AFAICT it would result in the xserver DRI2 code returning
protocol errors to clients, which I'm not sure they'll survive.

Patch
diff mbox

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 6f04397d43a7..bb9e9cb14b9d 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -708,7 +708,7 @@  static void nv_crtc_prepare(struct drm_crtc *crtc)
 	if (nv_two_heads(dev))
 		NVSetOwner(dev, nv_crtc->index);
 
-	drm_vblank_pre_modeset(dev, nv_crtc->index);
+	drm_vblank_off(dev, nv_crtc->index);
 	funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
 
 	NVBlankScreen(dev, nv_crtc->index, true);
@@ -740,7 +740,7 @@  static void nv_crtc_commit(struct drm_crtc *crtc)
 #endif
 
 	funcs->dpms(crtc, DRM_MODE_DPMS_ON);
-	drm_vblank_post_modeset(dev, nv_crtc->index);
+	drm_vblank_on(dev, nv_crtc->index);
 }
 
 static void nv_crtc_destroy(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 24be27d3cd18..b1fd86f83d69 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -599,7 +599,7 @@  nouveau_display_resume(struct drm_device *dev, bool runtime)
 {
 	struct nouveau_drm *drm = nouveau_drm(dev);
 	struct drm_crtc *crtc;
-	int ret, head;
+	int ret;
 
 	/* re-pin fb/cursors */
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
@@ -635,10 +635,6 @@  nouveau_display_resume(struct drm_device *dev, bool runtime)
 		nv_crtc->lut.depth = 0;
 	}
 
-	/* Make sure that drm and hw vblank irqs get resumed if needed. */
-	for (head = 0; head < dev->mode_config.num_crtc; head++)
-		drm_vblank_on(dev, head);
-
 	/* This should ensure we don't hit a locking problem when someone
 	 * wakes us up via a connector.  We should never go into suspend
 	 * while the display is on anyways.
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index ea3921652449..2ba2a145379b 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1023,6 +1023,10 @@  nv50_crtc_cursor_show_hide(struct nouveau_crtc *nv_crtc, bool show, bool update)
 static void
 nv50_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
+	if (mode == DRM_MODE_DPMS_ON)
+		drm_crtc_vblank_on(crtc);
+	else
+		drm_crtc_vblank_off(crtc);
 }
 
 static void
@@ -1062,6 +1066,8 @@  nv50_crtc_prepare(struct drm_crtc *crtc)
 	}
 
 	nv50_crtc_cursor_show_hide(nv_crtc, false, false);
+
+	drm_crtc_vblank_on(crtc);
 }
 
 static void
@@ -1071,6 +1077,8 @@  nv50_crtc_commit(struct drm_crtc *crtc)
 	struct nv50_mast *mast = nv50_mast(crtc->dev);
 	u32 *push;
 
+	drm_crtc_vblank_on(crtc);
+
 	push = evo_wait(mast, 32);
 	if (push) {
 		if (nv50_vers(mast) < G82_DISP_CORE_CHANNEL_DMA) {