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

On Thursday, 24 September 2026 09:01:05 Central European Summer Time Vidith Madhu wrote:

On Wed, 23 Sep 2026, Nicolas Frattaroli wrote:

quoted

On Wednesday, 23 September 2026 11:51:45 Central European Summer Time Michel Dänzer wrote:

quoted

On 9/21/26 17:51, Nicolas Frattaroli wrote:

quoted

+ * .. _VRR-MIN-NUMERATOR:
+ *
+ * "VRR_MIN_NUMERATOR":
+ *	Default &drm_crtc integer property forming the numerator of a
+ *	numerator/denominator pair of a frame rate to set as the minimum VRR
+ *	target rate. Set to 0 to disable.
+ *
+ * "VRR_MIN_DENOMINATOR":
+ *	Default &drm_crtc integer property forming the denominator of a
+ *	numerator/denominator pair of a frame rate to set as the minimum VRR
+ *	target rate. If :ref:`VRR_MIN_NUMERATOR <VRR-MIN-NUMERATOR>` is not
+ *	zero, it must be non-zero.
+ *	Otherwise, must also be zero.
+ *
+ * .. _VRR-MAX-NUMERATOR:
+ *
+ * "VRR_MAX_NUMERATOR":
+ *	Default &drm_crtc integer property forming the numerator of a
+ *	numerator/denominator pair of a frame rate to set as the maximum VRR
+ *	target rate. Set to 0 to disable.
+ *
+ * "VRR_MAX_DENOMINATOR":
+ *	Default &drm_crtc integer property forming the denominator of a
+ *	numerator/denominator pair of a frame rate to set as the maximum VRR
+ *	target rate. If :ref:`VRR_MAX_NUMERATOR <VRR-MAX-NUMERATOR>` is not
+ *	zero, it must be non-zero. Otherwise, must also be zero.
  */

Is there a reason that DENOMINATOR must be 0 when the corresponding NUMERATOR is? 0 divided by any number is still 0.

No, I think that's an arbitrary convention I settled on and don't enforce. I
guess it should be "Otherwise, may also be zero", because the only situation
I'm making userspace avoid is x/0 where x != 0.

quoted

quoted

Either way, should these rules be enforced in drm_atomic_crtc_set_property?

Or rather in atomic_check.

Due to complicating factors like EDID, CinemaVRR, and QMS TFRmin/TFRmax, checking
the properties for sensible values is done in the HDMI state helpers at the moment.

If/when there is a similar mechanism for DP, we can probably factor the common
parts out. I really do hope all drivers (including those that don't use the HDMI
state helpers) can at least share the hdmi_validate_vrr() logic, but I haven't
factored this out into an exported function yet because I don't know how similar
the DisplayPort-equivalent mechanisms requirements are, or how much of the state
derivation non-state-helper drivers (i.e. i915 and amdgpu) need.

Kind regards,
Nicolas Frattaroli

Thanks for bringing up support for VRR features on DP. Concerning the 
frame rate limits themselves, as I mentioned in patch [02/25] I don't 
think there's a reason to limit the validation to HDMI.

Agreed. I think factoring most of the stuff out into the CRTC atomic check
phase is what I'll do, and then keep the HDMI-specific validation and
derivation within the HDMI state helpers. That should somewhat simplify
the code as well I think.

There is in fact an analog of QMS for DP, called DP-FAVT mode - this is
quite a bit more streamlined than QMS and the validation should be
simpler. Would be nice to see these properties support that as well

Yep, the intent is that the properties are display protocol agnostic,
it just so happens that the only implementation I wrote so far is
specific to HDMI.

Also, good to hear the DP QMS-equivalent is more streamlined. :)

(this was briefly discussed at DisplayNext HackFest, if I understood
access to the DP spec is the main blocker here?).

It certainly is a blocker. We also don't have any fancy testing/validation
equipment, so validating implementations for protocols before they reach
consumer devices is difficult for us (Collabora) at this time. That's also
why it's a good idea for anyone who has access to the expensive specialised
protocol test equipment to validate that the HDMI VTEM EMP packets we're
constructing are actually within spec and not just a "happens to work" kind
of deal.

Kind regards
Nicolas Frattaroli