From patchwork Fri Jul 19 15:23:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 11050257 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 A055D14DB for ; Fri, 19 Jul 2019 15:23:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 905A728488 for ; Fri, 19 Jul 2019 15:23:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8487628928; Fri, 19 Jul 2019 15:23:28 +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=unavailable 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 2A43128488 for ; Fri, 19 Jul 2019 15:23:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 66E3B6E84B; Fri, 19 Jul 2019 15:23:23 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8E8066E847 for ; Fri, 19 Jul 2019 15:23:21 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id w13so34852165eds.4 for ; Fri, 19 Jul 2019 08:23:21 -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=hQYxeQelQhcqDOEv8FVQvu3WSiozCNh00f3Jkyn+B8E=; b=SgS++8LWi5INFnQk7UBBKXeGmDwyu3t17f/CIPXMase1CLTxJ+jJuTtYQgPGg77jYE wsyshoblcTBfWLS80ipFQ3mG33BLJlRBcJDVijdLiR3LBjqzHqKYDmxTQcDxKxA4PmfZ k87Qolrnlw4rRMSnFJeWleJTrtD4BKayF0cb84O8OqT2SBRqv0jVbJbSR/1AZ7foDFAx 9GeZf1rjs7xbZU2hquDBwNzDhrLEjjrsckgNiqUH/tEHaItUszCXCbrHKSRWPREnFrA9 wqirEFkLNADYIoZcOfyILgyPkmpKfSI51zQePK6ajStrrjJLEy8tEB53TY4he+AqqDRQ 9hRQ== X-Gm-Message-State: APjAAAWCD6ydXsIUPwKq6336hXgfFLMyvzb9ihwQjz3TzZcY9mM9AWeU MkReFiyR3BwcH5TugMCXQz4= X-Google-Smtp-Source: APXvYqwatOuLVAq6j6xl1QLlwcnkd44wZ7Ta3+jgh0v2589xg7CeXOoFjIDBmCg7sauq4nD1rVLmKw== X-Received: by 2002:a17:906:a308:: with SMTP id j8mr42144115ejz.167.1563549800134; Fri, 19 Jul 2019 08:23:20 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id t2sm8882538eda.95.2019.07.19.08.23.19 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 19 Jul 2019 08:23:19 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Fri, 19 Jul 2019 17:23:12 +0200 Message-Id: <20190719152314.7706-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.22.0 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=hQYxeQelQhcqDOEv8FVQvu3WSiozCNh00f3Jkyn+B8E=; b=BJgWLu4Y7HHomDvTOaoXPHPvmqkhEzndQnKyrnK2GNc6SvWCrVzfFpOuTBhvAh/ey3 5e13Yccm+orxtDuoLWPBep80lLeAgK+iFrDntNF30upsdiN/j+H+C47x8tDyDNLi8idX CwaZ3V9veqgrBpA2sMNl9LqTLzF/s3L0TzT3c= Subject: [Intel-gfx] [PATCH 1/3] drm/vblank: Document and fix vblank count barrier semantics X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Daniel Vetter , Daniel Vetter , Intel Graphics Development , Rodrigo Siqueira Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Noticed while reviewing code. I'm not sure whether this might or might not explain some of the missed vblank hilarity we've been seeing. I think those all go through the vblank completion event, which has unconditional barriers - it always takes the spinlock. Therefore no cc stable. v2: - Barrriers are hard, put them in in the right order (Chris). - Improve the comments a bit. Cc: Rodrigo Siqueira Cc: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_vblank.c | 38 +++++++++++++++++++++++++++++++++++- include/drm/drm_vblank.h | 13 +++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 603ab105125d..eb2a8304536c 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -295,11 +295,23 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; + u64 count; if (WARN_ON(pipe >= dev->num_crtcs)) return 0; - return vblank->count; + count = vblank->count; + + /* + * This read barrier corresponds to the implicit write barrier of the + * write seqlock in store_vblank(). Note that this is the only place + * where we need an explicit barrier, since all other access goes + * through drm_vblank_count_and_time(), which already has the required + * read barrier curtesy of the read seqlock. + */ + smp_rmb(); + + return count; } /** @@ -764,6 +776,14 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, * vblank interrupt (since it only reports the software vblank counter), see * drm_crtc_accurate_vblank_count() for such use-cases. * + * Note that for a given vblank counter value drm_crtc_handle_vblank() + * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time() + * provide a barrier: Any writes done before calling + * drm_crtc_handle_vblank() will be visible to callers of the later + * functions, iff the vblank count is the same or a later one. + * + * See also &drm_vblank_crtc.count. + * * Returns: * The software vblank counter. */ @@ -818,6 +838,14 @@ static u64 drm_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, * vblank events since the system was booted, including lost events due to * modesetting activity. Returns corresponding system timestamp of the time * of the vblank interval that corresponds to the current vblank counter value. + * + * Note that for a given vblank counter value drm_crtc_handle_vblank() + * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time() + * provide a barrier: Any writes done before calling + * drm_crtc_handle_vblank() will be visible to callers of the later + * functions, iff the vblank count is the same or a later one. + * + * See also &drm_vblank_crtc.count. */ u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc, ktime_t *vblanktime) @@ -1791,6 +1819,14 @@ EXPORT_SYMBOL(drm_handle_vblank); * * This is the native KMS version of drm_handle_vblank(). * + * Note that for a given vblank counter value drm_crtc_handle_vblank() + * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time() + * provide a barrier: Any writes done before calling + * drm_crtc_handle_vblank() will be visible to callers of the later + * functions, iff the vblank count is the same or a later one. + * + * See also &drm_vblank_crtc.count. + * * Returns: * True if the event was successfully handled, false on failure. */ diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index e528bb2f659d..5ec623740158 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -110,7 +110,18 @@ struct drm_vblank_crtc { seqlock_t seqlock; /** - * @count: Current software vblank counter. + * @count: + * + * Current software vblank counter. + * + * Note that for a given vblank counter value drm_crtc_handle_vblank() + * and drm_crtc_vblank_count() or drm_crtc_vblank_count_and_time() + * provide a barrier: Any writes done before calling + * drm_crtc_handle_vblank() will be visible to callers of the later + * functions, iff the vblank count is the same or a later one. + * + * IMPORTANT: This guarantee requires barriers, therefor never access + * this field directly. Use drm_crtc_vblank_count() instead. */ u64 count; /** From patchwork Fri Jul 19 15:23:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 11050259 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 5F82A14DB for ; Fri, 19 Jul 2019 15:23:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4F7B528488 for ; Fri, 19 Jul 2019 15:23:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4371A28928; Fri, 19 Jul 2019 15:23:29 +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 EE6F628488 for ; Fri, 19 Jul 2019 15:23:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3215B6E84F; Fri, 19 Jul 2019 15:23:24 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x543.google.com (mail-ed1-x543.google.com [IPv6:2a00:1450:4864:20::543]) by gabe.freedesktop.org (Postfix) with ESMTPS id 942126E847 for ; Fri, 19 Jul 2019 15:23:22 +0000 (UTC) Received: by mail-ed1-x543.google.com with SMTP id d4so34855036edr.13 for ; Fri, 19 Jul 2019 08:23:22 -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:in-reply-to :references:mime-version:content-transfer-encoding; bh=WWlIQkZTe6LQuA5KTQkgPsDt9/EldhERVA4pYeIqcAw=; b=kaDz2ZJrvaI2MYYOEvIq8Xzkb/ktZ1z30VJnuwfPNkQ0pBibMtYH4aBQ1hJWgvNhon +/Z8BNDcI83Vz2DAYtYYAM5dQuaffyGrU07OfPAaF1AYiG2i1fMvduUYlfBxCzAn3AUw TWozcZxAt9mYUfJu8q175HdUJxovvd6lvr4hctgPekgj3UCzzeQWPCZplxeRj+jqU29k nPz/djgSKjKi2CGffP628VCa4gNpO06RNZhPV3PTNLnpPKTxi+3onhY4VcfMSgdXk4Ea VL02ldluRtAJbWSZrh9V/1L8q/hEMeCHe7dXz4glmTzobbsGItAI6jiX9fM1zAUErldZ DbEw== X-Gm-Message-State: APjAAAUkCj5ywAiHkDYAx0pWjNNM4VOxBr/wB0VKWPkD4qhbBcz6xJA9 64kJm9gKknzLjzTv7p8qihM= X-Google-Smtp-Source: APXvYqyfkLGT2aKqtamMXOfmfLZSnwPQA+lhK7SiBEsVLHZ2b9Hu39LHr80BckD6LJF3aHXjESgtYg== X-Received: by 2002:aa7:ce91:: with SMTP id y17mr47601734edv.56.1563549801310; Fri, 19 Jul 2019 08:23:21 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id t2sm8882538eda.95.2019.07.19.08.23.20 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 19 Jul 2019 08:23:20 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Fri, 19 Jul 2019 17:23:13 +0200 Message-Id: <20190719152314.7706-2-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190719152314.7706-1-daniel.vetter@ffwll.ch> References: <20190719152314.7706-1-daniel.vetter@ffwll.ch> 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:in-reply-to:references :mime-version:content-transfer-encoding; bh=WWlIQkZTe6LQuA5KTQkgPsDt9/EldhERVA4pYeIqcAw=; b=dKlfpJXOJzQ+8LO2HOq7bLbf+GQ7TsiAa0dtPDBwc1+xymllHNcjE7cY4xouIVgJid ubTPLiy1vgoShvbGw7Te+yAMderkGmbbxlQ0M3rb6tRXu7C6BHmpzz10O6SuEqFxoX7/ AZ6//+C+k6i5DCzQrJ4f5RF6ogV9v54yv4LDo= Subject: [Intel-gfx] [PATCH 2/3] drm/vkms: Use wait_for_flip_done X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Haneen Mohammed , Rodrigo Siqueira , Daniel Vetter , Intel Graphics Development , Daniel Vetter Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP It's the recommended version, wait_for_vblanks is a bit a hacky interim thing that predates all the flip_done tracking. It's unfortunately still the default ... Signed-off-by: Daniel Vetter Cc: Rodrigo Siqueira Cc: Haneen Mohammed Cc: Daniel Vetter Reviewed-by: Rodrigo Siqueira --- drivers/gpu/drm/vkms/vkms_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 44ab9f8ef8be..80524a22412a 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -83,7 +83,7 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state) drm_atomic_helper_commit_hw_done(old_state); - drm_atomic_helper_wait_for_vblanks(dev, old_state); + drm_atomic_helper_wait_for_flip_done(dev, old_state); for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i) { struct vkms_crtc_state *vkms_state = From patchwork Fri Jul 19 15:23:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 11050265 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 3266214DB for ; Fri, 19 Jul 2019 15:23:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2236228488 for ; Fri, 19 Jul 2019 15:23:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 169E528928; Fri, 19 Jul 2019 15:23:34 +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=unavailable 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 BB45928488 for ; Fri, 19 Jul 2019 15:23:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CD5076E850; Fri, 19 Jul 2019 15:23:28 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@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 F2E756E84D for ; Fri, 19 Jul 2019 15:23:23 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id e3so34849611edr.10 for ; Fri, 19 Jul 2019 08:23:23 -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:in-reply-to :references:mime-version:content-transfer-encoding; bh=xFHOTLwp+b/dZE4sngfYD0eMlUdDzWSkwvsokg6afgA=; b=X4+x/Xrwpk4/bhAhpRlO79QdQYsptQck4Mz/a9CewfLy8KwxjYmzBQFxEnFmi5pr2E oUT/YT858wrx7gcsTu3nVDYVvJWdVEYszXgGLjRU569UeUnaKJK0eOtKKj7E94OboTOF UdkAhNzSYZJ45IjH8hE/uHJTSlwt5ZcKtxDl2SUE9eG4jArVSQCMpNn54QgcJ3x0YEm1 aUYGf/NMRKgRyj0m5gO2PzIl9ZU9BvcxFWmrKDRGG2Rb7rtQ9PUGtpFPQj2PYq/iNrgF D1nMntlPHTYgTVH/YQfM9WK6yG8PgMMm3scGsEFYnTPAUMXcDl7/rkmMKaFIPfo8sVnv ++SA== X-Gm-Message-State: APjAAAUL+ZDlsbJkUXzSdh6Tiyon8g6WP42E3cQdxzOsJC9cZQFLv9bQ RH42LXeFIEhGOMXGWw70+S3sp7E7 X-Google-Smtp-Source: APXvYqyNOC9FzBSgRhiu0c1Eppme2UT4kllyHI4EpR3J46DXhp//MuZX5G03DisxbZklt3UtnbLi/A== X-Received: by 2002:a17:906:cce9:: with SMTP id ot41mr12688698ejb.196.1563549802546; Fri, 19 Jul 2019 08:23:22 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id t2sm8882538eda.95.2019.07.19.08.23.21 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 19 Jul 2019 08:23:21 -0700 (PDT) From: Daniel Vetter To: DRI Development Date: Fri, 19 Jul 2019 17:23:14 +0200 Message-Id: <20190719152314.7706-3-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190719152314.7706-1-daniel.vetter@ffwll.ch> References: <20190719152314.7706-1-daniel.vetter@ffwll.ch> 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:in-reply-to:references :mime-version:content-transfer-encoding; bh=xFHOTLwp+b/dZE4sngfYD0eMlUdDzWSkwvsokg6afgA=; b=WsyOLyahh9jkWf8mBub4V5UlJN5P4orW8IeSpSX+i5yGPUzTnoZ0XEdRm/ONV1D6s1 GFrl1sz67IK/S/hq2UWLZbWxNLauZ4E26Ekt6A2k3fFDLRJWwtcaDAFC6dwPdxTeo7Hb JVwS3xntMOzTYCgBbDW/8b+bU2iVydZagiglg= Subject: [Intel-gfx] [PATCH 3/3] drm/vkms: Reduce critical section in vblank_simulate X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Haneen Mohammed , Rodrigo Siqueira , Daniel Vetter , Intel Graphics Development , Daniel Vetter Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP We can reduce the critical section in vkms_vblank_simulate under output->lock quite a lot: - hrtimer_forward_now just needs to be ordered correctly wrt drm_crtc_handle_vblank. We already access the hrtimer timestamp without locks. While auditing that I noticed that we don't correctly annotate the read there, so sprinkle a READ_ONCE to make sure the compiler doesn't do anything foolish. - drm_crtc_handle_vblank must stay under the lock to avoid races with drm_crtc_arm_vblank_event. - The access to vkms_ouptut->crc_state also must stay under the lock. - next problem is making sure the output->state structure doesn't get freed too early. First we rely on a given hrtimer being serialized: If we call drm_crtc_handle_vblank, then we are guaranteed that the previous call to vkms_vblank_simulate has completed. The other side of the coin is that the atomic updates waits for the vblank to happen before it releases the old state. Both taken together means that by the time the atomic update releases the old state, the hrtimer won't access it anymore (it might be accessing the new state at the same time, but that's ok). - state is invariant, except the few fields separate protected by state->crc_lock. So no need to hold the lock for that. - finally the queue_work. We need to make sure there's no races with the flush_work, i.e. when we call flush_work we need to guarantee that the hrtimer can't requeue the work again. This is guaranteed by the same vblank/hrtimer ordering guarantees like the reasoning above why state won't be freed too early: flush_work on the old state is called after wait_for_flip_done in the atomic commit code. Therefore we can also move everything after the output->crc_state out of the critical section. Motivated by suggestions from Rodrigo. Signed-off-by: Daniel Vetter Cc: Rodrigo Siqueira Cc: Haneen Mohammed Cc: Daniel Vetter Reviewed-by: Rodrigo Siqueira --- drivers/gpu/drm/vkms/vkms_crtc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 927dafaebc76..74f703b8d22a 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -16,17 +16,18 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) u64 ret_overrun; bool ret; - spin_lock(&output->lock); - ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, output->period_ns); WARN_ON(ret_overrun != 1); + spin_lock(&output->lock); ret = drm_crtc_handle_vblank(crtc); if (!ret) DRM_ERROR("vkms failure on handling vblank"); state = output->composer_state; + spin_unlock(&output->lock); + if (state && output->composer_enabled) { u64 frame = drm_crtc_accurate_vblank_count(crtc); @@ -48,8 +49,6 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) DRM_DEBUG_DRIVER("Composer worker already queued\n"); } - spin_unlock(&output->lock); - return HRTIMER_RESTART; } @@ -85,7 +84,7 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, struct vkms_output *output = &vkmsdev->output; struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; - *vblank_time = output->vblank_hrtimer.node.expires; + *vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires); if (WARN_ON(*vblank_time == vblank->time)) return true;