mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Jyri Sarha <jyri.sarha@iki.fi>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	 David Airlie <airlied@gmail.com>,
	Simona Vetter <simona@ffwll.ch>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>,
	 dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	 Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Subject: [PATCH 2/2] drm/tidss: Set vblank (event) time at crtc_atomic_enable
Date: Fri, 05 Sep 2025 16:58:07 +0300	[thread overview]
Message-ID: <20250905-tidss-fix-timestamp-v1-2-c2aedf31e2c9@ideasonboard.com> (raw)
In-Reply-To: <20250905-tidss-fix-timestamp-v1-0-c2aedf31e2c9@ideasonboard.com>

It was reported that Weston stops at an assert, which checks that the
page flip event timestamp is the same or newer than the previous
timestamp:

weston_output_finish_frame: Assertion `timespec_sub_to_nsec(stamp, &output->frame_time) >= 0' failed.

With manual tests, I can see that when I enable the CRTC, I get a page
flip event with a timestamp of 0. Tracking this down led to
drm_reset_vblank_timestamp() which does "t_vblank = 0" if
"high-precision query" is not available.

TI DSS does not have any hardware timestamping, and thus the default
ktime_get() is used in the DRM framework to get the vblank timestamp,
and ktime_get() is not "high precision" here.

It is not quite clear why the framework behaves this way, but I assume
the idea is that drm_crtc_vblank_on(), which calls
drm_reset_vblank_timestamp(), can be called at any time, and thus
ktime_get() wouldn't give a good timestamp. And, the idea is that the
driver would wait until next vblank after the CRTC enable, and then we
could get a good timestamp. This is hinted in the comment: "reinitialize
delayed at next vblank interrupt and assign 0 for now".

I think that makes sense. However, when we enable the CRTC in TI DSS,
i.e. we write the enable bit to the hardware, that's the exact moment
when the "vblank cycle" starts. It is the zero point in the cycle, and
thus ktime_get() would give a good timestamp.

I am not sure if this is applicable to other hardware, and if so, how
should it be solved in the framework. So, let's fix this in the tidss
driver at least for now.

This patch updates the vblank->time manually to ktime_get() just before
sending the vblank event, and we enable the crtc just before calling
ktime_get(). To get even more exact timing, the dispc_vp_enable() is
moved inside the event_lock spinlock.

With this, we get a proper timestamp for the page flip event from
enabling the CRTC, and Weston is happy.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/tidss/tidss_crtc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_crtc.c b/drivers/gpu/drm/tidss/tidss_crtc.c
index 1b767af8e1f6..6898f12bb364 100644
--- a/drivers/gpu/drm/tidss/tidss_crtc.c
+++ b/drivers/gpu/drm/tidss/tidss_crtc.c
@@ -244,11 +244,16 @@ static void tidss_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	dispc_vp_prepare(tidss->dispc, tcrtc->hw_videoport, crtc->state);
 
-	dispc_vp_enable(tidss->dispc, tcrtc->hw_videoport);
-
 	spin_lock_irqsave(&ddev->event_lock, flags);
 
+	dispc_vp_enable(tidss->dispc, tcrtc->hw_videoport);
+
 	if (crtc->state->event) {
+		unsigned int pipe = drm_crtc_index(crtc);
+		struct drm_vblank_crtc *vblank = &ddev->vblank[pipe];
+
+		vblank->time = ktime_get();
+
 		drm_crtc_send_vblank_event(crtc, crtc->state->event);
 		crtc->state->event = NULL;
 	}

-- 
2.43.0


  parent reply	other threads:[~2025-09-05 13:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-05 13:58 [PATCH 0/2] drm/tidss: Fix issue with page flip timestamp on enable Tomi Valkeinen
2025-09-05 13:58 ` [PATCH 1/2] drm/tidss: Restructure dispc_vp_prepare() and dispc_vp_enable() Tomi Valkeinen
2025-10-22 13:11   ` Devarsh Thakkar
2025-09-05 13:58 ` Tomi Valkeinen [this message]
2025-10-22 13:18   ` [PATCH 2/2] drm/tidss: Set vblank (event) time at crtc_atomic_enable Devarsh Thakkar

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=20250905-tidss-fix-timestamp-v1-2-c2aedf31e2c9@ideasonboard.com \
    --to=tomi.valkeinen@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jyri.sarha@iki.fi \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=pekka.paalanen@collabora.com \
    --cc=simona@ffwll.ch \
    --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

all inboxes | Powered by JetHome®