Re: [RFC 4/8] drm/i915/cmtg: program vrr registers of cmtg

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 17 Nov 2025, Animesh Manna <animesh.manna@intel.com> 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.

BR,
Jani.

>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  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