From patchwork Thu Jan 21 08:16:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mario Kleiner X-Patchwork-Id: 8079231 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 261029F6FA for ; Thu, 21 Jan 2016 08:16:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1AD07205D4 for ; Thu, 21 Jan 2016 08:16:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2652620390 for ; Thu, 21 Jan 2016 08:16:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 931646E1F7; Thu, 21 Jan 2016 00:16:42 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7CA2B6E944 for ; Thu, 21 Jan 2016 00:16:39 -0800 (PST) Received: by mail-wm0-f66.google.com with SMTP id u188so9135301wmu.0 for ; Thu, 21 Jan 2016 00:16:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=j9tOpnhiDfdN/JUQeH0qi8Iun3xQC8zWhQXczaVVqLs=; b=n9cSicrN4UVEwELbjJ7aL+p5/v1kxvcWtb9HE1nf348+f8a0cs5WdQ22oXREupb6+l fXxUEqZGSMET/lqMFy3EFAo3I6B0ARrJaYSOnv0oqnFo4BblReg4UGHXE9jNVHO7F8pt fJNhq272xFPuCpqPVNaqn8/fwYNd4Qwdw3ZwbbYbkY+fejY/ZS811gFzOvKrxjl6QhaZ 7F/rhGOFw4oxR2x+82yQO8VH7hVg1ZHKn/XK+EgJG+fJik6tsSDuL1Ej0YH+qzhdaBvg MEBGDyBXaUE4P44tgw+2Or2q/SRXhSwXSlF253UDj1fS/pKjPeq2R92lC4O0/5cadhG6 uvmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=j9tOpnhiDfdN/JUQeH0qi8Iun3xQC8zWhQXczaVVqLs=; b=ViRpsWS+t08dPfdJl1+iXVZWDXK9hJu1z7riC10XL47gTa6ghxugNTxV3MLkL6CIx5 Lrtwju+LwLmWK5Mt8cKjhgXW37DGUwl2+nrlaN0f30HkSAi1JyXP0v9fKwtBNRmjQA5A uUDOls3eVBtiT1JqJvuWabZBOqxkfPyjJ9KqhFYSTWQkr/6THmgnpgu/iUQ0/vyYp64Q MaXGAPqeykGqAh7t6rmDrBq0zEdTBKXj6TPeE3QgI4F19CUIP6vqdd1fGCta/iGhEttX OeQtnAr/o6SzDT1mPrOC6kbFVcqmfnpz46ZXDHG9Fh1pHoI6/bcry2M18ZtB4ST6A3Lj AwvQ== X-Gm-Message-State: AG10YOTGVamTMXcX8CHFYNMNZ+P0MPq4yIzrSv/MmVJkpu2k5z3IVVj4ReN21HXmgBbHKg== X-Received: by 10.28.230.92 with SMTP id d89mr9155867wmh.12.1453364198198; Thu, 21 Jan 2016 00:16:38 -0800 (PST) Received: from twisty.fritz.box (x5f702070.dyn.telefonica.de. [95.112.32.112]) by smtp.gmail.com with ESMTPSA id r4sm208612wjy.39.2016.01.21.00.16.35 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 21 Jan 2016 00:16:36 -0800 (PST) From: Mario Kleiner To: mario.kleiner.de@gmail.com Subject: [PATCH 1/2] drm/radeon: Use drm_vblank_off/on to fix vblank counter trouble. Date: Thu, 21 Jan 2016 09:16:13 +0100 Message-Id: <1453364174-11248-1-git-send-email-mario.kleiner.de@gmail.com> X-Mailer: git-send-email 1.9.1 Cc: daniel.vetter@ffwll.ch, michel@daenzer.net, dri-devel@lists.freedesktop.org, vbabka@suse.cz, alexander.deucher@amd.com, christian.koenig@amd.com X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Signed-off-by: Mario Kleiner 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); } } }