From mboxrd@z Thu Jan 1 00:00:00 1970 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 10408C4167B for ; Wed, 21 Dec 2022 22:04:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=JKtePHbZbFBzqyatQBTUMv0IQy5oybAeRu4/V8T/vE4=; b=AsiFn2kZSOblBW gPeI5N9E1VOq3LqEMatrVNTi2TZCee38cgEYN6El53sgu579vY4fZnTLp2H3Hy81E0Sfa4gcRu9DQ MK0VgSzgnKN3OHk3aFa0jBTsR549u3XHDQLWDbxy0Lbgmug14E8XJUrxyJRKf/F5NMKWg1juNRCV+ fJCrP7Fpi6XtKk+82rd5uHruXbVTBqA+gBjbFB8t1V0wWq/1tZQgh0+62BhfiBIUl2Ko4GW6/zDnA 5abZUsvu/dqOpfqiTdN+cSuIT1J7sFg/BXtQB03AfbY2607vcIeWpEt2MjKTSldgLZVWKmHWdCqAE IgM2j54GQvmZA1ZYJXUw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p87BI-002zAs-0w; Wed, 21 Dec 2022 22:03:20 +0000 Received: from mail-pg1-x530.google.com ([2607:f8b0:4864:20::530]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1p87Am-002yy6-Nx for linux-arm-kernel@lists.infradead.org; Wed, 21 Dec 2022 22:02:50 +0000 Received: by mail-pg1-x530.google.com with SMTP id 7so137904pga.1 for ; Wed, 21 Dec 2022 14:02:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=q+0GvlIHGVbxl/3eCKEWmbSw5vvxoPeGSxp8VoYXLkI=; b=jnOesX0JDINyp/ZOvK8a2iIfFafqsK1oo3tnsBW1zuZmdqOxuO5cDaUZdtiiXn6sl4 OsDiSLJN9W6qr8AEr8ooU7FFSa2uhRAw8aQ44loRJaR9yp/3QOt1FgAwAza6aU2eyFFQ +I8x3LJ1yYkTAtru1Iw5pf5Iooo+BkQE8ovBk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=q+0GvlIHGVbxl/3eCKEWmbSw5vvxoPeGSxp8VoYXLkI=; b=OqlvtpIbAphAQxhawAP9hH5x197GSfzRavGTdW65Gz/vc6rdpSTYNCm6ANW5b6KbCu nEMseW6ObsEwMqgVbVkbIM62yN7ecQJp9W9joGr3gNvWR6k7BVYAez1iTDa8E4XMiInW 7vMY2BOxXYMx4NkQcz8VMRZY8GNvnDQtZkiPJKbNbk9mCQJUZP3OMqVjoazS+FKgdvBF MTkVR4b+ZLNKhuClH+szH3NanAH0H7z5ro2EFu2Bz3ae+EWMF7N/lzgOiRzpXaLo98Tq wwVCl70QE9BL9SadnZpuWsvb/J5GflfAEz7t/j6U8udEmqCHfxd86WNM3MWs+mTgwyCf jGlw== X-Gm-Message-State: AFqh2kowtr4BZR/jauCblBWxc1V+lbqAFsUPqoFgvgD6MkDQBWxjKi7H l58MpG9xoi62INTqjYHGHLKpvw== X-Google-Smtp-Source: AMrXdXuXyLdgxcNpaIvNnkX4bXHyx8/3rf7c+ClwwJkWQFYLJU9WKKJfl/D0RqB1oQq75FtbjEgQ6A== X-Received: by 2002:a62:e90e:0:b0:577:1c59:a96c with SMTP id j14-20020a62e90e000000b005771c59a96cmr4015331pfh.2.1671660167146; Wed, 21 Dec 2022 14:02:47 -0800 (PST) Received: from google.com ([2620:15c:9d:2:7ca9:883d:f86b:5a1e]) by smtp.gmail.com with ESMTPSA id b27-20020aa78edb000000b005772bf1b61bsm11112984pfr.67.2022.12.21.14.02.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Dec 2022 14:02:46 -0800 (PST) Date: Wed, 21 Dec 2022 14:02:43 -0800 From: Brian Norris To: Mark Brown , Sean Paul Cc: kernelci-results@groups.io, bot@kernelci.org, Sean Paul , Douglas Anderson , gtucker@collabora.com, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec Subject: Re: renesas/master bisection: igt-kms-rockchip.kms_vblank.pipe-A-wait-forked on rk3399-gru-kevin Message-ID: References: <6398848e.170a0220.f8e8e.d44f@mx.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221221_140248_914131_E12CA1EA X-CRM114-Status: GOOD ( 32.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mark, Sean, (and dri-devel) On Wed, Dec 14, 2022 at 07:04:37PM -0800, Brian Norris wrote: > On Tue, Dec 13, 2022 at 04:51:11PM +0000, Mark Brown wrote: > > On Tue, Dec 13, 2022 at 05:56:30AM -0800, KernelCI bot wrote: > > > > The KernelCI bisection bot found regressions in at least two KMS tests > > in the Renesas tree on rk3399-gru-kevin just after the Renesas tree > > merged up mainline: > > > > igt-kms-rockchip.kms_vblank.pipe-A-wait-forked > > igt-kms-rockchip.kms_vblank.pipe-A-query-busy > > > > which it bisected to ca871659ec16 ("drm/bridge: analogix_dp: Support > > PSR-exit to disable transition"). I'm not *100%* sure I trust the > > bisection but it sure is suspicous that two separate bisects for related > > issues landed on the same commit. ... > > | IGT-Version: 1.26-gf8a4a0b (aarch64) (Linux: 6.1.0 aarch64) > > | <14>[ 35.444448] [IGT] drm_read: starting subtest short-buffer-wakeup > > | Starting subtest: short-buffer-wakeup > > | > > | (| drm_read:350) CRITICAL: Test assertion failure function generate_event, file ../tests/drm_read.c:65: > > | (drm_read:350) CRITICAL: <14>[ 36.155642] [IGT] drm_read: exiting, ret=98 > > | Failed assertion: kmstest_get_vblank(fd, pipe, DRM_VBLANK_EVENT) > > | > > | (drm_read:350) CRITICAL: Last errno: 22, Invalid argument > > | Stack trace: > > | > > | #0 ../lib/igt_core.c:1933 __igt_fail_assert() > > | #1 [+0xd5362770] > > | #2 [+0xd536193c] > > | #3 [__libc_start_main+0xe8] > > | #4 [+0xd5361974] > > | #5 [[ 36.162851] Console: switching to colour frame buffer device 300x100>+0xd5361974] > > | Subtest short-buffer-wakeup failed. ... > I'll look some more, but this might be a difference of test setup, such > that my setup has the issue before and after that commit, but your setup > sees a regression. I believe this is the case; things are broken both before and after, but depending on test sequencing, etc., they might have seemed less broken before. When we're failing, we're getting EINVAL from drm_vblank_get(). That essentially means that vblank has been disabled (drm_crtc_vblank_off()). As it happens, this is exactly what we do when we enter PSR (Panel Self Refresh). Now, these test cases (especially 'kms_vblank.pipe-A-wait-forked') seem to display a static image, and then wait for a bunch of vblank events. This is exactly the sort of case where we should enter PSR, and so we're likely to disable vblank events. Thus, if everything is working right, we will often hit EINVAL in ioctl(DRM_IOCTL_WAIT_VBLANK) ... -> drm_vblank_get(), and fail the test. As to why this appears to be a regression: like mentioned in my previous mail, these tests tend to hose the Analogix PSR state before my patch set. When PSR is hosed, we tend to stay with PSR disabled, and so drm_vblank_get() doesn't return EINVAL, and the test is happy. (Never mind that the display is likely non-functional.) [0] So how to fix this? 1. ignore it; it's "just" a test failure, IIUC [1] 2. change the test, to skip this test if the device has PSR 3. leave vblank enabled even in the presence of PSR 4. otherwise modify the vblank ioctl interface, such that one can "wait" for a vblank that won't come (because the display is in self-refresh / there are no new frames or vblanks) I don't know how to do #2, because this variant of PSR is intentionally opaque to user space. For #3: the downstream PSR support that these systems shipped with initially did not disable vblank in PSR. But that was massively rewritten/refactored by Sean Paul before it made it upstream, and now it does. I tried briefly to factor that part out (drivers/gpu/drm/rockchip/rockchip_drm_vop.c / drm_crtc_vblank_{on,off}()), but because drm_self_refresh_helper.c (ab?)uses the commit step to "disable" the CRTC for PSR (even though the CRTC is not fully disabled), I tend to hit this in drm_atomic_helper_commit_modeset_disables()->disable_outputs(): ret = drm_crtc_vblank_get(crtc); WARN_ONCE(ret != -EINVAL, "driver forgot to call drm_crtc_vblank_off()\n"); And I hit a few more problems too... ...I'm sure I could hack my way through this somehow, but this is all sounding like it could use some advice from someone more familiar with DRM and/or the drm_self_refresh_helper design. I've learned my way around this a bit by necessity, but I still feel like I don't know all the right answers here. (*cough*, Sean?) Brian [0] I definitely reproduce this part locally, before my patches. I can't find non-expired CI logs for "passing" runs to be sure that's what the CI is seeing though. [1] Or is it? I don't really know the DRM_IOCTL_WAIT_VBLANK ABI contract in the presence of PSR, but I believe the upstream PSR support has always worked this way. I could imagine DRM_IOCTL_WAIT_VBLANK users might not love seeing EINVAL here though. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel