On Fri, May 29, 2015 at 07:23:35PM +0200, Mario Kleiner wrote:
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@gmail.com Date: Tue Jul 29 02:36:44 2014 +0200
drm/nouveau: Dis/Enable vblank irqs during suspend/resumedrm_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@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 | 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.
Oh right totally missed the nv50+ code. I only grepped for pre/post_modeset ...
Below untested diff should help. I also realized that the pre-nv50 code lacks drm_vblank_on/off in the dpms callback, so there's more work to do anyway for this one here.
Thanks, Daniel
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c index 7da7958556a3..a16c37d8f7e1 100644 --- a/drivers/gpu/drm/nouveau/nv50_display.c +++ b/drivers/gpu/drm/nouveau/nv50_display.c @@ -997,6 +997,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 @@ -1036,6 +1040,8 @@ nv50_crtc_prepare(struct drm_crtc *crtc) }
nv50_crtc_cursor_show_hide(nv_crtc, false, false); + + drm_crtc_vblank_on(crtc); }
static void @@ -1045,6 +1051,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) {