Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Wed, Feb 22, 2023 at 07:44:42AM -0800, Rob Clark wrote: > On Wed, Feb 22, 2023 at 1:57 AM Pekka Paalanen wrote: > > > > On Tue, 21 Feb 2023 09:50:20 -0800 > > Rob Clark wrote: > > > > > On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä > > > wrote: > > > > > > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > > > Rob Clark wrote: > > > > > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > > > wrote: > > > > > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > > > Rob Clark wrote: > > > > > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences > > > > > > > > that an > > > > > > > > atomic update is waiting on. > > > > > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > > > --- > > > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > > > > > > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > > > drm_crtc *crtc, > > > > > > > > } > > > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > > > > > +/** > > > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > > > vblank > > > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > > > timestamp. > > > > > > > > + * > > > > > > > > + * Calculate the expected time of the next vblank based on > > > > > > > > time of previous > > > > > > > > + * vblank and frame duration > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > for VRR this targets the highest frame rate possible for the > > > > > > > current > > > > > > > VRR mode, right? > > > > > > > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > > > > > I don't know. :-) > > > > > > > > At least for i915 this will give you the maximum frame > > > > duration. > > > > > > I suppose one could argue that maximum frame duration is the actual > > > deadline. Anything less is just moar fps, but not going to involve > > > stalling until vblank N+1, AFAIU > > > > > > > Also this does not calculate the the start of vblank, it > > > > calculates the start of active video. > > > > > > Probably something like end of previous frame's video.. might not be > > > _exactly_ correct (because some buffering involved), but OTOH on the > > > GPU side, I expect the driver to set a timer for a few ms or so before > > > the deadline. So there is some wiggle room. > > > > The vblank timestamp is defined to be the time of the first active > > pixel of the frame in the video signal. At least that's the one that > > UAPI carries (when not tearing?). It is not the start of vblank period. > > > > With VRR, the front porch before the first active pixel can be multiple > > milliseconds. The difference between 144 Hz and 60 Hz is 9.7 ms for > > example. > > What we really want is the deadline for the hw to latch for the next > frame.. which as Ville pointed out is definitely before the end of > vblank. > > Honestly this sort of feature is a lot more critical for the non-VRR > case, and VRR is kind of a minority edge case. So I'd prefer not to > get too hung up on VRR. If there is an easy way for the helpers to > detect VRR, I'd be perfectly fine not setting a deadline hint in that > case, and let someone who actually has a VRR display figure out how to > handle that case. The formula I gave you earlier works for both VRR and non-VRR. -- Ville Syrjälä Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Wed, Feb 22, 2023 at 2:37 AM Luben Tuikov wrote:
>
> On 2023-02-18 16:15, Rob Clark wrote:
> > From: Rob Clark
> >
> > Will be used in the next commit to set a deadline on fences that an
> > atomic update is waiting on.
> >
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/drm_vblank.c | 32
> > include/drm/drm_vblank.h | 1 +
> > 2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 2ff31717a3de..caf25ebb34c5 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc
> > *crtc,
> > }
> > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> >
> > +/**
> > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > + * @crtc: the crtc for which to calculate next vblank time
> > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > + *
> > + * Calculate the expected time of the next vblank based on time of previous
> > + * vblank and frame duration
> > + */
> > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> > +{
> > + unsigned int pipe = drm_crtc_index(crtc);
> > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > + u64 count;
> > +
> > + if (!vblank->framedur_ns)
> > + return -EINVAL;
> > +
> > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > +
> > + /*
> > + * If we don't get a valid count, then we probably also don't
> > + * have a valid time:
> > + */
> > + if (!count)
> > + return -EINVAL;
> > +
> > + *vblanktime = ktime_add(*vblanktime,
> > ns_to_ktime(vblank->framedur_ns));
>
> I'd rather this not do any arithmetic, i.e. add, and simply return the
> calculated
> remaining time, i.e. time left--so instead of add, it would simply assign
> the remaining time, and possibly rename the vblanktime to something like
> "time_to_vblank."
>
Note that since I sent the last iteration, I've renamed it to
drm_crtc_next_vblank_start().
I would prefer to keep the arithmetic, because I have another use for
this helper in drm/msm (for async/cursor updates, where we want to set
an hrtimer for start of vblank). It is a bit off the topic of this
series, but I can include the patch when I repost.
BR,
-R
> Changing the top comment to "calculate the time remaining to the next vblank".
> --
> Regards,
> Luben
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > +
> > static void send_vblank_event(struct drm_device *dev,
> > struct drm_pending_vblank_event *e,
> > u64 seq, ktime_t now)
> > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > index 733a3e2d1d10..a63bc2c92f3c 100644
> > --- a/include/drm/drm_vblank.h
> > +++ b/include/drm/drm_vblank.h
> > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > ktime_t *vblanktime);
> > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime);
> > 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,
>
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Wed, Feb 22, 2023 at 1:57 AM Pekka Paalanen wrote: > > On Tue, 21 Feb 2023 09:50:20 -0800 > Rob Clark wrote: > > > On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä > > wrote: > > > > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > > Rob Clark wrote: > > > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > > wrote: > > > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > > Rob Clark wrote: > > > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences that > > > > > > > an > > > > > > > atomic update is waiting on. > > > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > > --- > > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > > > > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > > drm_crtc *crtc, > > > > > > > } > > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > > > +/** > > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > > vblank > > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > > timestamp. > > > > > > > + * > > > > > > > + * Calculate the expected time of the next vblank based on time > > > > > > > of previous > > > > > > > + * vblank and frame duration > > > > > > > > > > > > Hi, > > > > > > > > > > > > for VRR this targets the highest frame rate possible for the current > > > > > > VRR mode, right? > > > > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > > > I don't know. :-) > > > > > > At least for i915 this will give you the maximum frame > > > duration. > > > > I suppose one could argue that maximum frame duration is the actual > > deadline. Anything less is just moar fps, but not going to involve > > stalling until vblank N+1, AFAIU > > > > > Also this does not calculate the the start of vblank, it > > > calculates the start of active video. > > > > Probably something like end of previous frame's video.. might not be > > _exactly_ correct (because some buffering involved), but OTOH on the > > GPU side, I expect the driver to set a timer for a few ms or so before > > the deadline. So there is some wiggle room. > > The vblank timestamp is defined to be the time of the first active > pixel of the frame in the video signal. At least that's the one that > UAPI carries (when not tearing?). It is not the start of vblank period. > > With VRR, the front porch before the first active pixel can be multiple > milliseconds. The difference between 144 Hz and 60 Hz is 9.7 ms for > example. What we really want is the deadline for the hw to latch for the next frame.. which as Ville pointed out is definitely before the end of vblank. Honestly this sort of feature is a lot more critical for the non-VRR case, and VRR is kind of a minority edge case. So I'd prefer not to get too hung up on VRR. If there is an easy way for the helpers to detect VRR, I'd be perfectly fine not setting a deadline hint in that case, and let someone who actually has a VRR display figure out how to handle that case. BR, -R
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On 2023-02-18 16:15, Rob Clark wrote:
> From: Rob Clark
>
> Will be used in the next commit to set a deadline on fences that an
> atomic update is waiting on.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_vblank.c | 32
> include/drm/drm_vblank.h | 1 +
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 2ff31717a3de..caf25ebb34c5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
>
> +/**
> + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> + * @crtc: the crtc for which to calculate next vblank time
> + * @vblanktime: pointer to time to receive the next vblank timestamp.
> + *
> + * Calculate the expected time of the next vblank based on time of previous
> + * vblank and frame duration
> + */
> +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> +{
> + unsigned int pipe = drm_crtc_index(crtc);
> + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> + u64 count;
> +
> + if (!vblank->framedur_ns)
> + return -EINVAL;
> +
> + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> +
> + /*
> + * If we don't get a valid count, then we probably also don't
> + * have a valid time:
> + */
> + if (!count)
> + return -EINVAL;
> +
> + *vblanktime = ktime_add(*vblanktime, ns_to_ktime(vblank->framedur_ns));
I'd rather this not do any arithmetic, i.e. add, and simply return the
calculated
remaining time, i.e. time left--so instead of add, it would simply assign
the remaining time, and possibly rename the vblanktime to something like
"time_to_vblank."
Changing the top comment to "calculate the time remaining to the next vblank".
--
Regards,
Luben
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> +
> static void send_vblank_event(struct drm_device *dev,
> struct drm_pending_vblank_event *e,
> u64 seq, ktime_t now)
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 733a3e2d1d10..a63bc2c92f3c 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> ktime_t *vblanktime);
> +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime);
> 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,
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, 21 Feb 2023 09:50:20 -0800 Rob Clark wrote: > On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä > wrote: > > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > Rob Clark wrote: > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > wrote: > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > Rob Clark wrote: > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences that an > > > > > > atomic update is waiting on. > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > --- > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > drm_crtc *crtc, > > > > > > } > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > +/** > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > vblank > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > timestamp. > > > > > > + * > > > > > > + * Calculate the expected time of the next vblank based on time of > > > > > > previous > > > > > > + * vblank and frame duration > > > > > > > > > > Hi, > > > > > > > > > > for VRR this targets the highest frame rate possible for the current > > > > > VRR mode, right? > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > I don't know. :-) > > > > At least for i915 this will give you the maximum frame > > duration. > > I suppose one could argue that maximum frame duration is the actual > deadline. Anything less is just moar fps, but not going to involve > stalling until vblank N+1, AFAIU > > > Also this does not calculate the the start of vblank, it > > calculates the start of active video. > > Probably something like end of previous frame's video.. might not be > _exactly_ correct (because some buffering involved), but OTOH on the > GPU side, I expect the driver to set a timer for a few ms or so before > the deadline. So there is some wiggle room. The vblank timestamp is defined to be the time of the first active pixel of the frame in the video signal. At least that's the one that UAPI carries (when not tearing?). It is not the start of vblank period. With VRR, the front porch before the first active pixel can be multiple milliseconds. The difference between 144 Hz and 60 Hz is 9.7 ms for example. Thanks, pq pgpWIStHIkryN.pgp Description: OpenPGP digital signature
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 11:39:40PM +0200, Ville Syrjälä wrote: > On Tue, Feb 21, 2023 at 11:54:55AM -0800, Rob Clark wrote: > > On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä > > wrote: > > > > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > > Rob Clark wrote: > > > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > > wrote: > > > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > > Rob Clark wrote: > > > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences that > > > > > > > an > > > > > > > atomic update is waiting on. > > > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > > --- > > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > > > > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > > drm_crtc *crtc, > > > > > > > } > > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > > > +/** > > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > > vblank > > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > > timestamp. > > > > > > > + * > > > > > > > + * Calculate the expected time of the next vblank based on time > > > > > > > of previous > > > > > > > + * vblank and frame duration > > > > > > > > > > > > Hi, > > > > > > > > > > > > for VRR this targets the highest frame rate possible for the current > > > > > > VRR mode, right? > > > > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > > > I don't know. :-) > > > > > > At least for i915 this will give you the maximum frame > > > duration. > > > > > > Also this does not calculate the the start of vblank, it > > > calculates the start of active video. > > > > AFAIU, vsync_end/vsync_start are in units of line, so I could do something > > like: > > > > vsync_lines = vblank->hwmode.vsync_end - vblank->hwmode.vsync_start; > > vsyncdur = ns_to_ktime(vblank->linedur_ns * vsync_lines); > > framedur = ns_to_ktime(vblank->framedur_ns); > > *vblanktime = ktime_add(*vblanktime, ktime_sub(framedur, vsyncdur)); > > Something like this should work: > vblank_start = framedur_ns * crtc_vblank_start / crtc_vtotal > deadline = vblanktime + vblank_start > > That would be the expected time when the next start of vblank > happens. Except that drm_vblank_count_and_time() will give you the last sampled timestamp, which may be long ago in the past. Would need to add an _accurate version of that if we want to be guaranteed a fresh sample. -- Ville Syrjälä Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 11:54:55AM -0800, Rob Clark wrote: > On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä > wrote: > > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > Rob Clark wrote: > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > wrote: > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > Rob Clark wrote: > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences that an > > > > > > atomic update is waiting on. > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > --- > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > drm_crtc *crtc, > > > > > > } > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > +/** > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > vblank > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > timestamp. > > > > > > + * > > > > > > + * Calculate the expected time of the next vblank based on time of > > > > > > previous > > > > > > + * vblank and frame duration > > > > > > > > > > Hi, > > > > > > > > > > for VRR this targets the highest frame rate possible for the current > > > > > VRR mode, right? > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > I don't know. :-) > > > > At least for i915 this will give you the maximum frame > > duration. > > > > Also this does not calculate the the start of vblank, it > > calculates the start of active video. > > AFAIU, vsync_end/vsync_start are in units of line, so I could do something > like: > > vsync_lines = vblank->hwmode.vsync_end - vblank->hwmode.vsync_start; > vsyncdur = ns_to_ktime(vblank->linedur_ns * vsync_lines); > framedur = ns_to_ktime(vblank->framedur_ns); > *vblanktime = ktime_add(*vblanktime, ktime_sub(framedur, vsyncdur)); Something like this should work: vblank_start = framedur_ns * crtc_vblank_start / crtc_vtotal deadline = vblanktime + vblank_start That would be the expected time when the next start of vblank happens. -- Ville Syrjälä Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä
wrote:
>
> On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote:
> > On Mon, 20 Feb 2023 07:55:41 -0800
> > Rob Clark wrote:
> >
> > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen
> > > wrote:
> > > >
> > > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > > Rob Clark wrote:
> > > >
> > > > > From: Rob Clark
> > > > >
> > > > > Will be used in the next commit to set a deadline on fences that an
> > > > > atomic update is waiting on.
> > > > >
> > > > > Signed-off-by: Rob Clark
> > > > > ---
> > > > > drivers/gpu/drm/drm_vblank.c | 32
> > > > > include/drm/drm_vblank.h | 1 +
> > > > > 2 files changed, 33 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 2ff31717a3de..caf25ebb34c5 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct
> > > > > drm_crtc *crtc,
> > > > > }
> > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > > > >
> > > > > +/**
> > > > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > > > + * @crtc: the crtc for which to calculate next vblank time
> > > > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > > > + *
> > > > > + * Calculate the expected time of the next vblank based on time of
> > > > > previous
> > > > > + * vblank and frame duration
> > > >
> > > > Hi,
> > > >
> > > > for VRR this targets the highest frame rate possible for the current
> > > > VRR mode, right?
> > > >
> > >
> > > It is based on vblank->framedur_ns which is in turn based on
> > > mode->crtc_clock. Presumably for VRR that ends up being a maximum?
> >
> > I don't know. :-)
>
> At least for i915 this will give you the maximum frame
> duration.
>
> Also this does not calculate the the start of vblank, it
> calculates the start of active video.
AFAIU, vsync_end/vsync_start are in units of line, so I could do something like:
vsync_lines = vblank->hwmode.vsync_end - vblank->hwmode.vsync_start;
vsyncdur = ns_to_ktime(vblank->linedur_ns * vsync_lines);
framedur = ns_to_ktime(vblank->framedur_ns);
*vblanktime = ktime_add(*vblanktime, ktime_sub(framedur, vsyncdur));
?
BR,
-R
>
> >
> > You need a number of clock cycles in addition to the clock frequency,
> > and that could still be minimum, maximum, the last realized one, ...
> >
> > VRR works by adjusting the front porch length IIRC.
> >
> >
> > Thanks,
> > pq
> >
> > > BR,
> > > -R
> > >
> > >
> > > >
> > > > Thanks,
> > > > pq
> > > >
> > > > > + */
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime)
> > > > > +{
> > > > > + unsigned int pipe = drm_crtc_index(crtc);
> > > > > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > > > + u64 count;
> > > > > +
> > > > > + if (!vblank->framedur_ns)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > > > +
> > > > > + /*
> > > > > + * If we don't get a valid count, then we probably also don't
> > > > > + * have a valid time:
> > > > > + */
> > > > > + if (!count)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + *vblanktime = ktime_add(*vblanktime,
> > > > > ns_to_ktime(vblank->framedur_ns));
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > > > +
> > > > > static void send_vblank_event(struct drm_device *dev,
> > > > > struct drm_pending_vblank_event *e,
> > > > > u64 seq, ktime_t now)
> > > > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > > > --- a/include/drm/drm_vblank.h
> > > > > +++ b/include/drm/drm_vblank.h
> > > > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device
> > > > > *dev);
> > > > > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > > > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > > ktime_t *vblanktime);
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime);
> > > > > 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,
> > > >
> >
>
>
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 5:01 AM Ville Syrjälä
wrote:
>
> On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote:
> > On Mon, 20 Feb 2023 07:55:41 -0800
> > Rob Clark wrote:
> >
> > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen
> > > wrote:
> > > >
> > > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > > Rob Clark wrote:
> > > >
> > > > > From: Rob Clark
> > > > >
> > > > > Will be used in the next commit to set a deadline on fences that an
> > > > > atomic update is waiting on.
> > > > >
> > > > > Signed-off-by: Rob Clark
> > > > > ---
> > > > > drivers/gpu/drm/drm_vblank.c | 32
> > > > > include/drm/drm_vblank.h | 1 +
> > > > > 2 files changed, 33 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 2ff31717a3de..caf25ebb34c5 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct
> > > > > drm_crtc *crtc,
> > > > > }
> > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > > > >
> > > > > +/**
> > > > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > > > + * @crtc: the crtc for which to calculate next vblank time
> > > > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > > > + *
> > > > > + * Calculate the expected time of the next vblank based on time of
> > > > > previous
> > > > > + * vblank and frame duration
> > > >
> > > > Hi,
> > > >
> > > > for VRR this targets the highest frame rate possible for the current
> > > > VRR mode, right?
> > > >
> > >
> > > It is based on vblank->framedur_ns which is in turn based on
> > > mode->crtc_clock. Presumably for VRR that ends up being a maximum?
> >
> > I don't know. :-)
>
> At least for i915 this will give you the maximum frame
> duration.
I suppose one could argue that maximum frame duration is the actual
deadline. Anything less is just moar fps, but not going to involve
stalling until vblank N+1, AFAIU
> Also this does not calculate the the start of vblank, it
> calculates the start of active video.
Probably something like end of previous frame's video.. might not be
_exactly_ correct (because some buffering involved), but OTOH on the
GPU side, I expect the driver to set a timer for a few ms or so before
the deadline. So there is some wiggle room.
BR,
-R
> >
> > You need a number of clock cycles in addition to the clock frequency,
> > and that could still be minimum, maximum, the last realized one, ...
> >
> > VRR works by adjusting the front porch length IIRC.
> >
> >
> > Thanks,
> > pq
> >
> > > BR,
> > > -R
> > >
> > >
> > > >
> > > > Thanks,
> > > > pq
> > > >
> > > > > + */
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime)
> > > > > +{
> > > > > + unsigned int pipe = drm_crtc_index(crtc);
> > > > > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > > > + u64 count;
> > > > > +
> > > > > + if (!vblank->framedur_ns)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > > > +
> > > > > + /*
> > > > > + * If we don't get a valid count, then we probably also don't
> > > > > + * have a valid time:
> > > > > + */
> > > > > + if (!count)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + *vblanktime = ktime_add(*vblanktime,
> > > > > ns_to_ktime(vblank->framedur_ns));
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > > > +
> > > > > static void send_vblank_event(struct drm_device *dev,
> > > > > struct drm_pending_vblank_event *e,
> > > > > u64 seq, ktime_t now)
> > > > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > > > --- a/include/drm/drm_vblank.h
> > > > > +++ b/include/drm/drm_vblank.h
> > > > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device
> > > > > *dev);
> > > > > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > > > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > > ktime_t *vblanktime);
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime);
> > > > > 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,
> > > >
> >
>
>
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 03:11:33PM +0200, Pekka Paalanen wrote: > On Tue, 21 Feb 2023 15:01:35 +0200 > Ville Syrjälä wrote: > > > On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote: > > > On Mon, 20 Feb 2023 07:55:41 -0800 > > > Rob Clark wrote: > > > > > > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen > > > > wrote: > > > > > > > > > > On Sat, 18 Feb 2023 13:15:53 -0800 > > > > > Rob Clark wrote: > > > > > > > > > > > From: Rob Clark > > > > > > > > > > > > Will be used in the next commit to set a deadline on fences that an > > > > > > atomic update is waiting on. > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > --- > > > > > > drivers/gpu/drm/drm_vblank.c | 32 > > > > > > include/drm/drm_vblank.h | 1 + > > > > > > 2 files changed, 33 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c > > > > > > b/drivers/gpu/drm/drm_vblank.c > > > > > > index 2ff31717a3de..caf25ebb34c5 100644 > > > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct > > > > > > drm_crtc *crtc, > > > > > > } > > > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time); > > > > > > > > > > > > +/** > > > > > > + * drm_crtc_next_vblank_time - calculate the time of the next > > > > > > vblank > > > > > > + * @crtc: the crtc for which to calculate next vblank time > > > > > > + * @vblanktime: pointer to time to receive the next vblank > > > > > > timestamp. > > > > > > + * > > > > > > + * Calculate the expected time of the next vblank based on time of > > > > > > previous > > > > > > + * vblank and frame duration > > > > > > > > > > Hi, > > > > > > > > > > for VRR this targets the highest frame rate possible for the current > > > > > VRR mode, right? > > > > > > > > > > > > > It is based on vblank->framedur_ns which is in turn based on > > > > mode->crtc_clock. Presumably for VRR that ends up being a maximum? > > > > > > I don't know. :-) > > > > At least for i915 this will give you the maximum frame > > duration. > > Really maximum duration? So minimum VRR frequency? Yes. Doing otherwise would complicate the actual timestamp calculation even further. The actual timestamps i915 generates will however match the start of active video, regardless of how long vblank was extended. The only exception might be if you query the timestamp during vblank but VRR exit has not yet been triggered, ie. not commit has been made during the frame. In that case the timestamp will correspond to the max frame duration, which may or may not end up being the case. Depends totally whether a commit will still happen during the vblank to trigger an early vblank exit. > > > Also this does not calculate the the start of vblank, it > > calculates the start of active video. > > Oh indeed, so it's too late. What one would actually need for the > deadline is the driver's deadline to present for the immediately next > start of active video. > > And with VRR that should probably aim for the maximum frame frequency, > not minimum? Yeah, max frame rate seems like the easiest thing to use there. The other option might be some average value based on recent history, but figuring tht out would seem like a lot more work. -- Ville Syrjälä Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, 21 Feb 2023 15:01:35 +0200
Ville Syrjälä wrote:
> On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote:
> > On Mon, 20 Feb 2023 07:55:41 -0800
> > Rob Clark wrote:
> >
> > > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen
> > > wrote:
> > > >
> > > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > > Rob Clark wrote:
> > > >
> > > > > From: Rob Clark
> > > > >
> > > > > Will be used in the next commit to set a deadline on fences that an
> > > > > atomic update is waiting on.
> > > > >
> > > > > Signed-off-by: Rob Clark
> > > > > ---
> > > > > drivers/gpu/drm/drm_vblank.c | 32
> > > > > include/drm/drm_vblank.h | 1 +
> > > > > 2 files changed, 33 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_vblank.c
> > > > > b/drivers/gpu/drm/drm_vblank.c
> > > > > index 2ff31717a3de..caf25ebb34c5 100644
> > > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct
> > > > > drm_crtc *crtc,
> > > > > }
> > > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > > > >
> > > > > +/**
> > > > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > > > + * @crtc: the crtc for which to calculate next vblank time
> > > > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > > > + *
> > > > > + * Calculate the expected time of the next vblank based on time of
> > > > > previous
> > > > > + * vblank and frame duration
> > > >
> > > > Hi,
> > > >
> > > > for VRR this targets the highest frame rate possible for the current
> > > > VRR mode, right?
> > > >
> > >
> > > It is based on vblank->framedur_ns which is in turn based on
> > > mode->crtc_clock. Presumably for VRR that ends up being a maximum?
> >
> > I don't know. :-)
>
> At least for i915 this will give you the maximum frame
> duration.
Really maximum duration? So minimum VRR frequency?
> Also this does not calculate the the start of vblank, it
> calculates the start of active video.
Oh indeed, so it's too late. What one would actually need for the
deadline is the driver's deadline to present for the immediately next
start of active video.
And with VRR that should probably aim for the maximum frame frequency,
not minimum?
Thanks,
pq
>
> >
> > You need a number of clock cycles in addition to the clock frequency,
> > and that could still be minimum, maximum, the last realized one, ...
> >
> > VRR works by adjusting the front porch length IIRC.
> >
> >
> > Thanks,
> > pq
> >
> > > BR,
> > > -R
> > >
> > >
> > > >
> > > > Thanks,
> > > > pq
> > > >
> > > > > + */
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime)
> > > > > +{
> > > > > + unsigned int pipe = drm_crtc_index(crtc);
> > > > > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > > > + u64 count;
> > > > > +
> > > > > + if (!vblank->framedur_ns)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > > > +
> > > > > + /*
> > > > > + * If we don't get a valid count, then we probably also don't
> > > > > + * have a valid time:
> > > > > + */
> > > > > + if (!count)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + *vblanktime = ktime_add(*vblanktime,
> > > > > ns_to_ktime(vblank->framedur_ns));
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > > > +
> > > > > static void send_vblank_event(struct drm_device *dev,
> > > > > struct drm_pending_vblank_event *e,
> > > > > u64 seq, ktime_t now)
> > > > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > > > --- a/include/drm/drm_vblank.h
> > > > > +++ b/include/drm/drm_vblank.h
> > > > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device
> > > > > *dev);
> > > > > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > > > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > > ktime_t *vblanktime);
> > > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > > *vblanktime);
> > > > > 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,
> > > >
> >
>
>
>
pgp0k2h598k2l.pgp
Description: OpenPGP digital signature
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Tue, Feb 21, 2023 at 10:45:51AM +0200, Pekka Paalanen wrote:
> On Mon, 20 Feb 2023 07:55:41 -0800
> Rob Clark wrote:
>
> > On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen wrote:
> > >
> > > On Sat, 18 Feb 2023 13:15:53 -0800
> > > Rob Clark wrote:
> > >
> > > > From: Rob Clark
> > > >
> > > > Will be used in the next commit to set a deadline on fences that an
> > > > atomic update is waiting on.
> > > >
> > > > Signed-off-by: Rob Clark
> > > > ---
> > > > drivers/gpu/drm/drm_vblank.c | 32
> > > > include/drm/drm_vblank.h | 1 +
> > > > 2 files changed, 33 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > > index 2ff31717a3de..caf25ebb34c5 100644
> > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc
> > > > *crtc,
> > > > }
> > > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > > >
> > > > +/**
> > > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > > + * @crtc: the crtc for which to calculate next vblank time
> > > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > > + *
> > > > + * Calculate the expected time of the next vblank based on time of
> > > > previous
> > > > + * vblank and frame duration
> > >
> > > Hi,
> > >
> > > for VRR this targets the highest frame rate possible for the current
> > > VRR mode, right?
> > >
> >
> > It is based on vblank->framedur_ns which is in turn based on
> > mode->crtc_clock. Presumably for VRR that ends up being a maximum?
>
> I don't know. :-)
At least for i915 this will give you the maximum frame
duration.
Also this does not calculate the the start of vblank, it
calculates the start of active video.
>
> You need a number of clock cycles in addition to the clock frequency,
> and that could still be minimum, maximum, the last realized one, ...
>
> VRR works by adjusting the front porch length IIRC.
>
>
> Thanks,
> pq
>
> > BR,
> > -R
> >
> >
> > >
> > > Thanks,
> > > pq
> > >
> > > > + */
> > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > *vblanktime)
> > > > +{
> > > > + unsigned int pipe = drm_crtc_index(crtc);
> > > > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > > + u64 count;
> > > > +
> > > > + if (!vblank->framedur_ns)
> > > > + return -EINVAL;
> > > > +
> > > > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > > +
> > > > + /*
> > > > + * If we don't get a valid count, then we probably also don't
> > > > + * have a valid time:
> > > > + */
> > > > + if (!count)
> > > > + return -EINVAL;
> > > > +
> > > > + *vblanktime = ktime_add(*vblanktime,
> > > > ns_to_ktime(vblank->framedur_ns));
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > > +
> > > > static void send_vblank_event(struct drm_device *dev,
> > > > struct drm_pending_vblank_event *e,
> > > > u64 seq, ktime_t now)
> > > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > > --- a/include/drm/drm_vblank.h
> > > > +++ b/include/drm/drm_vblank.h
> > > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device
> > > > *dev);
> > > > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > > ktime_t *vblanktime);
> > > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > > *vblanktime);
> > > > 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,
> > >
>
--
Ville Syrjälä
Intel
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Mon, 20 Feb 2023 07:55:41 -0800
Rob Clark wrote:
> On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen wrote:
> >
> > On Sat, 18 Feb 2023 13:15:53 -0800
> > Rob Clark wrote:
> >
> > > From: Rob Clark
> > >
> > > Will be used in the next commit to set a deadline on fences that an
> > > atomic update is waiting on.
> > >
> > > Signed-off-by: Rob Clark
> > > ---
> > > drivers/gpu/drm/drm_vblank.c | 32
> > > include/drm/drm_vblank.h | 1 +
> > > 2 files changed, 33 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > index 2ff31717a3de..caf25ebb34c5 100644
> > > --- a/drivers/gpu/drm/drm_vblank.c
> > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc
> > > *crtc,
> > > }
> > > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> > >
> > > +/**
> > > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > > + * @crtc: the crtc for which to calculate next vblank time
> > > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > > + *
> > > + * Calculate the expected time of the next vblank based on time of
> > > previous
> > > + * vblank and frame duration
> >
> > Hi,
> >
> > for VRR this targets the highest frame rate possible for the current
> > VRR mode, right?
> >
>
> It is based on vblank->framedur_ns which is in turn based on
> mode->crtc_clock. Presumably for VRR that ends up being a maximum?
I don't know. :-)
You need a number of clock cycles in addition to the clock frequency,
and that could still be minimum, maximum, the last realized one, ...
VRR works by adjusting the front porch length IIRC.
Thanks,
pq
> BR,
> -R
>
>
> >
> > Thanks,
> > pq
> >
> > > + */
> > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> > > +{
> > > + unsigned int pipe = drm_crtc_index(crtc);
> > > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > > + u64 count;
> > > +
> > > + if (!vblank->framedur_ns)
> > > + return -EINVAL;
> > > +
> > > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > > +
> > > + /*
> > > + * If we don't get a valid count, then we probably also don't
> > > + * have a valid time:
> > > + */
> > > + if (!count)
> > > + return -EINVAL;
> > > +
> > > + *vblanktime = ktime_add(*vblanktime,
> > > ns_to_ktime(vblank->framedur_ns));
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > > +
> > > static void send_vblank_event(struct drm_device *dev,
> > > struct drm_pending_vblank_event *e,
> > > u64 seq, ktime_t now)
> > > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > > index 733a3e2d1d10..a63bc2c92f3c 100644
> > > --- a/include/drm/drm_vblank.h
> > > +++ b/include/drm/drm_vblank.h
> > > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> > > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > > ktime_t *vblanktime);
> > > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t
> > > *vblanktime);
> > > 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,
> >
pgpfJHRTdk6MZ.pgp
Description: OpenPGP digital signature
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Mon, Feb 20, 2023 at 1:08 AM Pekka Paalanen wrote:
>
> On Sat, 18 Feb 2023 13:15:53 -0800
> Rob Clark wrote:
>
> > From: Rob Clark
> >
> > Will be used in the next commit to set a deadline on fences that an
> > atomic update is waiting on.
> >
> > Signed-off-by: Rob Clark
> > ---
> > drivers/gpu/drm/drm_vblank.c | 32
> > include/drm/drm_vblank.h | 1 +
> > 2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > index 2ff31717a3de..caf25ebb34c5 100644
> > --- a/drivers/gpu/drm/drm_vblank.c
> > +++ b/drivers/gpu/drm/drm_vblank.c
> > @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc
> > *crtc,
> > }
> > EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
> >
> > +/**
> > + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> > + * @crtc: the crtc for which to calculate next vblank time
> > + * @vblanktime: pointer to time to receive the next vblank timestamp.
> > + *
> > + * Calculate the expected time of the next vblank based on time of previous
> > + * vblank and frame duration
>
> Hi,
>
> for VRR this targets the highest frame rate possible for the current
> VRR mode, right?
>
It is based on vblank->framedur_ns which is in turn based on
mode->crtc_clock. Presumably for VRR that ends up being a maximum?
BR,
-R
>
> Thanks,
> pq
>
> > + */
> > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> > +{
> > + unsigned int pipe = drm_crtc_index(crtc);
> > + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> > + u64 count;
> > +
> > + if (!vblank->framedur_ns)
> > + return -EINVAL;
> > +
> > + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> > +
> > + /*
> > + * If we don't get a valid count, then we probably also don't
> > + * have a valid time:
> > + */
> > + if (!count)
> > + return -EINVAL;
> > +
> > + *vblanktime = ktime_add(*vblanktime,
> > ns_to_ktime(vblank->framedur_ns));
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> > +
> > static void send_vblank_event(struct drm_device *dev,
> > struct drm_pending_vblank_event *e,
> > u64 seq, ktime_t now)
> > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> > index 733a3e2d1d10..a63bc2c92f3c 100644
> > --- a/include/drm/drm_vblank.h
> > +++ b/include/drm/drm_vblank.h
> > @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> > u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> > u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> > ktime_t *vblanktime);
> > +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime);
> > 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,
>
Re: [PATCH v4 10/14] drm/vblank: Add helper to get next vblank time
On Sat, 18 Feb 2023 13:15:53 -0800
Rob Clark wrote:
> From: Rob Clark
>
> Will be used in the next commit to set a deadline on fences that an
> atomic update is waiting on.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/drm_vblank.c | 32
> include/drm/drm_vblank.h | 1 +
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 2ff31717a3de..caf25ebb34c5 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -980,6 +980,38 @@ u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> }
> EXPORT_SYMBOL(drm_crtc_vblank_count_and_time);
>
> +/**
> + * drm_crtc_next_vblank_time - calculate the time of the next vblank
> + * @crtc: the crtc for which to calculate next vblank time
> + * @vblanktime: pointer to time to receive the next vblank timestamp.
> + *
> + * Calculate the expected time of the next vblank based on time of previous
> + * vblank and frame duration
Hi,
for VRR this targets the highest frame rate possible for the current
VRR mode, right?
Thanks,
pq
> + */
> +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime)
> +{
> + unsigned int pipe = drm_crtc_index(crtc);
> + struct drm_vblank_crtc *vblank = &crtc->dev->vblank[pipe];
> + u64 count;
> +
> + if (!vblank->framedur_ns)
> + return -EINVAL;
> +
> + count = drm_vblank_count_and_time(crtc->dev, pipe, vblanktime);
> +
> + /*
> + * If we don't get a valid count, then we probably also don't
> + * have a valid time:
> + */
> + if (!count)
> + return -EINVAL;
> +
> + *vblanktime = ktime_add(*vblanktime, ns_to_ktime(vblank->framedur_ns));
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_next_vblank_time);
> +
> static void send_vblank_event(struct drm_device *dev,
> struct drm_pending_vblank_event *e,
> u64 seq, ktime_t now)
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 733a3e2d1d10..a63bc2c92f3c 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -230,6 +230,7 @@ bool drm_dev_has_vblank(const struct drm_device *dev);
> u64 drm_crtc_vblank_count(struct drm_crtc *crtc);
> u64 drm_crtc_vblank_count_and_time(struct drm_crtc *crtc,
> ktime_t *vblanktime);
> +int drm_crtc_next_vblank_time(struct drm_crtc *crtc, ktime_t *vblanktime);
> 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,
pgpUU45qLklSo.pgp
Description: OpenPGP digital signature
