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

[Leo Li <sunpeng.li@amd.com>]

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:
> 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. 

- Leo

>  		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,
>