Re: [PATCH 1/4] drm/i915/display: Compute TRANS_VRR_VSYNC

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/12/2024 11:06 PM, Mitul Golani wrote:
> Compute TRANS_VRR_VSYNC which sets the position
> for hardware to send the Vsync at a fixed position
> relative to the end of the Vblank.
> ---
>   drivers/gpu/drm/i915/display/intel_display_types.h | 1 +
>   drivers/gpu/drm/i915/display/intel_vrr.c           | 9 +++++++++
>   drivers/gpu/drm/i915/i915_reg.h                    | 2 ++
>   3 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 01eb6e4e6049..c73a0037f6c8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1411,6 +1411,7 @@ struct intel_crtc_state {
>   		bool enable, in_range;
>   		u8 pipeline_full;
>   		u16 flipline, vmin, vmax, guardband;
> +		u32 vsync_end, vsync_start;
>   	} vrr;
>   
>   	/* Stream Splitter for eDP MSO */
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 5d905f932cb4..59628c360a78 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -149,6 +149,13 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>   
>   	crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
>   
> +	crtc_state->vrr.vsync_start =
> +			(crtc_state->hw.adjusted_mode.crtc_vtotal -
> +			VSYNC_START(crtc_state->hw.adjusted_mode.vsync_start));

The macros VSYNC_START/END dont seem to be correct here. This register 
uses 0-12 bits for vsync start and 16-28 for vsync_end.


> +	crtc_state->vrr.vsync_end =
> +			(crtc_state->hw.adjusted_mode.crtc_vtotal -
> +			(VSYNC_END(crtc_state->hw.adjusted_mode.vsync_end) >> 16));
> +
>   	/*
>   	 * For XE_LPD+, we use guardband and pipeline override
>   	 * is deprecated.
> @@ -201,6 +208,8 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
>   
>   	intel_de_write(dev_priv, TRANS_VRR_VMIN(cpu_transcoder), crtc_state->vrr.vmin - 1);
>   	intel_de_write(dev_priv, TRANS_VRR_VMAX(cpu_transcoder), crtc_state->vrr.vmax - 1);
> +	intel_de_write(dev_priv, TRANS_VRR_VSYNC(cpu_transcoder),
> +				   crtc_state->vrr.vsync_end << 16 | crtc_state->vrr.vsync_start);

I think we should write this only when we really send the Adaptive Sync 
SDP, when the source and sink both support the AS SDP.


Regards,

Ankit



>   	intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder), trans_vrr_ctl(crtc_state));
>   	intel_de_write(dev_priv, TRANS_VRR_FLIPLINE(cpu_transcoder), crtc_state->vrr.flipline - 1);
>   }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e00557e1a57f..3449e65fdf1b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2007,7 +2007,9 @@
>   #define _TRANS_VRR_CTL_B		0x61420
>   #define _TRANS_VRR_CTL_C		0x62420
>   #define _TRANS_VRR_CTL_D		0x63420
> +#define _TRANS_VRR_VSYNC_A		0x60078
>   #define TRANS_VRR_CTL(trans)			_MMIO_TRANS2(trans, _TRANS_VRR_CTL_A)
> +#define TRANS_VRR_VSYNC(trans)			_MMIO_TRANS2(trans, _TRANS_VRR_VSYNC_A)
>   #define   VRR_CTL_VRR_ENABLE			REG_BIT(31)
>   #define   VRR_CTL_IGN_MAX_SHIFT			REG_BIT(30)
>   #define   VRR_CTL_FLIP_LINE_EN			REG_BIT(29)