diff mbox series

[18/20] drm/i915/vrr: Use fixed timings for platforms that support VRR

Message ID 20250224061717.1095226-19-ankit.k.nautiyal@intel.com (mailing list archive)
State New
Headers show
Series Use VRR timing generator for fixed refresh rate modes | expand

Commit Message

Nautiyal, Ankit K Feb. 24, 2025, 6:17 a.m. UTC 
For fixed refresh rate use fixed timings for all platforms that support
VRR. For this add checks to avoid computing and reading VRR for
platforms that do not support VRR.
For platforms that do support VRR, readback vrr timings whether or not
VRR_CTL_FLIP_LINE_EN is set in VRR_CTL or not.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 43 ++++++++++++------------
 1 file changed, 22 insertions(+), 21 deletions(-)

Comments

Ville Syrjälä Feb. 26, 2025, 3:11 p.m. UTC | #1 
On Mon, Feb 24, 2025 at 11:47:15AM +0530, Ankit Nautiyal wrote:
> For fixed refresh rate use fixed timings for all platforms that support
> VRR. For this add checks to avoid computing and reading VRR for
> platforms that do not support VRR.
> For platforms that do support VRR, readback vrr timings whether or not
> VRR_CTL_FLIP_LINE_EN is set in VRR_CTL or not.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 43 ++++++++++++------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 551dcc16f0d4..975fed9930c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -344,6 +344,9 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	int vmin = 0, vmax = 0;
>  
> +	if (!HAS_VRR(display))
> +		return;
> +
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		return;
>  
> @@ -358,9 +361,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  
>  	vmin = intel_vrr_compute_vmin(crtc_state);
>  
> -	if (vmin >= vmax)
> -		return;
> -
>  	crtc_state->vrr.vmin = vmin;
>  	crtc_state->vrr.vmax = vmax;

I think your earlier pathc left vmax==0 here for the !in_range so
this looks a bit wrong. But if you change the earlier patch like I
suggested to set vmax=vmin then this would be fine.

>  
> @@ -373,7 +373,7 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  	 */
>  	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
>  
> -	if (crtc_state->uapi.vrr_enabled)
> +	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
>  		intel_vrr_compute_vrr_timings(crtc_state);
>  	else if (is_cmrr_frac_required(crtc_state) && is_edp)
>  		intel_vrr_compute_cmrr_timings(crtc_state);
> @@ -640,6 +640,9 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 trans_vrr_ctl, trans_vrr_vsync;
>  
> +	if (!HAS_VRR(display))
> +		return;

I think the caller is already checking that. But I suppose we could
move the checks into the VRR code.

> +
>  	trans_vrr_ctl = intel_de_read(display,
>  				      TRANS_VRR_CTL(display, cpu_transcoder));
>  
> @@ -663,23 +666,21 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
>  			crtc_state->vrr.pipeline_full =
>  				REG_FIELD_GET(VRR_CTL_PIPELINE_FULL_MASK, trans_vrr_ctl);
>  
> -	if (trans_vrr_ctl & VRR_CTL_FLIP_LINE_EN) {
> -		crtc_state->vrr.flipline = intel_de_read(display,
> -							 TRANS_VRR_FLIPLINE(display, cpu_transcoder)) + 1;
> -		crtc_state->vrr.vmax = intel_de_read(display,
> -						     TRANS_VRR_VMAX(display, cpu_transcoder)) + 1;
> -		crtc_state->vrr.vmin = intel_de_read(display,
> -						     TRANS_VRR_VMIN(display, cpu_transcoder)) + 1;
> -
> -		if (HAS_AS_SDP(display)) {
> -			trans_vrr_vsync =
> -				intel_de_read(display,
> -					      TRANS_VRR_VSYNC(display, cpu_transcoder));
> -			crtc_state->vrr.vsync_start =
> -				REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
> -			crtc_state->vrr.vsync_end =
> -				REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
> -		}

I think you want to keep the VRR_CTL_FLIP_LINE_EN check around the
TRANS_VRR_FLIPLINE read at least, because we want the state checker
to catch any misprogrammng of VRR_CTL_FLIP_LINE_EN.

> +	crtc_state->vrr.flipline = intel_de_read(display,
> +						 TRANS_VRR_FLIPLINE(display, cpu_transcoder)) + 1;
> +	crtc_state->vrr.vmax = intel_de_read(display,
> +					     TRANS_VRR_VMAX(display, cpu_transcoder)) + 1;
> +	crtc_state->vrr.vmin = intel_de_read(display,
> +					     TRANS_VRR_VMIN(display, cpu_transcoder)) + 1;
> +
> +	if (HAS_AS_SDP(display)) {
> +		trans_vrr_vsync =
> +			intel_de_read(display,
> +				      TRANS_VRR_VSYNC(display, cpu_transcoder));
> +		crtc_state->vrr.vsync_start =
> +			REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
> +		crtc_state->vrr.vsync_end =
> +			REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
>  	}
>  
>  	crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&
> -- 
> 2.45.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 551dcc16f0d4..975fed9930c1 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -344,6 +344,9 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	int vmin = 0, vmax = 0;
 
+	if (!HAS_VRR(display))
+		return;
+
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
 		return;
 
@@ -358,9 +361,6 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 
 	vmin = intel_vrr_compute_vmin(crtc_state);
 
-	if (vmin >= vmax)
-		return;
-
 	crtc_state->vrr.vmin = vmin;
 	crtc_state->vrr.vmax = vmax;
 
@@ -373,7 +373,7 @@  intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
 	 */
 	crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display);
 
