On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote:
> The pageflip completion timestamps transmitted to userspace
> via pageflip completion events are supposed to describe the
> time at which the first pixel of the new post-pageflip scanout
> buffer leaves the video output of the gpu. This time is
> identical to end of vblank, when active scanout starts.
>
> For a crtc in standard fixed refresh rate, the end of vblank
> is identical to the vblank timestamps calculated by
> drm_update_vblank_count() at each vblank interrupt, or each
> vblank dis-/enable. Therefore pageflip events just carry
> that vblank timestamp as their pageflip timestamp.
>
> For a crtc switched to variable refresh rate mode (vrr), the
> pageflip completion timestamps are identical to the vblank
> timestamps iff the pageflip was executed early in vblank,
> before the minimum vblank duration elapsed. In this case
> the time of display onset is identical to when the crtc
> is running in fixed refresh rate.
>
> However, if a pageflip completes later in the vblank, inside
> the "extended front porch" in vrr mode, then the vblank will
> terminate at a fixed (back porch) duration after flip, so
> the display onset time is delayed correspondingly. In this
> case the vblank timestamp computed at vblank irq time would
> be too early, and we need a way to calculate an estimated
> pageflip timestamp that will be later than the vblank timestamp.
>
> How a driver determines such a "late flip" timestamp is hw
> and driver specific, but this patch adds a new helper function
> that allows the driver to propose such an alternate "late flip"
> timestamp for use in pageflip events:
>
> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>
> When sending out pageflip events, we now compare that proposed
> flip_timestamp against the vblank timestamp of the current
> vblank of flip completion and choose to send out the greater/
> later timestamp as flip completion timestamp.
>
> The most simple way for a kms driver to supply a suitable
> flip_timestamp in vrr mode would be to simply take a timestamp
> at start of the pageflip completion handler, e.g., pageflip
> irq handler: flip_timestamp = ktime_get(); and then set that
> as proposed "late" alternative timestamp via ...
> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>
> More clever approaches could try to add some corrective offset
> for fixed back porch duration, or ideally use hardware features
> like hw timestamps to calculate the exact end time of vblank.
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de@xxxxxxxxx>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us
the right timestampe, once? With this I guess if you do a vblank query in
between the wrong and the right vblank you'll get the bogus value. Not
really great for userspace.
-Daniel
> ---
> drivers/gpu/drm/drm_vblank.c | 49 +++++++++++++++++++++++++++++++++++-
> include/drm/drm_vblank.h | 8 ++++++
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 98e091175921..4b3a4c38fabe 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -814,10 +814,21 @@ static void send_vblank_event(struct drm_device *dev,
> u64 seq, ktime_t now)
> {
> struct timespec64 tv;
> + ktime_t alt_flip_time;
>
> switch (e->event.base.type) {
> - case DRM_EVENT_VBLANK:
> case DRM_EVENT_FLIP_COMPLETE:
> + /*
> + * For flip completion events, override "now" time
> + * with alt_flip_time provided by the driver via
> + * drm_crtc_set_vrr_pageflip_timestamp() in VRR mode
> + * if that time is later than given "now" vblank time.
> + */
> + alt_flip_time = dev->vblank[e->pipe].alt_flip_time;
> + if (alt_flip_time > now)
> + now = alt_flip_time;
> + /* Fallthrough */
> + case DRM_EVENT_VBLANK:
> tv = ktime_to_timespec64(now);
> e->event.vbl.sequence = seq;
> /*
> @@ -916,11 +927,47 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>
> now = ktime_get();
> }
> +
> e->pipe = pipe;
> send_vblank_event(dev, e, seq, now);
> }
> EXPORT_SYMBOL(drm_crtc_send_vblank_event);
>
> +/**
> + * drm_crtc_set_vrr_pageflip_timestamp - helper to set alternate pageflip time
> + * @crtc: the source CRTC of the pageflip completion event
> + * @flip_time: The alternate pageflip completion timestamp in VRR mode
> + *
> + * In variable refresh rate mode (VRR), a pageflip completion timestamp carried
> + * by the pageflip event can never be earlier than the vblank timestamp of the
> + * vblank of flip completion, as that vblank timestamp defines the end of the
> + * shortest possible vblank duration. In case of a delayed flip completion
> + * inside the extended VRR front porch however, the end of vblank can be much
> + * later, so the driver must assign an estimated timestamp of that later end of
> + * vblank. For a CRTC in VRR mode, the driver should use this helper function to
> + * set an alternate flip completion timestamp in case of late flip completions
> + * in extended vblank. In the most simple case, this @flip_time timestamp could
> + * simply be a ktime_get() timestamp taken at the start of the pageflip
> + * completion routine, with some constant duration of the back porch interval
> + * added, although more precise estimates may be possible on some hardware if
> + * the hardware provides some means of timestamping the true end of vblank.
> + *
> + * When sending out pageflip events, e.g., via drm_crtc_send_vblank_event(), it
> + * will use either the standard vblank timestamp, calculated for a minimum
> + * duration vblank, or the provided @flip_time if that time is later than the
> + * vblank timestamp, to get the best possible estimate of start of display of
> + * the new post-pageflip scanout buffer.
> + */
> +void drm_crtc_set_vrr_pageflip_timestamp(struct drm_crtc *crtc,
> + ktime_t flip_time)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(crtc)];
> +
> + vblank->alt_flip_time = flip_time;
> +}
> +EXPORT_SYMBOL(drm_crtc_set_vrr_pageflip_timestamp);
> +
> static int __enable_vblank(struct drm_device *dev, unsigned int pipe)
> {
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 6ad9630d4f48..aacf44694ab6 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -117,6 +117,12 @@ struct drm_vblank_crtc {
> * @time: Vblank timestamp corresponding to @count.
> */
> ktime_t time;
> + /**
> + * @alt_flip_time: Vblank timestamp for end of extended vblank due to
> + * a late pageflip completion in variable refresh rate mode. Pageflip
> + * events will carry the later one of @time and @alt_flip_time.
> + */
> + ktime_t alt_flip_time;
>
> /**
> * @refcount: Number of users/waiters of the vblank interrupt. Only when
> @@ -179,6 +185,8 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs);
> u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> ktime_t *vblanktime);
> +void drm_crtc_set_vrr_pageflip_timestamp(struct drm_crtc *crtc,
> + ktime_t flip_time);
> void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
> struct drm_pending_vblank_event *e);
> void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
