Re: [PATCH v1 3/8] drm/i915/display: Introduce CMRR fraction level to vrr crtc state

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 5/19/2026 10:33 AM, Mitul Golani wrote:
> CMRR (Content Match Refresh Rate) requires fractional multipliers
> applied to vtotal to precisely match the target refresh rate. Introduce
> enum cmrr_level to encode three distinct fraction cases:
> 
>    CMRR_DEFAULT: No fractional adjustment; use the fixed refresh rate
>                  timings as-is.
> 
>    CMRR_HIGH:    Apply a 1001/1000 multiplier to vtotal, targeting a
>                  slightly higher effective refresh rate (e.g. 60.06 Hz
>                  for a 60 Hz mode). Used for video content playback.
> 

Looking at BSpec-68925, the ratio 1001/1000 is never mentioned.
My understanding is that CMRR always dithers *down* the actual refresh 
rate (defined by the edid) to a a desired refresh rate of the content.

Looking at the use-case of CVT RB3 4K, the refresh rate calculated from
the edid timings would be 60.021Hz which can be brought to either a 
integer rate of 60Hz(Desktop use-cases) or video rate of 60/1.001 Hz.

The spec also mentions CVT RB2, which apparently has a rounded down 
pixel clock that results in a refresh rate of 59.99Hz, I guess in that 
case we can only accurately support video modes (60/1.001) using CMRR
and not exact 60Hz.

Few other things to support this claim.

1. The algorithm in the BSpec only defines one set of fraction (for 
video modes)

         refresh_rate_muliplier = 1000;
         refresh_rate_divider = 1001;

2. There is only one bit (BIT[5]) to represent "Target Refresh Rate 
Divider" in DB4 of Adaptive Sync SDP which is documented as

	0 = 1.000 (Nominal RR)
	1 = 1.001 (Nominal RR / 1.001)

Also, I could not really find a standard that defines the multiplier of 
1001/1000.

Any reason/use-case, you found to dither up the RR?

Coming to the API interface the series choses, It does not really work 
for the CVT RB3 timings that was mentioned above. Since the idea is to 
dither down 60.021Hz to 60Hz or 60/1.001 Hz, the assumption that you 
will always apply a multiplier of 1000/1001 on the refresh rate derived 
from edid to bring down the refresh rate, does not fit.

Therefore, I think even for the debugfs implementation we should try to 
incorporate the uAPI design (or a version of it) proposed during the 
Display Hackfest.[1]

==
Chaitanya

[1] 
https://gitlab.freedesktop.org/-/project/2891/uploads/9db8886701e2598271a8c4c6dc4dc2b1/display_next_hackfest_2026.pdf

>    CMRR_LOW:     Apply a 1000/1001 multiplier to vtotal, targeting a
>                  slightly lower effective refresh rate. Used when the
>                  pixel clock needs to be pulled down to match content.
> 
> Add the level field to the vrr.cmrr crtc state and state dump so it
> can be tracked.
> 
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display_types.h |  7 +++++++
>   drivers/gpu/drm/i915/display/intel_vrr.c           | 12 ++++++++++++
>   drivers/gpu/drm/i915/display/intel_vrr.h           |  2 ++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index ce280349622b..1d5aee13afb1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -999,6 +999,12 @@ struct intel_casf {
>   	bool enable;
>   };
>   
> +enum cmrr_level {
> +	CMRR_DEFAULT,
> +	CMRR_LOW,
> +	CMRR_HIGH,
> +};
> +
>   struct intel_crtc_state {
>   	/*
>   	 * uapi (drm) state. This is the software state shown to userspace.
> @@ -1400,6 +1406,7 @@ struct intel_crtc_state {
>   		struct {
>   			bool enable;
>   			u64 cmrr_n, cmrr_m;
> +			enum cmrr_level level;
>   		} cmrr;
>   	} vrr;
>   
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 87d52b206bdb..8d79d289378b 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -1228,3 +1228,15 @@ int intel_vrr_dcb_vmax_vblank_start_final(const struct intel_crtc_state *crtc_st
>   
>   	return intel_vrr_vblank_start(crtc_state, VRR_DCB_VMAX(tmp) + 1);
>   }
> +
> +char *intel_vrr_cmrr_level_to_string(enum cmrr_level level)
> +{
> +	switch (level) {
> +	case CMRR_LOW:
> +		return "Low";
> +	case CMRR_HIGH:
> +		return "High";
> +	default:
> +		return "Default";
> +	}
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 4f16ca4af91f..86707b8af2e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -7,6 +7,7 @@
>   #define __INTEL_VRR_H__
>   
>   #include <linux/types.h>
> +#include "intel_display_types.h"
>   
>   struct drm_connector_state;
>   struct intel_atomic_state;
> @@ -53,5 +54,6 @@ int intel_vrr_dcb_vmin_vblank_start_next(const struct intel_crtc_state *crtc_sta
>   int intel_vrr_dcb_vmax_vblank_start_next(const struct intel_crtc_state *crtc_state);
>   int intel_vrr_dcb_vmin_vblank_start_final(const struct intel_crtc_state *crtc_state);
>   int intel_vrr_dcb_vmax_vblank_start_final(const struct intel_crtc_state *crtc_state);
> +char *intel_vrr_cmrr_level_to_string(enum cmrr_level level);
>   
>   #endif /* __INTEL_VRR_H__ */