Re: [PATCH v7 07/10] drm/i915/vrr: Latch CMRR ratio via fastset on debugfs write

On 7/28/2026 8:29 PM, Mitul Golani wrote:
> Writing the per-CRTC "intel_vrr_target_refresh_rate" debugfs file only
> updates crtc->force_cmrr, a side channel that is not tracked by the
> atomic state. The requested ratio therefore does not reach the hardware
> until some unrelated commit recomputes the pipe config.
> 
> Kick an internal atomic commit for the CRTC from the debugfs write and
> mark the CRTC mode as changed so the pipe config is recomputed.
> 
> Signed-off-by: Mitul Golani <[email protected]>
> ---
>   drivers/gpu/drm/i915/display/intel_vrr.c | 63 ++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 7746b2ead7d1..056d513ab637 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -8,10 +8,12 @@
>   #include <linux/seq_file.h>
>   #include <linux/string.h>
>   
> +#include <drm/drm_atomic.h>
>   #include <drm/drm_print.h>
>   #include <drm/intel/step.h>
>   
>   #include "intel_alpm.h"
> +#include "intel_atomic.h"
>   #include "intel_cmtg.h"
>   #include "intel_crtc.h"
>   #include "intel_de.h"
> @@ -1312,6 +1314,58 @@ static int intel_vrr_debugfs_target_rr_open(struct inode *inode, struct file *fi
>   	return single_open(file, intel_vrr_debugfs_target_rr_show, inode->i_private);
>   }
>   
> +/*
> + * Force an internal fastset commit on @crtc so that a CMRR ratio programmed
> + * via debugfs gets recomputed and latched into hardware. CMRR only alters the
> + * (average) vtotal, which is handled by the LRR fastset path, so this does not
> + * trigger a full modeset (no blank out).
> + */
> +static int intel_vrr_cmrr_fastset_force(struct intel_crtc *crtc)

We need to be careful with the wording here. This commit may or may not 
be downgraded to a fastset, depending on whether the computed vblank 
value (with LRR disabled) changes. We need to figure out if we can also 
seamlessly change Vtotal in case of CMRR but until then lets not 
characterise it as a fastset commit. Let's just call it a commit.

> +{
> +	struct intel_display *display = to_intel_display(crtc);
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_commit *state;
> +	struct intel_crtc_state *crtc_state;
> +	int ret = 0;
> +
> +	state = drm_atomic_commit_alloc(display->drm);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
> +
> +	state->acquire_ctx = &ctx;
> +	to_intel_atomic_state(state)->internal = true;
> +
> +retry:
> +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state)) {
> +		ret = PTR_ERR(crtc_state);
> +		goto out;
> +	}
> +
> +	if (!crtc_state->hw.active)
> +		goto out;
> +
> +	/* Mark mode as changed to trigger a pipe recompute + update() */
> +	crtc_state->uapi.mode_changed = true;
> +
> +	ret = drm_atomic_commit(state);
> +out:
> +	if (ret == -EDEADLK) {
> +		drm_atomic_commit_clear(state);
> +		ret = drm_modeset_backoff(&ctx);
> +		if (!ret)
> +			goto retry;
> +	}
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	drm_atomic_commit_put(state);
> +
> +	return ret;
> +}
> +
>   static ssize_t intel_vrr_debugfs_target_rr_write(struct file *file, const char __user *ubuf,
>   						 size_t len, loff_t *offp)
>   {
> @@ -1340,6 +1394,15 @@ static ssize_t intel_vrr_debugfs_target_rr_write(struct file *file, const char _
>   	crtc->force_cmrr.numerator = numerator;
>   	crtc->force_cmrr.denominator = denominator;
>   
> +	/*
> +	 * The debugfs value is a side channel that is not tracked by the atomic
> +	 * state, so kick an internal fastset commit to recompute and latch the
> +	 * new CMRR parameters without a full modeset.
> +	 */
> +	ret = intel_vrr_cmrr_fastset_force(crtc);
> +	if (ret)
> +		return ret;
> +

restore the values of num/den to last successful value on failure. 
Otherwise, all subsequent commits fail as force_cmrr holds on to bad values.

>   	return len;
>   }
>