On 05/29/2015 07:35 PM, Daniel Vetter wrote:
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@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.
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
The diff on top of your patch is now tested and helps. suspend->resume
is now fine on nv50. In your patch, nouveau_display_resume() would also
need to get a now unused "int head" removed to make the compiler happy.
-mario
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) {
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
