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@suse.cz Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: michel@daenzer.net Cc: vbabka@suse.cz Cc: ville.syrjala@linux.intel.com Cc: daniel.vetter@ffwll.ch Cc: dri-devel@lists.freedesktop.org Cc: alexander.deucher@amd.com Cc: christian.koenig@amd.com --- 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); } } }