On Tue, Sep 22, 2026 at 02:53:36PM +0200, Nicolas Frattaroli wrote: > 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: > > > 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 <[email protected]> > > > --- > > > 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.
Alternatively, and since we only have a single driver enabling it (for now), I don't think having some driver specific code for now to handle all this is that bad. Helpers only really make sense when we start sharing code between drivers. Maxime
signature.asc
Description: PGP signature
