> -----Original Message-----
> From: Jani Nikula <[email protected]>
> Sent: Monday, November 17, 2025 8:40 PM
> To: Manna, Animesh <[email protected]>; intel-
> [email protected]; [email protected]
> Cc: Manna, Animesh <[email protected]>
> Subject: Re: [RFC 4/8] drm/i915/cmtg: program vrr registers of cmtg
>
> On Mon, 17 Nov 2025, Animesh Manna <[email protected]> wrote:
> > Enable vrr if it is enabled on cmtg registers.
>
> This violates the basic principle that hardware and software states are kept
> separate. When we write the software state to the hardware, making parts
> of it conditional on the existing hardware state results in non-deterministic
> behaviour.
Thanks for review.
Taken care by using s/w state in next version, currently debug ongoing with
flipQ, will float after that.
Regards,
Animesh
>
> BR,
> Jani.
>
> >
> > Signed-off-by: Animesh Manna <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_cmtg.c | 19 +++++++++++++++++++
> > .../gpu/drm/i915/display/intel_cmtg_regs.h | 5 +++++
> > 2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > index 5e9aaa50b38f..3dfb691913cb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
> > @@ -17,6 +17,7 @@
> > #include "intel_display_power.h"
> > #include "intel_display_regs.h"
> > #include "intel_display_types.h"
> > +#include "intel_vrr_regs.h"
> >
> > /**
> > * DOC: Common Primary Timing Generator (CMTG) @@ -213,6 +214,7
> @@
> > static void intel_cmtg_set_timings(const struct intel_crtc_state
> > *crtc_state) {
> > struct intel_display *display = to_intel_display(crtc_state);
> > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > + u32 vctl;
> >
> > intel_de_write(display, TRANS_HTOTAL_CMTG(cpu_transcoder),
> > intel_de_read(display, TRANS_HTOTAL(display,
> > cpu_transcoder))); @@ -226,6 +228,23 @@ static void
> intel_cmtg_set_timings(const struct intel_crtc_state *crtc_state)
> > intel_de_read(display, TRANS_VBLANK(display,
> cpu_transcoder)));
> > intel_de_write(display, TRANS_VSYNC_CMTG(cpu_transcoder),
> > intel_de_read(display, TRANS_VSYNC(display,
> > cpu_transcoder)));
> > +
> > + vctl = intel_de_read(display, TRANS_VRR_CTL(display,
> cpu_transcoder));
> > + if (vctl & VRR_CTL_VRR_ENABLE) {
> > + u32 vmax, flipline, vmin;
> > +
> > + vmax = intel_de_read(display, TRANS_VRR_VMAX(display,
> cpu_transcoder));
> > + flipline = intel_de_read(display,
> TRANS_VRR_FLIPLINE(display, cpu_transcoder));
> > + if (vmax != flipline)
> > + return;
> > +
> > + vmin = intel_de_read(display, TRANS_VRR_VMIN(display,
> > +cpu_transcoder));
> > +
> > + intel_de_write(display,
> TRANS_VRR_VMAX_CMTG(cpu_transcoder), vmax);
> > + intel_de_write(display,
> TRANS_VRR_VMIN_CMTG(cpu_transcoder), vmin);
> > + intel_de_write(display,
> TRANS_VRR_FLIPLINE_CMTG(cpu_transcoder), flipline);
> > + intel_de_write(display,
> TRANS_VRR_CTL_CMTG(cpu_transcoder), vctl);
> > + }
> > }
> >
> > void intel_cmtg_enable(const struct intel_crtc_state *crtc_state)
> > diff --git a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > index 47403bbcac7d..37dee7165852 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cmtg_regs.h
> > @@ -33,4 +33,9 @@ enum cmtg {
> > #define TRANS_VBLANK_CMTG(id) _MMIO(0x6F010 + (id) *
> 0x100)
> > #define TRANS_VSYNC_CMTG(id) _MMIO(0x6F014 + (id) *
> 0x100)
> >
> > +#define TRANS_VRR_CTL_CMTG(id) _MMIO(0x6F420 + (id) *
> 0x100)
> > +#define TRANS_VRR_VMAX_CMTG(id) _MMIO(0x6F424 +
> (id) * 0x100)
> > +#define TRANS_VRR_VMIN_CMTG(id) _MMIO(0x6F434 +
> (id) * 0x100)
> > +#define TRANS_VRR_FLIPLINE_CMTG(id) _MMIO(0x6F438 + (id) *
> 0x100)
> > +
> > #endif /* __INTEL_CMTG_REGS_H__ */
>
> --
> Jani Nikula, Intel
