From patchwork Tue Jan 10 01:18:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 13094559 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 33E6AC54EBD for ; Tue, 10 Jan 2023 01:19:16 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A7AC610E044; Tue, 10 Jan 2023 01:19:14 +0000 (UTC) Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6146710E044 for ; Tue, 10 Jan 2023 01:19:13 +0000 (UTC) Received: by mail-pj1-x102e.google.com with SMTP id w4-20020a17090ac98400b002186f5d7a4cso14787552pjt.0 for ; Mon, 09 Jan 2023 17:19:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=JRJYNk03xPF7I0f3RiiUNSrwj7K0SCkYwCDphRZWj3c=; b=LYUy8d956TFa4i0X/jINb9cLqY6o2DO8RoacHNTbbqpl8LHI2/Qg246QCSQgHVA+nH HJSdqiBTvghIgiNFKN+RnWDfVLcTTn1WiBk2hYjjliNu1Hb2C6+1JXHTA8Yqa0uUud+P UdwMdphpyLdOm8qn7t1kn+/887P41sckwQAH0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JRJYNk03xPF7I0f3RiiUNSrwj7K0SCkYwCDphRZWj3c=; b=FjFEskKnM28nPeQ/fZOf4u5Bi46Hy6vKGgMZf0UUzzVMyX/zJCnxXtIDrFKoT4IUtk InvAaFvizznvOT9xyQbq9jpdcMXZA69HtwqjRfUT7Y/5IJudMlaDWMB4GQTVH1nukbDh yiwEHrU2lyv2XlKBT1gbN+2FeBNXPXBpwvB1zW7//qbO7sh6LKvpFN/c+mBomIEnArAM akf29yPRPatVsKFVIxjUz2Sci+jzllryoU5q6JxsEgGcCvdms2EHWChu5x04pV8hU1OT A+q0iWUxvjoOvdI5geA9Cmy7wLQU5Bn6DgaqZIPGdSu1Kblvtpf7Y4sZovSmLe4hjtmV WdzA== X-Gm-Message-State: AFqh2kp2a1FHEndNA+Yo+dHN3cAgu4trdHXstfgEG4WrAMnIPuwywsAm A4aOgpN1NeryimXXHvbCNoe+dg== X-Google-Smtp-Source: AMrXdXt7E/WtGBXtZLOsFB0eXRM7CAxST+jXno99j/6BC6C7198kPAZn7z/EVahK9YCxBH+9/0JnBQ== X-Received: by 2002:a17:902:e951:b0:193:2ed4:561a with SMTP id b17-20020a170902e95100b001932ed4561amr5989384pll.38.1673313552926; Mon, 09 Jan 2023 17:19:12 -0800 (PST) Received: from localhost ([2620:15c:9d:2:99d8:feca:9efd:a216]) by smtp.gmail.com with UTF8SMTPSA id u1-20020a170902714100b001933355456esm17519plm.215.2023.01.09.17.19.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 Jan 2023 17:19:12 -0800 (PST) From: Brian Norris To: =?utf-8?q?Heiko_St=C3=BCbner?= , Daniel Vetter , Sean Paul Subject: [PATCH v3 1/2] drm/atomic: Allow vblank-enabled + self-refresh "disable" Date: Mon, 9 Jan 2023 17:18:16 -0800 Message-Id: <20230109171809.v3.1.I3904f697863649eb1be540ecca147a66e42bfad7@changeid> X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog 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: =?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" The self-refresh helper framework overloads "disable" to sometimes mean "go into self-refresh mode," and this mode activates automatically (e.g., after some period of unchanging display output). In such cases, the display pipe is still considered "on", and user-space is not aware that we went into self-refresh mode. Thus, users may expect that vblank-related features (such as DRM_IOCTL_WAIT_VBLANK) still work properly. However, we trigger the WARN_ONCE() here if a CRTC driver tries to leave vblank enabled. Add a different expectation: that CRTCs *should* leave vblank enabled when going into self-refresh. This patch is preparation for another patch -- "drm/rockchip: vop: Leave vblank enabled in self-refresh" -- which resolves conflicts between the above self-refresh behavior and the API tests in IGT's kms_vblank test module. == Some alternatives discussed: == It's likely that on many display controllers, vblank interrupts will turn off when the CRTC is disabled, and so in some cases, self-refresh may not support vblank. To support such cases, we might consider additions to the generic helpers such that we fire vblank events based on a timer. However, there is currently only one driver using the common self-refresh helpers (i.e., rockchip), and at least as of commit bed030a49f3e ("drm/rockchip: Don't fully disable vop on self refresh"), the CRTC hardware is powered enough to continue to generate vblank interrupts. So we chose the simpler option of leaving vblank interrupts enabled. We can reevaluate this decision and perhaps augment the helpers if/when we gain a second driver that has different requirements. v3: * include discussion summary v2: * add 'ret != 0' warning case for self-refresh * describe failing test case and relation to drm/rockchip patch better Cc: # dependency for "drm/rockchip: vop: Leave # vblank enabled in self-refresh" Signed-off-by: Brian Norris --- drivers/gpu/drm/drm_atomic_helper.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index d579fd8f7cb8..a22485e3e924 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1209,7 +1209,16 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) continue; ret = drm_crtc_vblank_get(crtc); - WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n"); + /* + * Self-refresh is not a true "disable"; ensure vblank remains + * enabled. + */ + if (new_crtc_state->self_refresh_active) + WARN_ONCE(ret != 0, + "driver disabled vblank in self-refresh\n"); + else + WARN_ONCE(ret != -EINVAL, + "driver forgot to call drm_crtc_vblank_off()\n"); if (ret == 0) drm_crtc_vblank_put(crtc); } From patchwork Tue Jan 10 01:18:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Norris X-Patchwork-Id: 13094560 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 2E95BC5479D for ; Tue, 10 Jan 2023 01:19:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 285DA10E0E7; Tue, 10 Jan 2023 01:19:18 +0000 (UTC) Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by gabe.freedesktop.org (Postfix) with ESMTPS id C1EF010E0EC for ; Tue, 10 Jan 2023 01:19:15 +0000 (UTC) Received: by mail-pj1-x1029.google.com with SMTP id o7-20020a17090a0a0700b00226c9b82c3aso11697207pjo.3 for ; Mon, 09 Jan 2023 17:19:15 -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=XYUtIGSdiXv+1u4fwXxMmXFQoGZCC8A3hPLYtYLpNZQ=; b=Bmk2SAk4VLW306BHVtiboysQjz5H/DFJf0Tjh0hvrntMIxLtck9ATcuiUFhmj7LHNL B2lYETyD1f5zrcSLwCFTSAlf7/2XYNt9xuJqUxURQT7gjmsPAV0cEbg1BsWx9KUl86lc IzdBVOADWqnelw8o3fWnEGPECU4Vf3WBPVXls= 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=XYUtIGSdiXv+1u4fwXxMmXFQoGZCC8A3hPLYtYLpNZQ=; b=O2mRldKVKEztjEQKITlXD/k504mNOiYTkrktgSzG6xIjQ3rR/dKrmfpAH3kc/KMXei x/YsnxIxeqFw6nP0vx7W4GNNEjPoP8NLGWb5NVZ33Wag+C4qoJYPVJvJQJYpohuRrzTV rVZ5I1YoRSldKS0mjwVgCEFu3TYv9uhHEu2mePhtWq46oONlI105BrX2CRP/OP1PC6p9 8yBnOJTVf5MYV/k+Bb4QB02Fcv8hsIQdN8lcIn69Yzwt5VJzHy2uVegAZWKgBqZf8WDd KZWnHWParyub3IvHMEeLal+c35eAaFgl8uSsdIvTTSnJ5JgYSjr3nHLP69bBGeG9hMe6 SFGQ== X-Gm-Message-State: AFqh2kpGp0pNNWK9qdf3+BH+AQJuAWR4xO/YowIOLCCaltxiL3PUegxc +Rr7vVsirIk+ysslapOKtxhCBA== X-Google-Smtp-Source: AMrXdXtC5EUTKJLeK/jZ2SnSPdj4TAQrYDbyicW7TAjCMQdn4sHpMINHMClNMz/+JBeoOEMkxs7HIw== X-Received: by 2002:a05:6a21:999d:b0:9e:9685:f15e with SMTP id ve29-20020a056a21999d00b0009e9685f15emr100766242pzb.0.1673313555362; Mon, 09 Jan 2023 17:19:15 -0800 (PST) Received: from localhost ([2620:15c:9d:2:99d8:feca:9efd:a216]) by smtp.gmail.com with UTF8SMTPSA id j1-20020a170902c3c100b0018853416bbcsm6827097plj.7.2023.01.09.17.19.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 Jan 2023 17:19:14 -0800 (PST) From: Brian Norris To: =?utf-8?q?Heiko_St=C3=BCbner?= , Daniel Vetter , Sean Paul Subject: [PATCH v3 2/2] drm/rockchip: vop: Leave vblank enabled in self-refresh Date: Mon, 9 Jan 2023 17:18:17 -0800 Message-Id: <20230109171809.v3.2.Ic07cba4ab9a7bd3618a9e4258b8f92ea7d10ae5a@changeid> X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog In-Reply-To: <20230109171809.v3.1.I3904f697863649eb1be540ecca147a66e42bfad7@changeid> References: <20230109171809.v3.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. The bug has been around a while (longer than KernelCI noticed), but was only exposed once self-refresh was bugfixed more recently, and so KernelCI could properly test it. Some other notes in: https://lore.kernel.org/dri-devel/Y6OCg9BPnJvimQLT@google.com/ Re: renesas/master bisection: igt-kms-rockchip.kms_vblank.pipe-A-wait-forked on rk3399-gru-kevin == 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. v3: * no update 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);