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

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:

quoted

On 2026-09-28 04:10, Michel Dänzer wrote:

quoted

quoted

quoted

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.

quoted

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.

quoted

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

quoted

Thanks,
Leo

quoted

-- Earthling Michel Dänzer \ GNOME / Xwayland / Mesa developer https://redhat.com \ Libre software enthusiast