Re: [PATCH RFC 03/25] drm/atomic-helper: Set mode_changed on vrr_enabled change

On Monday, 21 September 2026 23:59:11 Central European Summer Time Leo Li wrote:

Hi Nicholas,

Thanks for sending this out. I'm looking into an amdgpu implementation
and had some comments along the way. Will reply inline on respective
patches.

On 2026-09-21 11:51, Nicolas Frattaroli wrote:

quoted

A change in whether VRR is enabled or not on a CRTC will be of interest
to any VRR implementation, as they may need to recalculate certain
internal state in response. So far, this was handled ad-hoc in the
drivers that implement VRR.

Handle this in the common DRM atomic core by setting the mode_changed
flag when this occurs. The rationale for using mode_changed is that VRR
is deeply intertwined with the current mode with regards to its timing.
Consequently, any component that cares about a mode change is likely
also going to care about VRR being enabled/disabled.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9d006f98413a..0664d5779282 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c

@@ -694,6 +694,12 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			new_crtc_state->active_changed = true;
 		}
 
+		if (old_crtc_state->vrr_enabled != new_crtc_state->vrr_enabled) {
+			drm_dbg_atomic(dev, "[CRTC:%d:%s] VRR changed\n",
+				       crtc->base.id, crtc->name);
+			new_crtc_state->mode_changed = true;
+		}
+

I don't think this is the case for all hardware, at least not for amdgpu.
The parts in HW that adjust vtotal can be programmed without requiring
bandwidth recalculations or reprogramming of pipes.

Thanks for pointing this out. I was already unsure of this patch before,
and I don't think Rockchip requires these recalculations either. Originally
I added this when I looked at the i915 code iirc and saw them setting it.

I think in the commit message I also conflate needing to know about VRR
being changed with needing to do a full modeset (which is what mode_changed
basically means).

I'll drop this patch on the next revision.

Kind regards,
Nicolas Frattaroli

- Leo

quoted

 		if (new_crtc_state->enable != has_connectors) {
 			drm_dbg_atomic(dev, "[CRTC:%d:%s] enabled/connectors mismatch (%d/%d)\n",
 				       crtc->base.id, crtc->name,