Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 1 Aug 2020 15:49:29 -0300
From:   Melissa Wen <melissa.srw@gmail.com>
To:     Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>,
        Haneen Mohammed <hamohammed.sa@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        David Airlie <airlied@linux.ie>,
        Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>,
        Sidong Yang <realwakka@gmail.com>
Cc:     twoerner@gmail.com, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, kernel-usp@googlegroups.com
Subject: [PATCH] drm/vkms: guarantee vblank when capturing crc
Message-ID: <20200801184929.2eaxyoq6fm3nk4ey@smtp.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

VKMS needs vblank interrupts enabled to capture CRC. When vblank is
disabled, tests like kms_cursor_crc and kms_pipe_crc_basic getting stuck
waiting for a capture that will not occur until vkms wakes up. This
patch ensures that vblank remains enabled as long as the CRC capture is
needed.

It clears the execution of the following kms_cursor_crc subtests:
1. pipe-A-cursor-[size,alpha-opaque, NxN-(on-screen, off-screen, sliding,
random, fast-moving])] - successful when running individually.
2. pipe-A-cursor-dpms passes again
3. pipe-A-cursor-suspend also passes

The issue was initially tracked in the sequential execution of IGT
kms_cursor_crc subtest: when running the test sequence or one of its
subtests twice, the odd execs complete and the pairs get stuck in an
endless wait. In the IGT code, calling a wait_for_vblank on preparing
for CRC capture prevented the busy-wait. But the problem persisted in
the pipe-A-cursor-dpms and -suspend subtests.

Checking the history, the pipe-A-cursor-dpms subtest was successful
when, in vkms_atomic_commit_tail, instead of using the flip_done op, it
used wait_for_vblanks. Another way to prevent blocking was
wait_one_vblank when enabling crtc. However, in both cases,
pipe-A-cursor-suspend persisted blocking in the 2nd start of CRC
capture, which may indicate that something got stuck in the step of CRC
setup. Indeed, wait_one_vblank in the crc setup was able to sync things
and free all kms_cursor_crc subtests.

Besides, other alternatives to force enabling vblanks or prevent
disabling them such as calling drm_crtc_put_vblank or modeset_enables
before commit_planes + offdelay = 0, also unlock all subtests
executions. These facts together converge on the lack of vblank to
unblock the crc capture.

Finally, considering the vkms's dependence on vblank interruptions to
perform tasks, this patch acquires a vblank ref when setup CRC source
and releases ref when disabling crc capture, ensuring vblanks happen to
compute CRC.

Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Co-developed-by: Sidong Yang <realwakka@gmail.com>
Signed-off-by: Sidong Yang <realwakka@gmail.com>
Co-developed-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Melissa Wen <melissa.srw@gmail.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 4af2f19480f4..1161eaa383f1 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -241,6 +241,14 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
 
 	ret = vkms_crc_parse_source(src_name, &enabled);
 
+	/* Ensure that vblank interruptions are enabled for crc capture */
+	/* Enabling CRC: acquire vblank ref */
+	if (enabled)
+		drm_crtc_vblank_get(crtc);
+	/* Disabling CRC: release vblank ref */
+	if (!src_name)
+		drm_crtc_vblank_put(crtc);
+
 	spin_lock_irq(&out->lock);
 	out->composer_enabled = enabled;
 	spin_unlock_irq(&out->lock);
-- 
2.27.0