From patchwork Mon Feb 11 03:22:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mario Kleiner X-Patchwork-Id: 10805071 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2B57D6C2 for ; Mon, 11 Feb 2019 03:22:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 19ACF29AF5 for ; Mon, 11 Feb 2019 03:22:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0D8A129AF7; Mon, 11 Feb 2019 03:22:48 +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=-5.2 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 704BE29AF5 for ; Mon, 11 Feb 2019 03:22:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6D5FE6E13B; Mon, 11 Feb 2019 03:22:42 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm1-x342.google.com (mail-wm1-x342.google.com [IPv6:2a00:1450:4864:20::342]) by gabe.freedesktop.org (Postfix) with ESMTPS id BBA026E0EE; Mon, 11 Feb 2019 03:22:39 +0000 (UTC) Received: by mail-wm1-x342.google.com with SMTP id r17so14528640wmh.5; Sun, 10 Feb 2019 19:22:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=5Zf+bNqcnSOdY5DubBuACKrreVcvr0qL2xZNZJm/Yvw=; b=jTA7w/YfceMZdNuiXbPoEBOy6J8jqojB1amWqgiK5UzTB9WVNzxzOKWTmNpNoVD7UX J6QAQyF+7I19F90Bv8tLkAy7sSIOJsA0svG7518M/C4aIWxOQ6deN8jvNhtCWi58+/zq fmdEqdLJbp34IvOkaJ/X4SZhwKVlqzID7AekJfP4wxZjmYP6jqq6wKysO8h1NdWaJMdr oPQaFwvwewjfETgGLtEoTVNWhFkgY+mY5Sh4yJHTo1boNjcXdCxFUH9c8DUrjvGHegXv bCP63X24nWm2EFoLkdohs3v0L0WxoI5CDSUezAHwmC+sTEbOUyvqtVd6BN3N7iVE4RjH wusQ== X-Gm-Message-State: AHQUAuY+AT4ST7I1I08WFgQDdzZaTUJ2Hy5gl1PPwc2EHpFW6vue43tT thalTrtAIGGnBbdMVFazBAdCzLj/ X-Google-Smtp-Source: AHgI3IbahV8XDXCODwvKFiApW3iMTXHUoMGKxExNIugUfUeCyNXD6WhwaQJc36ic5G3o8Q0HP+mIbw== X-Received: by 2002:a5d:52c3:: with SMTP id r3mr6362723wrv.163.1549855358164; Sun, 10 Feb 2019 19:22:38 -0800 (PST) Received: from twisty.cin.medizin.uni-tuebingen.de ([2a01:c23:8429:3400:e9fd:d3f1:412c:598a]) by smtp.gmail.com with ESMTPSA id m21sm19379971wmi.43.2019.02.10.19.22.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 10 Feb 2019 19:22:37 -0800 (PST) From: Mario Kleiner To: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org Subject: [PATCH 1/3] drm/amdgpu: Fix get_crtc_scanoutpos behavior in vrr when vpos >= vtotal. Date: Mon, 11 Feb 2019 04:22:23 +0100 Message-Id: <20190211032225.9488-2-mario.kleiner.de@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190211032225.9488-1-mario.kleiner.de@gmail.com> References: <20190211032225.9488-1-mario.kleiner.de@gmail.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: michel@daenzer.net, stable@vger.kernel.org, Alex Deucher , Nicholas Kazlauskas MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit 520f08df45fbe300ed650da786a74093d658b7e1 ("drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal") While the explanation in that commit is correct wrt. the hardware counter sometimes returning a position >= vtotal in vrr mode if the query happens while we are inside the "extended front porch", the math without this commit still returns the desired *vpos value for achieving a proper vblank timestamping behavior in vrr mode, according to the kernel documentation about how vblank timestamps should behave in vrr mode. According to the vrr mode docs, the vblank timestamp (calculated by drm_calc_vbltimestamp_from_scanoutpos()) is always supposed to be the one corresponding to the end of vblank as if vblank duration is == minimum vblank duration under vrr == vblank duration of the regular fixed refresh rate mode timing. Iow. it must be the same timestamp as would show up in non-vrr mode, even if the current vblank may be much longer (by an unknown amount of time), due to an extended front porch duration. Doing the math on this shows that we get the correct *vpos values to feed into the drm_calc_vbltimestamp_from_scanoutpos() helper for this to happen by using exactly the same correction: *vpos = *vpos - vtotal - vbl_end Therefore we don't need any special case for *vpos >= vtotal and the special case would cause wrong timestamps. The only quirk compared to non-vrr mode is that this can return a *vpos >= 0 despite being in vblank, ie. while returning the DRM_SCANOUTPOS_IN_VBLANK flag. Auditing all callers of the function shows this doesn't matter, as they all use the dedicated DRM_SCANOUTPOS_IN_VBLANK flag to check for "in vblank", but not the sign of *vpos. This patch should make sure that amdgpu_display_get_crtc_scanoutpos() always returns a *vpos value which leads to a stable vblank timestamp, regardless where the scanout position is during the function call. This stability is important to not confuse the vblank_disable_immediate logic in DRM cores vblank counting, and to provide some stable timestamp for future improvements of the pageflip timestamping under vrr. Fixes: 520f08df45fb ("drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal") Signed-off-by: Mario Kleiner Cc: Cc: Nicholas Kazlauskas Cc: Harry Wentland Cc: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index 4e944737b708..13da4e3549f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -779,12 +779,22 @@ bool amdgpu_display_crtc_scaling_mode_fixup(struct drm_crtc *crtc, * Returns vpos as a positive number while in active scanout area. * Returns vpos as a negative number inside vblank, counting the number * of scanlines to go until end of vblank, e.g., -1 means "one scanline - * until start of active scanout / end of vblank." + * until start of active scanout / end of vblank." Note that in variable + * refresh rate mode (vrr), vpos may be positive inside the extended front + * porch, despite being inside vblank, and a negative number does not + * always define the number of scanline to true end of vblank. Instead + * the vpos values behave as if the crtc would operate in fixed refresh + * rate mode. This allows the drm_calc_vbltimestamp_from_scanoutpos() + * helper to calculate appropriate and stable vblank timestamps as specified + * for vrr mode - corresponding to the shortest vblank duration under vrr. + * In vrr mode therefore check the returned Flags for presence of + * DRM_SCANOUTPOS_IN_VBLANK to detect if the scanout is currently inside + * or outside true vblank. * * \return Flags, or'ed together as follows: * * DRM_SCANOUTPOS_VALID = Query successful. - * DRM_SCANOUTPOS_INVBL = Inside vblank. + * DRM_SCANOUTPOS_IN_VBLANK = Inside vblank. * DRM_SCANOUTPOS_ACCURATE = Returned position is accurate. A lack of * this flag means that returned position may be offset by a constant but * unknown small number of scanlines wrt. real scanout position. @@ -876,12 +886,7 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device *dev, /* Inside "upper part" of vblank area? Apply corrective offset if so: */ if (in_vbl && (*vpos >= vbl_start)) { vtotal = mode->crtc_vtotal; - - /* With variable refresh rate displays the vpos can exceed - * the vtotal value. Clamp to 0 to return -vbl_end instead - * of guessing the remaining number of lines until scanout. - */ - *vpos = (*vpos < vtotal) ? (*vpos - vtotal) : 0; + *vpos = *vpos - vtotal; } /* Correct for shifted end of vbl at vbl_end. */