From patchwork Wed Jul 20 21:00:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: cpaul@redhat.com X-Patchwork-Id: 9240599 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 81546600CB for ; Wed, 20 Jul 2016 21:01:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7462127C05 for ; Wed, 20 Jul 2016 21:01:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6931327C38; Wed, 20 Jul 2016 21:01:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2C06A27C05 for ; Wed, 20 Jul 2016 21:01:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 012C56E8F0; Wed, 20 Jul 2016 21:00:59 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 001836E8E8; Wed, 20 Jul 2016 21:00:32 +0000 (UTC) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0EEBC769E6; Wed, 20 Jul 2016 21:00:32 +0000 (UTC) Received: from ecstaticemu.bos.redhat.com (dhcp-25-142.bos.redhat.com [10.18.25.142]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6KL0PFl017874; Wed, 20 Jul 2016 17:00:30 -0400 From: Lyude To: intel-gfx@lists.freedesktop.org Subject: [PATCH 4/6] drm/i915/skl: Always wait for pipes to update after a flush Date: Wed, 20 Jul 2016 17:00:00 -0400 Message-Id: <1469048403-32016-5-git-send-email-cpaul@redhat.com> In-Reply-To: <1469048403-32016-1-git-send-email-cpaul@redhat.com> References: <1469048403-32016-1-git-send-email-cpaul@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 20 Jul 2016 21:00:32 +0000 (UTC) Cc: Radhakrishna Sripada , "open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list" , stable@vger.kernel.org, Daniel Vetter , Lyude 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP As we've learned, all watermark updates on Skylake have to be strictly atomic or things fail. While the bspec doesn't mandate that we need to wait for pipes to finish after the third iteration of flushes, not doing so gives us the opportunity to break this atomicity later. This example assumes that we're lucky enough not to be interrupted by the scheduler at any point during this: - Start with pipe A and pipe B enabled - Enable pipe C - Flush pipe A in pass 1, wait until update finishes - Flush pipe B in pass 3, continue without waiting for next vblank - Start another wm update - We enter the next vblank for pipe B before we finish writing all the vm values - *Underrun* As such, we always need to wait for each pipe we flush to update so as to never break this atomicity. Fixes: 0e8fb7ba7ca5 ("drm/i915/skl: Flush the WM configuration") Signed-off-by: Lyude Cc: stable@vger.kernel.org Cc: Ville Syrjälä Cc: Daniel Vetter Cc: Radhakrishna Sripada Cc: Hans de Goede Cc: Matt Roper --- drivers/gpu/drm/i915/intel_pm.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 788db86..2e31df4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3859,8 +3859,11 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv, /* * Third pass: flush the pipes that got more space allocated. * - * We don't need to actively wait for the update here, next vblank - * will just get more DDB space with the correct WM values. + * While the hardware doesn't require to wait for the next vblank here, + * continuing before the pipe finishes updating could result in us + * trying to update the wm values again before the pipe finishes + * updating, which results in the hardware using intermediate wm values + * and subsequently underrunning pipes. */ for_each_intel_crtc(dev, crtc) { if (!crtc->active) @@ -3876,6 +3879,16 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv, continue; skl_wm_flush_pipe(dev_priv, pipe, 3); + + /* + * The only time we can get away with not waiting for an update + * is when we just enabled the pipe, e.g. when it doesn't have + * vblanks enabled anyway. + */ + if (drm_crtc_vblank_get(&crtc->base) == 0) { + intel_wait_for_vblank(dev, pipe); + drm_crtc_vblank_put(&crtc->base); + } } }