Re: [PATCH RFC 13/25] drm: Add VRR target frame rate properties

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

On Tuesday, 29 September 2026 20:14:34 Central European Summer Time Nicolas Frattaroli wrote:
> On Tuesday, 29 September 2026 16:34:58 Central European Summer Time Leo Li wrote:
> > 
> > On 2026-09-28 04:10, Michel Dänzer wrote:
> > >>> You can picture VRR limiting as always being active, but with a limit rational
> > >>> of 0 it uses the display's limit as per the EDID, which is what unlimited game
> > >>> mode is. So with how it's implemented right now in hdmi_validate_vrr(), your
> > >>> example would set a maximum target, but leave the minimum at whatever the
> > >>> display defaults to.
> > >>>
> > >>> Now that I'm thinking through this, a possible problem is that
> > >>> drm_crtc_helper_vrr_is_fixed_rate() operates on the user supplied limits, but
> > >>> if the display supplied lower limit is equal to the user supplied upper limit,
> > >>> then we have a fixed rate scenario without recognising it as such. I think I
> > >>> need to have a ponder on what the least surprising behaviour for userspace
> > >>> is in that instance. The display limit stuff gets a bit complex due to
> > >>> CinemaVRR and QMS TFRmin/TFRmax.
> > >>>
> > >>> I'll improve the documentation on the next revision to make the meanings more
> > >>> explicit.
> > >> Perhaps a simple way is to require simultaneous setting MIN and MAX pairs?
> > >> IOW, require userspace to set MIN and MAX simultaneously to >0, or =0. For example:
> > >>
> > >> if ((vrr_min_n == 0 || vrr_min_d == 0 ||
> > >>      vrr_max_n == 0 || vrr_max_d == 0) &&
> > >>     (vrr_min_n > 0 || vrr_max_n > 0))
> > >> 	return -EINVAL;
> > >> That way, it's never ambiguous what userspace has requested for the range.
> > >> They can copy the EDID supported range if they don't care about limiting one side, rather than leaving it at 0.
> > > Determining the actual limits can be non-trivial (though I guess that might be fine as long as libdisplay-info can work them out), if user space gets them wrong, it might accidentally apply a narrower limit than intended.
> > > 
> > > 
> > >> It's then also clear if they requested a static Hz.
> > > I do see the benefit of your suggestion for this though.
> > 
> > Xaver and I were chatting about this at XDC, and yeah it'll be difficult
> > to match KMD's monitor range, especially if KMD decides to patch it with
> > quirks and whatnot.
> > 
> > Since we are handing compositors control over vrr range, does it sound
> > sensible to expose KMD's monitor range as a read-only property pair on
> > the drm connector? We probably don't need a num/den pair for it, it's
> > not like panels advertise fractional VRR ranges (right?).
> 
> Sounds good to me.
> 
> And yeah, we don't really need num/denom for it; the safe assumption is that the
> minimum range is expressed with a denominator of 1.001 whereas the maximum range
> is expressed with a numerator of 1. That's sort of non-obvious for userspace

err, *denominator of 1 here. It's late.

While I'm already sending this correction, I'm now thinking that we could also
either use special values (like 0/n again, but with reworked logic to get the
default frame rate?) or maybe really do have a numerator/denominator display
pair.

Entirely possible I'll keep the 0/n behaviour but refactor the "get range from
connector" part into its own thing. I think the hdmi_validate_vrr function
right now is trying to do too much and suffers in clarity and reusability as
a result.

> though, so I'll need to do some thinking around the specified behaviour.
> 
> This sounds like a mainly theoretical concern but it's a real one, the most
> common 1.001 rates we'll run into are likely 24/1.001 or 30/1.001 and those
> are also very reasonable for a monitor to have as a lower limit. And when
> they specify that lower limit, they'll do it as just the integer rounded
> value, but seemingly expect them to be understood as the 1.001 value for
> the lower limit.
> 
> > Fun fact: vrr_range is exposed today over debugfs for IGT testing
> > https://elixir.bootlin.com/linux/v7.3-rc5/source/drivers/gpu/drm/drm_debugfs.c#L586
> 
> Speaking of that, v2 will deduplicate my accidentally rebased-over
> mostly duplicated EDID parsing and fix this function to report the
> newly added vrr_min/vrr_max fields of the display_info. (Since
> apparently monitor_range is populated by VESA and messing with it
> may not be okay?)
> 
> Kind regards,
> Nicolas Frattaroli
> 
> > 
> > Thanks,
> > Leo
> > 
> > > 
> > > 
> > > -- Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer https://redhat.com \ Libre software enthusiast
> > 
> > 
> 
>