Re: [PATCH v2 09/27] drm: Add VRR target frame rate properties

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Thursday, 12 November 2026 06:24:50 Central European Standard Time Vidith Madhu wrote:
> 
> On Thu, 5 Nov 2026, Nicolas Frattaroli wrote:
> 
> > Userspace may wish to set a target frame rate for the Variable Refresh
> > Rate mechanism. There are multiple possible ways such a target could be
> > communicated.
> > 
> > It's desirable for userspace to not only set a fixed target, but a
> > target range within which VRR may operate. This is because even if a
> > sink advertises a certain range as supported, said range may not be the
> > ideal range to use, as numbers on the extreme end of the range may
> > induce flickering or other glitches.
> > 
> > There's four choices to how one bracketing value for the target range
> > could be communicated:
> > 1. Target frame rate in something like millihertz
> > 2. Target frame period in something like usecs
> > 3. Target vtotal value
> > 4. Target frame rate as a fraction expressed by two integers
> > 
> > Choice 1 is unpleasant as it would introduce some amount of rounding to
> > common broadcast framerates like 24/1.001, which seems minor except that
> > exact matches to standard framerates would rely on making all userspace
> > and kernel agree on a specific method of rounding. This is sure to go
> > wrong, so is disqualified.
> > 
> > Choice 2 has the same problem as 1, with the added complication that
> > the EDID's VRR range is expressed in frames per second, and other HDMI
> > features express things in frames per second as well, which would
> > require possibly imprecise computations.
> > 
> > Choice 3 is disqualified as mechanisms other than VRR, such as FVA, may
> > also modify the vtotal value. Additionally, it has the same drawbacks of
> > choice 2 and 1.
> > 
> > Choice 4 is therefore what's implemented. The frame rate 24/1.001Hz, for
> > example, is expressed as nominator=24000 and denominator=1001. As there
> > is both a minimum and a maximum, there are 4 properties in total: two
> > numerators, and two denominators. A fixed rate with no variability is
> > expressed by setting the minimum fractional and the maximum fractional
> > properties to the same value.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_atomic.c      | 14 ++++++++++++
> >  drivers/gpu/drm/drm_atomic_uapi.c | 24 ++++++++++++++++++++
> >  drivers/gpu/drm/drm_connector.c   | 48 +++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_crtc.c        | 34 +++++++++++++++++++++++++++
> >  drivers/gpu/drm/drm_mode_config.c | 20 ++++++++++++++++
> >  include/drm/drm_crtc.h            |  6 +++++
> >  include/drm/drm_mode_config.h     | 45 ++++++++++++++++++++++++++++++++++++
> >  7 files changed, 191 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 9bb761eda59f..da5c6925c349 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -489,6 +489,7 @@ EXPORT_SYMBOL(drm_atomic_get_crtc_state);
> >  static int drm_atomic_crtc_check(const struct drm_crtc_state *old_crtc_state,
> >  				 const struct drm_crtc_state *new_crtc_state)
> >  {
> > +	const struct drm_crtc_vrr_state *vrr = &new_crtc_state->vrr_state;
> >  	struct drm_crtc *crtc = new_crtc_state->crtc;
> >  
> >  	/* NOTE: we explicitly don't enforce constraints such as primary
> > @@ -544,6 +545,19 @@ static int drm_atomic_crtc_check(const struct drm_crtc_state *old_crtc_state,
> >  		return -EINVAL;
> >  	}
> >  
> > +	/*
> > +	 * Reject VRR limiter ranges that only specify one end of the limit, or
> I think specifying just one end of the limit should still be a valid case, 
> the client may want to use the default bound on one side implied by the 
> base timings/min. refresh rate and constrain the other.

That is what the RFC did, but unfortunately this leads to problems for
the kernel and non-obvious behaviour for userspace. Specifically, userspace
may set what is a fixed rate without intending to do so, and the kernel needs
to do a chicken-and-egg dance to determine whether something could be a fixed
rate to see if CinemaVRR lowering the minimum rate applies at which point it
may no longer be a fixed rate.

The new display-min/max properties are so that userspace knows what limits the
kernel is using (so that it factors in quirks applied outside of what's in the
EDID) and get to essentially specify one end that way.

> 
> > +	 * specify a numerator > 0 with a denominator that is 0.
> > +	 */
> > +	if ((!vrr->vrr_min_n || !vrr->vrr_min_d ||
> > +	     !vrr->vrr_max_n || !vrr->vrr_max_d) &&
> > +	    (vrr->vrr_min_n > 0 || vrr->vrr_max_n > 0)) {
> > +		drm_dbg_atomic(crtc->dev,
> > +			       "[CRTC:%d:%s] incomplete VRR limiter range\n",
> > +			       crtc->base.id, crtc->name);
> > +		return -EINVAL;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
>> [snip]





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip