dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Daniel Vetter <daniel.vetter@intel.com>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Maxime Ripard <maxime@cerno.tech>
Cc: Melissa Wen <mwen@igalia.com>, dri-devel@lists.freedesktop.org
Subject: [PATCH 13/14] drm/vc4: crtc: Fix out of order frames during asynchronous page flips
Date: Tue,  3 May 2022 14:13:40 +0200	[thread overview]
Message-ID: <20220503121341.983842-14-maxime@cerno.tech> (raw)
In-Reply-To: <20220503121341.983842-1-maxime@cerno.tech>

When doing an asynchronous page flip (PAGE_FLIP ioctl with the
DRM_MODE_PAGE_FLIP_ASYNC flag set), the current code waits for the
possible GPU buffer being rendered through a call to
vc4_queue_seqno_cb().

On the BCM2835-37, the GPU driver is part of the vc4 driver and that
function is defined in vc4_gem.c to wait for the buffer to be rendered,
and once it's done, call a callback.

However, on the BCM2711 used on the RaspberryPi4, the GPU driver is
separate (v3d) and that function won't do anything. This was working
because we were going into a path, due to uninitialized variables, that
was always scheduling the callback.

However, we were never actually waiting for the buffer to be rendered
which was resulting in frames being displayed out of order.

The generic API to signal those kind of completion in the kernel are the
DMA fences, and fortunately the v3d drivers supports them and signal
when its job is done. That API also provides an equivalent function that
allows to have a callback being executed when the fence is signalled as
done.

Let's change our driver a bit to rely on the previous function for the
older SoCs, and on DMA fences for the BCM2711.

Signed-off-by: Maxime Ripard <maxime@cerno.tech>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 41 ++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index e0ae7bef08fa..8e1369fca937 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -776,6 +776,7 @@ struct vc4_async_flip_state {
 	struct drm_pending_vblank_event *event;
 
 	union {
+		struct dma_fence_cb fence;
 		struct vc4_seqno_cb seqno;
 	} cb;
 };
@@ -835,6 +836,43 @@ static void vc4_async_page_flip_seqno_complete(struct vc4_seqno_cb *cb)
 		vc4_bo_dec_usecnt(bo);
 }
 
+static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
+					       struct dma_fence_cb *cb)
+{
+	struct vc4_async_flip_state *flip_state =
+		container_of(cb, struct vc4_async_flip_state, cb.fence);
+
+	vc4_async_page_flip_complete(flip_state);
+	dma_fence_put(fence);
+}
+
+static int vc4_async_set_fence_cb(struct drm_device *dev,
+				  struct vc4_async_flip_state *flip_state)
+{
+	struct drm_framebuffer *fb = flip_state->fb;
+	struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0);
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct dma_fence *fence;
+	int ret;
+
+	if (!vc4->is_vc5) {
+		struct vc4_bo *bo = to_vc4_bo(&cma_bo->base);
+
+		return vc4_queue_seqno_cb(dev, &flip_state->cb.seqno, bo->seqno,
+					  vc4_async_page_flip_seqno_complete);
+	}
+
+	ret = dma_resv_get_singleton(cma_bo->base.resv, false, &fence);
+	if (ret)
+		return ret;
+
+	if (dma_fence_add_callback(fence, &flip_state->cb.fence,
+				   vc4_async_page_flip_fence_complete))
+		vc4_async_page_flip_fence_complete(fence, &flip_state->cb.fence);
+
+	return 0;
+}
+
 static int
 vc4_async_page_flip_common(struct drm_crtc *crtc,
 			   struct drm_framebuffer *fb,
@@ -874,8 +912,7 @@ vc4_async_page_flip_common(struct drm_crtc *crtc,
 	 */
 	drm_atomic_set_fb_for_plane(plane->state, fb);
 
-	vc4_queue_seqno_cb(dev, &flip_state->cb.seqno, bo->seqno,
-			   vc4_async_page_flip_seqno_complete);
+	vc4_async_set_fence_cb(dev, flip_state);
 
 	/* Driver takes ownership of state on successful async commit. */
 	return 0;
-- 
2.35.1


  parent reply	other threads:[~2022-05-03 12:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-03 12:13 [PATCH 00/14] drm/vc4: Properly separate v3d on BCM2711, and fix frame ordering Maxime Ripard
2022-05-03 12:13 ` [PATCH 01/14] drm/vc4: plane: Prevent async update if we don't have a dlist Maxime Ripard
2022-05-03 12:13 ` [PATCH 02/14] drm/vc4: Consolidate Hardware Revision Check Maxime Ripard
2022-05-03 12:13 ` [PATCH 03/14] drm/vc4: bo: Rename vc4_dumb_create Maxime Ripard
2022-05-03 12:13 ` [PATCH 04/14] drm/vc4: bo: Split out Dumb buffers fixup Maxime Ripard
2022-05-03 12:13 ` [PATCH 05/14] drm/vc4: drv: Register a different driver on BCM2711 Maxime Ripard
2022-05-09 16:37   ` Melissa Wen
2022-05-03 12:13 ` [PATCH 06/14] drm/vc4: kms: Register a different drm_mode_config_funcs " Maxime Ripard
2022-05-03 12:13 ` [PATCH 07/14] drm/vc4: plane: Register a different drm_plane_helper_funcs " Maxime Ripard
2022-05-03 12:13 ` [PATCH 08/14] drm/vc4: drv: Skip BO Backend Initialization " Maxime Ripard
2022-05-03 12:13 ` [PATCH 09/14] drm/vc4: crtc: Use an union to store the page flip callback Maxime Ripard
2022-05-03 12:13 ` [PATCH 10/14] drm/vc4: crtc: Move the BO handling out of common page-flip callback Maxime Ripard
2022-05-03 12:13 ` [PATCH 11/14] drm/vc4: crtc: Move the BO Handling out of Common Page-Flip Handler Maxime Ripard
2022-05-03 12:13 ` [PATCH 12/14] drm/vc4: crtc: Don't call into BO Handling on Async Page-Flips on BCM2711 Maxime Ripard
2022-05-03 12:13 ` Maxime Ripard [this message]
2022-05-03 13:53   ` [PATCH 13/14] drm/vc4: crtc: Fix out of order frames during asynchronous page flips kernel test robot
2022-05-09 17:10   ` Melissa Wen
2022-05-09 17:15     ` Melissa Wen
2022-05-12 10:44       ` Melissa Wen
2022-06-06 13:59         ` Maxime Ripard
2022-06-08 11:24           ` Melissa Wen
2022-05-03 12:13 ` [PATCH 14/14] drm/vc4: Warn if some v3d code is run on BCM2711 Maxime Ripard
2022-05-09 16:52   ` Melissa Wen
2022-05-31  9:40     ` Maxime Ripard
2022-05-09 18:26 ` [PATCH 00/14] drm/vc4: Properly separate v3d on BCM2711, and fix frame ordering Melissa Wen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220503121341.983842-14-maxime@cerno.tech \
    --to=maxime@cerno.tech \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mwen@igalia.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox