The hardware vblank counter of AMD gpu's resets to zero during a
modeset. The new implementation of drm_update_vblank_count() from
commit 4dfd6486 "drm: Use vblank timestamps to guesstimate how
many vblanks were missed", introduced in Linux 4.4, treats that
as a counter wraparound and causes the software vblank counter
to jump forward by a large distance of up to 2^24 counts. This
interacts badly with 32-bit wraparound handling in
drm_handle_vblank_events(), causing that function to no longer
deliver pending vblank events to clients.
This leads to client hangs especially if clients perform OpenGL
or DRI3/Present animations while a modeset happens and triggers
the hw vblank counter reset. One prominent example is a hang of
KDE Plasma 5's startup progress splash screen during login, making
the KDE session unuseable.
Another small potential race exists when executing a modeset while
vblank interrupts are enabled or just get enabled: The modeset updates
radeon_crtc->lb_vblank_lead_lines during radeon_display_bandwidth_update,
so if vblank interrupt handling or enable would try to access that variable
multiple times at the wrong moment as part of drm_update_vblank_counter,
while the scanout happens to be within lb_vblank_lead_lines before the
start of vblank, it could cause inconsistent vblank counting and again
trigger a jump of the software vblank counter, causing similar client
hangs. The most easy way to avoid this small race is to not allow
vblank enable or vblank irq's during modeset.
This patch replaces calls to drm_vblank_pre/post_modeset in the
drivers dpms code with calls to drm_vblank_off/on, as recommended
for drivers with hw counters that reset to zero during modeset.
Those calls disable vblank interrupts during the modeset sequence
and reinitialize vblank counts and timestamps after the modeset
properly, taking hw counter reset into account, thereby fixing
the problem of forward jumping counters.
During a modeset, calls to drm_vblank_get() will no-op/intentionally
fail, so no vblank events or pageflips can be queued during modesetting.
Radeons static and dynpm power management uses drm_vblank_get to enable
vblank irqs to synchronize reclocking to start of vblank. If a modeset
would happen in parallel with such a power management action, drm_vblank_get
would be suppressed, sync to vblank wouldn't work and a visual glitch could
happen. However that glitch would hopefully be hidden by the blanking of
the crtc during modeset. A small fix to power management makes sure to
check for this and prevent unbalanced vblank reference counts due to
mismatched drm_vblank_get/put.
Reported-by: Vlastimil Babka <vbabka@xxxxxxx>
Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
Cc: michel@xxxxxxxxxxx
Cc: vbabka@xxxxxxx
Cc: ville.syrjala@xxxxxxxxxxxxxxx
Cc: daniel.vetter@xxxxxxxx
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Cc: alexander.deucher@xxxxxxx
Cc: christian.koenig@xxxxxxx
---
drivers/gpu/drm/radeon/atombios_crtc.c | 10 ++++++----
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 ++--
drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index 801dd60..1c853e0 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -275,23 +275,25 @@ void atombios_crtc_dpms(struct drm_crtc *crtc, int mode)
if (ASIC_IS_DCE3(rdev) && !ASIC_IS_DCE6(rdev))
atombios_enable_crtc_memreq(crtc, ATOM_ENABLE);
atombios_blank_crtc(crtc, ATOM_DISABLE);
- drm_vblank_post_modeset(dev, radeon_crtc->crtc_id);
+ /* adjust pm to dpms *before* drm_vblank_on */
+ radeon_pm_compute_clocks(rdev);
+ drm_vblank_on(dev, radeon_crtc->crtc_id);
radeon_crtc_load_lut(crtc);
break;
case DRM_MODE_DPMS_STANDBY:
case DRM_MODE_DPMS_SUSPEND:
case DRM_MODE_DPMS_OFF:
- drm_vblank_pre_modeset(dev, radeon_crtc->crtc_id);
+ drm_vblank_off(dev, radeon_crtc->crtc_id);
if (radeon_crtc->enabled)
atombios_blank_crtc(crtc, ATOM_ENABLE);
if (ASIC_IS_DCE3(rdev) && !ASIC_IS_DCE6(rdev))
atombios_enable_crtc_memreq(crtc, ATOM_DISABLE);
atombios_enable_crtc(crtc, ATOM_DISABLE);
radeon_crtc->enabled = false;
+ /* adjust pm to dpms *after* drm_vblank_off */
+ radeon_pm_compute_clocks(rdev);
break;
}
- /* adjust pm to dpms */
- radeon_pm_compute_clocks(rdev);
}
static void
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index 32b338f..24152df 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -331,13 +331,13 @@ static void radeon_crtc_dpms(struct drm_crtc *crtc, int mode)
RADEON_CRTC_DISP_REQ_EN_B));
WREG32_P(RADEON_CRTC_EXT_CNTL, crtc_ext_cntl, ~(mask | crtc_ext_cntl));
}
- drm_vblank_post_modeset(dev, radeon_crtc->crtc_id);
+ drm_vblank_on(dev, radeon_crtc->crtc_id);
radeon_crtc_load_lut(crtc);
break;
case DRM_MODE_DPMS_STANDBY:
case DRM_MODE_DPMS_SUSPEND:
case DRM_MODE_DPMS_OFF:
- drm_vblank_pre_modeset(dev, radeon_crtc->crtc_id);
+ drm_vblank_off(dev, radeon_crtc->crtc_id);
if (radeon_crtc->crtc_id)
WREG32_P(RADEON_CRTC2_GEN_CNTL, mask, ~(RADEON_CRTC2_EN | mask));
else {
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 59abebd..339a6c5 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -276,8 +276,12 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev)
if (rdev->irq.installed) {
for (i = 0; i < rdev->num_crtc; i++) {
if (rdev->pm.active_crtcs & (1 << i)) {
- rdev->pm.req_vblank |= (1 << i);
- drm_vblank_get(rdev->ddev, i);
+ /* This can fail if a modeset is in progress */
+ if (0 == drm_vblank_get(rdev->ddev, i))
+ rdev->pm.req_vblank |= (1 << i);
+ else
+ DRM_DEBUG_DRIVER("crtc %d no vblank, can glitch\n",
+ i);
}
}
}
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
