Re: [PATCH 1/3] drm/nouveau: Use drm_vblank_on/off consistently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 05/29/2015 07:19 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 06:50:06PM +0200, Mario Kleiner wrote:
On 05/27/2015 11:04 AM, Daniel Vetter wrote:
In

commit 9cba5efab5a8145ae6c52ea273553f069c294482
Author: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
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.

Cc: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
  drivers/gpu/drm/nouveau/dispnv04/crtc.c   | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_display.c | 4 ----
  2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 3d96b49fe662..dab24066fa21 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);
  }

The above hunk is probably correct, but i couldn't test it without
sufficiently old pre-nv 50 hardware.


  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 8670d90cdc11..d824023f9fc6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -620,10 +620,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.


Tested this one and this hunk breaks suspend/resume. After a suspend/resume
cycle, all OpenGL apps and composited desktop are dead, as the core can't
get any vblank irq's enabled anymore.

So the drm_vblank_on() is still needed here.

Hm that's very surprising. As mentioned above the force_mode_restore
should be calling nv_crtc_prepare already and fix this all up for us. I
guess I need to dig out my nv card and trace what's really going on here.

Enabling interrupts when the crtc is off isn't a good idea.
-Daniel


I think the nv_crtc_prepare() path modified in your first hunk is only for the original nv04 display engine for very old cards. nv50+ (GeForce-8 and later) take different paths.

-mario
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux