From patchwork Sat Jan 7 01:23:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 13091944 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B9BDAC5479D for ; Sat, 7 Jan 2023 01:24:00 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 72CC010E2E2; Sat, 7 Jan 2023 01:23:58 +0000 (UTC) Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6709C10E2E2 for ; Sat, 7 Jan 2023 01:23:57 +0000 (UTC) Received: by mail-pl1-x629.google.com with SMTP id jl4so3505671plb.8 for ; Fri, 06 Jan 2023 17:23:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Fqc+kkMFie/fztyQhVGKblB/deI6poCZbFpfQY0gQ5M=; b=T58kzIf5KVpaNXEtW9T33Mo50vYqvco1twQmTAboyo/KCu3tpXXPxRX3is2nRboBkK IFmSrS0lC/MbF3+tyDud6LC3NdPNHNDld4SpKQ/1bF5KEgQ1Cd/Puai5wuLafJcGw5hQ Xu6lrgspDIHnr68pABxoI/C7OTjXX4iVHNjOg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Fqc+kkMFie/fztyQhVGKblB/deI6poCZbFpfQY0gQ5M=; b=oGp8fRoZUPfbXdDhM6hUxK9xvr/HEB5Z/TnNKavU6fHj0QbiXjRJeohvqJBrJKmXYq onHUUPKePP6um/qj6gMFXNdm4SMZSzTsXNgoSVTwexY2Vkkm3IxQUnldIYs9CbenUApi 0Z3yISIKJ4d5Eg5rVyXe1HKYXqxQnu6MofWmRdHrYoAtT0Ag+l2FAk87y4H2p5bSIWbq kzGLQuKEX6H14C7svr5gW0C+mYBpsH7KNuSQJys3nR6sV9WMeUvHoA/ltKNI4RJmQpzm RttwIpUvx8zuJzi4Mdd9xyLtlqCNbmKjnFOL03Rx9mvCYeINVNZwlhdpZ9vVG2NQIGkz ml2g== X-Gm-Message-State: AFqh2kpf642HktOBfUnRgVUe4v5Dq59XBGYwknI3YzkCf0Kq3S4Xf4dj vi2rtYti71PpZEmapmfu1GdsXg== X-Google-Smtp-Source: AMrXdXvm4NOW/ltnbwyEvzMNORkJ0sSf/IJ/q3eNi+aIWeAhDUuIo1PyIhxCieNcGFJa22JoQ5TygQ== X-Received: by 2002:a17:902:ee13:b0:189:13df:9d86 with SMTP id z19-20020a170902ee1300b0018913df9d86mr65902309plb.14.1673054636979; Fri, 06 Jan 2023 17:23:56 -0800 (PST) Received: from localhost ([2620:15c:9d:2:bc4e:2cc9:68b3:15dc]) by smtp.gmail.com with UTF8SMTPSA id cp12-20020a170902e78c00b0019276616eb0sm1569183plb.83.2023.01.06.17.23.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 Jan 2023 17:23:56 -0800 (PST) From: Brian Norris To: =?utf-8?q?Heiko_St=C3=BCbner?= , Daniel Vetter , Sean Paul Subject: [PATCH v2 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh Date: Fri, 6 Jan 2023 17:23:23 -0800 Message-Id: <20230106172310.v2.2.Ic07cba4ab9a7bd3618a9e4258b8f92ea7d10ae5a@changeid> X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog In-Reply-To: <20230106172310.v2.1.I3904f697863649eb1be540ecca147a66e42bfad7@changeid> References: <20230106172310.v2.1.I3904f697863649eb1be540ecca147a66e42bfad7@changeid> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "kernelci.org bot" , =?utf-8?q?Michel_D=C3=A4nzer?= , Brian Norris , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Sandy Huang , linux-rockchip@lists.infradead.org, stable@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" If we disable vblank when entering self-refresh, vblank APIs (like DRM_IOCTL_WAIT_VBLANK) no longer work. But user space is not aware when we enter self-refresh, so this appears to be an API violation -- that DRM_IOCTL_WAIT_VBLANK fails with EINVAL whenever the display is idle and enters self-refresh. The downstream driver used by many of these systems never used to disable vblank for PSR, and in fact, even upstream, we didn't do that until radically redesigning the state machine in commit 6c836d965bad ("drm/rockchip: Use the helpers for PSR"). Thus, it seems like a reasonable API fix to simply restore that behavior, and leave vblank enabled. Note that this appears to potentially unbalance the drm_crtc_vblank_{off,on}() calls in some cases, but: (a) drm_crtc_vblank_on() documents this as OK and (b) if I do the naive balancing, I find state machine issues such that we're not in sync properly; so it's easier to take advantage of (a). This issue was exposed by IGT's kms_vblank tests, and reported by KernelCI. Backporting notes: Marking as 'Fixes' commit 6c836d965bad ("drm/rockchip: Use the helpers for PSR"), but it probably depends on commit bed030a49f3e ("drm/rockchip: Don't fully disable vop on self refresh") as well. We also need the previous patch ("drm/atomic: Allow vblank-enabled + self-refresh "disable""), of course. v2: * skip unnecessary lock/unlock Fixes: 6c836d965bad ("drm/rockchip: Use the helpers for PSR") Cc: Link: https://lore.kernel.org/dri-devel/Y5itf0+yNIQa6fU4@sirena.org.uk/ Reported-by: "kernelci.org bot" Signed-off-by: Brian Norris --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index fa1f4ee6d195..9fea03121247 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -717,13 +717,13 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc, if (crtc->state->self_refresh_active) rockchip_drm_set_win_enabled(crtc, false); + if (crtc->state->self_refresh_active) + goto out; + mutex_lock(&vop->vop_lock); drm_crtc_vblank_off(crtc); - if (crtc->state->self_refresh_active) - goto out; - /* * Vop standby will take effect at end of current frame, * if dsp hold valid irq happen, it means standby complete. @@ -757,9 +757,9 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc, vop_core_clks_disable(vop); pm_runtime_put(vop->dev); -out: mutex_unlock(&vop->vop_lock); +out: if (crtc->state->event && !crtc->state->active) { spin_lock_irq(&crtc->dev->event_lock); drm_crtc_send_vblank_event(crtc, crtc->state->event);