From patchwork Thu Jun 6 08:44:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10978829 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 3583C76 for ; Thu, 6 Jun 2019 08:44:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2680828734 for ; Thu, 6 Jun 2019 08:44:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 19E8E28758; Thu, 6 Jun 2019 08:44:17 +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,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 9875E28734 for ; Thu, 6 Jun 2019 08:44:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E1FD389823; Thu, 6 Jun 2019 08:44:13 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id AE34589823 for ; Thu, 6 Jun 2019 08:44:12 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id c26so2200083edt.1 for ; Thu, 06 Jun 2019 01:44:12 -0700 (PDT) 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:mime-version :content-transfer-encoding; bh=8LivutWXtin3aFLst0DUTNZU5SUHL5LhX41frjZ0hno=; b=GbwniMfrjtl+xihy46AjSNOm6jwVF1PCMNILbAQ8e7Hwmted3l/bx/qgafVwfpw/Cp 2XW4oEoCHidRSErhAxmNXU2xfz+usXsCOQ/GVL6thOQ++UJ1jk8ymLWTVMLB3nWMI07P AN6QtghGtEsS7192MxJoSb6gB46ytN73PMWNXr62HQccklLCoKhZdf3yMQnN1xigAWaY GIFkS0nlxZq17plE1a2f0a0wrW6l1a5dtRJbo+2uZkH2fZospQbjYKP+c6GZiV8ecu1z YkpIaE0lG3pu6xpVa5hJDDijOCkcEixEwCtmcnQpMd8axFTeFMHPRfNYrLhY48/XayOV kYxw== X-Gm-Message-State: APjAAAVZoKOnnNjYCGewUCafZmM+/RZR8lxLoDylMcpXaidd5pjPFYEY DDgAO009N3Ci5/v6eoHbXJMR3zaKIOw= X-Google-Smtp-Source: APXvYqzAuhiv2py8d2tZ4c+uhkTmtJt0IA4br7Ux7gACGq0M2qN9pW2KgGwHRtEljRhCb8YU7hW74g== X-Received: by 2002:a50:91ef:: with SMTP id h44mr21418527eda.276.1559810651097; Thu, 06 Jun 2019 01:44:11 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id n14sm215208ejb.14.2019.06.06.01.44.10 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Thu, 06 Jun 2019 01:44:10 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH] drm/vkms: Forward timer right after drm_crtc_handle_vblank Date: Thu, 6 Jun 2019 10:44:04 +0200 Message-Id: <20190606084404.12014-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=8LivutWXtin3aFLst0DUTNZU5SUHL5LhX41frjZ0hno=; b=XFhTf+VGvz40bXb1MuWEk438g3QpD7yIV5GQa856hG5mXU15Qxvp+TRhS2F2aeVjLT gpS6zKCd5QVzJFWICMu2nkslalifnhqU0SIEbosnEzC6D7rkfKH9e/LuG4y9HDss3dHz B5Cd4YWYjKbm8uHgJ4lohIMJEijx1Bzk1eedc= 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: Daniel Vetter , Daniel Vetter , Rodrigo Siqueira , Shayenne Moura Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP In commit def35e7c592616bc09be328de8795e5e624a3cf8 Author: Shayenne Moura Date: Wed Jan 30 14:06:36 2019 -0200 drm/vkms: Bugfix extra vblank frame we fixed the vblank counter to give accurate results outside of drm_crtc_handle_vblank, which fixed bugs around vblank timestamps being off-by-one and causing the vblank counter to jump when it shouldn't. The trouble is that this completely broke crc generation. Shayenne and Rodrigo tracked this down to the vblank timestamp going backwards in time somehow. Which then resulted in an underflow in drm_vblank.c code, which resulted in all kinds of things breaking really badly. The reason for this is that once we've called drm_crtc_handle_vblank and the hrtimer isn't forwarded yet, we're returning a vblank timestamp in the past. This race is really hard to hit since it's small, except when you enable crc generation: In that case there's a call to drm_crtc_accurate_vblank right in-betwen, so we're guaranteed to hit the bug. The fix is to roll the hrtimer forward _before_ we do the vblank processing (which has a side-effect of incrementing the vblank counter), and we always subtract one frame from the hrtimer - since now it's always one frame in the future. To make sure we don't hit this again also add a WARN_ON checking for whether our timestamp is somehow moving into the past, which is never should. This also aligns more with how real hw works: 1. first all registers are updated with the new timestamp/vblank counter values. 2. then an interrupt is generated 3. kernel interrupt handler eventually fires. So doing this aligns vkms closer with what drm_vblank.c expects. Document this also in a comment. Cc: Shayenne Moura Cc: Rodrigo Siqueira Signed-off-by: Daniel Vetter Tested-by: Rodrigo Siqueira Reviewed-by: Rodrigo Siqueira --- drivers/gpu/drm/vkms/vkms_crtc.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 7508815fac11..1bbe099b7db8 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -15,6 +15,10 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) spin_lock(&output->lock); + ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, + output->period_ns); + WARN_ON(ret_overrun != 1); + ret = drm_crtc_handle_vblank(crtc); if (!ret) DRM_ERROR("vkms failure on handling vblank"); @@ -35,10 +39,6 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) DRM_WARN("failed to queue vkms_crc_work_handle"); } - ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, - output->period_ns); - WARN_ON(ret_overrun != 1); - spin_unlock(&output->lock); return HRTIMER_RESTART; @@ -74,11 +74,21 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, { struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev); struct vkms_output *output = &vkmsdev->output; + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; *vblank_time = output->vblank_hrtimer.node.expires; - if (!in_vblank_irq) - *vblank_time -= output->period_ns; + if (WARN_ON(*vblank_time == vblank->time)) + return true; + + /* + * To prevent races we roll the hrtimer forward before we do any + * interrupt processing - this is how real hw works (the interrupt is + * only generated after all the vblank registers are updated) and what + * the vblank core expects. Therefore we need to always correct the + * timestampe by one frame. + */ + *vblank_time -= output->period_ns; return true; }