-	if (crtc_state->uapi.vrr_enabled)
+	if (crtc_state->uapi.vrr_enabled && vmin < vmax)
 		intel_vrr_compute_vrr_timings(crtc_state);
 	else if (is_cmrr_frac_required(crtc_state) && is_edp)
 		intel_vrr_compute_cmrr_timings(crtc_state);
@@ -640,6 +640,9 @@  void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	u32 trans_vrr_ctl, trans_vrr_vsync;
 
+	if (!HAS_VRR(display))
+		return;
+
 	trans_vrr_ctl = intel_de_read(display,
 				      TRANS_VRR_CTL(display, cpu_transcoder));
 
@@ -663,23 +666,21 @@  void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
 			crtc_state->vrr.pipeline_full =
 				REG_FIELD_GET(VRR_CTL_PIPELINE_FULL_MASK, trans_vrr_ctl);
 
-	if (trans_vrr_ctl & VRR_CTL_FLIP_LINE_EN) {
-		crtc_state->vrr.flipline = intel_de_read(display,
-							 TRANS_VRR_FLIPLINE(display, cpu_transcoder)) + 1;
-		crtc_state->vrr.vmax = intel_de_read(display,
-						     TRANS_VRR_VMAX(display, cpu_transcoder)) + 1;
-		crtc_state->vrr.vmin = intel_de_read(display,
-						     TRANS_VRR_VMIN(display, cpu_transcoder)) + 1;
-
-		if (HAS_AS_SDP(display)) {
-			trans_vrr_vsync =
-				intel_de_read(display,
-					      TRANS_VRR_VSYNC(display, cpu_transcoder));
-			crtc_state->vrr.vsync_start =
-				REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
-			crtc_state->vrr.vsync_end =
-				REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
-		}
+	crtc_state->vrr.flipline = intel_de_read(display,
+						 TRANS_VRR_FLIPLINE(display, cpu_transcoder)) + 1;
+	crtc_state->vrr.vmax = intel_de_read(display,
+					     TRANS_VRR_VMAX(display, cpu_transcoder)) + 1;
+	crtc_state->vrr.vmin = intel_de_read(display,
+					     TRANS_VRR_VMIN(display, cpu_transcoder)) + 1;
+
+	if (HAS_AS_SDP(display)) {
+		trans_vrr_vsync =
+			intel_de_read(display,
+				      TRANS_VRR_VSYNC(display, cpu_transcoder));
+		crtc_state->vrr.vsync_start =
+			REG_FIELD_GET(VRR_VSYNC_START_MASK, trans_vrr_vsync);
+		crtc_state->vrr.vsync_end =
+			REG_FIELD_GET(VRR_VSYNC_END_MASK, trans_vrr_vsync);
 	}
 
 	crtc_state->vrr.enable = trans_vrr_ctl & VRR_CTL_VRR_ENABLE &&