From patchwork Wed May 18 12:02:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mario Kleiner X-Patchwork-Id: 9118171 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 16C279F37F for ; Wed, 18 May 2016 12:03:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D32ED202EC for ; Wed, 18 May 2016 12:03:30 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 9F26C20320 for ; Wed, 18 May 2016 12:03:29 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8155E88158; Wed, 18 May 2016 12:03:26 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3CC0688158 for ; Wed, 18 May 2016 12:03:24 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id r12so12314402wme.0 for ; Wed, 18 May 2016 05:03:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=2u7pozMkrCSOTeVeu6yL59iMsh9aC/Xsl3WDh3jxnqM=; b=heZEjqf8lWKvX5tU9vh1Dbe7LkkXAZysRzwn0hvOYmN/ZkAr5ELEjQelbJ0Bh5sBuh RtANPzdVDlzK4qCDQt9wbg+OqKcljPtbyJVOXE7gHyTcy+cn5X56GDorV7O/YboRQ/9s BRU5v6XVOExf+S4/wCkvChKjDwwnukJ1AZ51+H1TqHhUC73ls4Mp7F6P2S9S1vNi/pDy yNJrJwfRLTHYa4TW4gEb6qB1CFIIQ8MKo7QYGxF5Ua5sK3f/wmSGvp9PtTzDLnw6r9ab 5j8H1tmwyB0sIgmx5yBlVw56hpt560GEl8RM0QY1VErFihtbaKJ3fCi95jXwX3q9Lv68 wu5g== 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=2u7pozMkrCSOTeVeu6yL59iMsh9aC/Xsl3WDh3jxnqM=; b=FgcZekDGxi1bH63UUrLKzHLtifo1EM5kTPOeUJW1jUZiIJGts4OCtY7v1wnI/ezvcd 5Dr7v30Y67coVHP05SV/Pmv72vLhvwF3Un2S8da1OK72+dJj1GmH9N/LR5JkJ17QYQwc h9i++jIW+QHNyvhK0ClDKjTH8b+deNfbQSg51Ii+VID0Z3lGXniqzXBKM3CnZVInXXWJ A7O8C/+lmDZW00/U7IqV1cXMYwGTZVqH4Wwe1tkfR1vFwBnj370eF+Tw2GAL1JzyudLj vxyYvguPavr99RLprWyhiEphi1dDGoW/kpAo8Jf6fAsXtJVFPNblHHzKDFpjSiAGmmXN Ufgg== X-Gm-Message-State: AOPr4FVGCYrDA+hPzfx3hiUhKUmS7IOZXaL1YsPH860MNK8X3wKiQl0bnkAjsY+1gWIjWA== X-Received: by 10.194.221.37 with SMTP id qb5mr6662605wjc.171.1463573002725; Wed, 18 May 2016 05:03:22 -0700 (PDT) Received: from twisty.cin.medizin.uni-tuebingen.de (cin-11.medizin.uni-tuebingen.de. [134.2.118.242]) by smtp.gmail.com with ESMTPSA id gk4sm8314547wjd.7.2016.05.18.05.03.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 18 May 2016 05:03:22 -0700 (PDT) From: Mario Kleiner To: dri-devel@lists.freedesktop.org Subject: [PATCH] drm/vc4: Make pageflip completion handling more robust. Date: Wed, 18 May 2016 14:02:46 +0200 Message-Id: <1463572966-20205-1-git-send-email-mario.kleiner.de@gmail.com> X-Mailer: git-send-email 2.7.0 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=-5.5 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=ham 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 Protect both the setup of the pageflip event and the latching of the new requested displaylist head pointer by the event lock, so we can't get into a situation where vc4_atomic_flush latches the new display list via HVS_WRITE, then immediately gets preempted before queueing the pageflip event, then the page-flip completes in hw and the vc4_crtc_handle_page_flip() runs and no-ops due to lack of a pending pageflip event, then vc4_atomic_flush continues and only then queues the pageflip event - after the page flip handling already no-oped. This would cause flip completion handling only at the next vblank - one frame too late. In vc4_crtc_handle_page_flip() check the actual DL head pointer in SCALER_DISPLACTX against the requested pointer for page flip to make sure that the flip actually really completed in the current vblank and doesn't get deferred to the next one because the DL head pointer was written a bit too late into SCALER_DISPLISTX, after start of vblank, and missed the boat. This avoids handling a pageflip completion too early - one frame too early. According to Eric, DL head pointer updates which were written into the HVS DISPLISTX reg get committed to hardware at the last pixel of active scanout. Our vblank interrupt handler, as triggered by PV_INT_VFP_START irq, gets to run earliest at the first pixel of HBLANK at the end of the last scanline of active scanout, ie. vblank irq handling runs at least 1 pixel duration after a potential pageflip completion happened in hardware. This ordering of events in the hardware, together with the lock protection and SCALER_DISPLACTX sampling of this patch, guarantees that pageflip completion handling only runs at exactly the vblank irq of actual pageflip completion in all cases. Background info from Eric about the relative timing of HVS, PV's and trigger points for interrupts, DL updates: https://lists.freedesktop.org/archives/dri-devel/2016-May/107510.html Tested on RPi 2B with hardware timing measurement equipment and shown to no longer complete flips too early or too late. Signed-off-by: Mario Kleiner Cc: Eric Anholt Reviewed-by: Eric Anholt --- drivers/gpu/drm/vc4/vc4_crtc.c | 28 ++++++++++++++++++---------- drivers/gpu/drm/vc4/vc4_regs.h | 4 ++++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index c34f2bb..1981ca3 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -456,14 +456,6 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc, WARN_ON_ONCE(dlist_next - dlist_start != vc4_state->mm.size); - HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), - vc4_state->mm.start); - - if (debug_dump_regs) { - DRM_INFO("CRTC %d HVS after:\n", drm_crtc_index(crtc)); - vc4_hvs_dump_state(dev); - } - if (crtc->state->event) { unsigned long flags; @@ -473,8 +465,20 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc, spin_lock_irqsave(&dev->event_lock, flags); vc4_crtc->event = crtc->state->event; - spin_unlock_irqrestore(&dev->event_lock, flags); crtc->state->event = NULL; + + HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), + vc4_state->mm.start); + + spin_unlock_irqrestore(&dev->event_lock, flags); + } else { + HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), + vc4_state->mm.start); + } + + if (debug_dump_regs) { + DRM_INFO("CRTC %d HVS after:\n", drm_crtc_index(crtc)); + vc4_hvs_dump_state(dev); } } @@ -500,10 +504,14 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc *vc4_crtc) { struct drm_crtc *crtc = &vc4_crtc->base; struct drm_device *dev = crtc->dev; + struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); + u32 chan = vc4_crtc->channel; unsigned long flags; spin_lock_irqsave(&dev->event_lock, flags); - if (vc4_crtc->event) { + if (vc4_crtc->event && + (vc4_state->mm.start == HVS_READ(SCALER_DISPLACTX(chan)))) { drm_crtc_send_vblank_event(crtc, vc4_crtc->event); vc4_crtc->event = NULL; drm_crtc_vblank_put(crtc); diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h index 6163b95..f99eece 100644 --- a/drivers/gpu/drm/vc4/vc4_regs.h +++ b/drivers/gpu/drm/vc4/vc4_regs.h @@ -341,6 +341,10 @@ #define SCALER_DISPLACT0 0x00000030 #define SCALER_DISPLACT1 0x00000034 #define SCALER_DISPLACT2 0x00000038 +#define SCALER_DISPLACTX(x) (SCALER_DISPLACT0 + \ + (x) * (SCALER_DISPLACT1 - \ + SCALER_DISPLACT0)) + #define SCALER_DISPCTRL0 0x00000040 # define SCALER_DISPCTRLX_ENABLE BIT(31) # define SCALER_DISPCTRLX_RESET BIT(30)