Re: [PATCH 16/20] drm/i915/display: Use fixed rr timings in intel_set_transcoder_timings_lrr()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2/26/2025 8:30 PM, Ville Syrjälä wrote:
On Mon, Feb 24, 2025 at 11:47:13AM +0530, Ankit Nautiyal wrote:
To have Guardband/Pipeline_full reconfigured seamlessly, move the
guardband and pipeline_full checks out from the pure !fastset block in
intel_pipe_config_compare().
Update the intel_set_transcoder_timings_lrr() function to use
fixed refresh rate timings for platforms which always use
VRR timing generator.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/display/intel_display.c | 43 ++++++++++++++++----
  drivers/gpu/drm/i915/display/intel_vrr.c     | 13 +++++-
  drivers/gpu/drm/i915/display/intel_vrr.h     |  2 +
  3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 752ed44d8892..d3c12bc3645c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2854,6 +2854,9 @@ static void intel_set_transcoder_timings_lrr(const struct intel_crtc_state *crtc
  	intel_de_write(dev_priv, TRANS_VTOTAL(dev_priv, cpu_transcoder),
  		       VACTIVE(crtc_vdisplay - 1) |
  		       VTOTAL(crtc_vtotal - 1));
+
+	intel_vrr_set_fixed_rr_timings(crtc_state);
+	intel_vrr_update_guardband(crtc_state, false);
  }
static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state)
@@ -5127,17 +5130,36 @@ pipe_config_cx0pll_mismatch(struct drm_printer *p, bool fastset,
  	intel_cx0pll_dump_hw_state(display, b);
  }
-static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_state)
+static
+bool allow_vrr_guardband_fastset(const struct intel_crtc_state *old_crtc_state,
+				 const struct intel_crtc_state *new_crtc_state)
+{
+	struct intel_display *display = to_intel_display(old_crtc_state);
+
+	if (!HAS_VRR(display))
+		return false;
+
+	if (DISPLAY_VER(display) >= 13)
+		return new_crtc_state->vrr.guardband != old_crtc_state->vrr.guardband;
+
+	return new_crtc_state->vrr.pipeline_full != old_crtc_state->vrr.pipeline_full;
+}
None of this stuff should be necessary. allow_vblank_delay_fastset()
only says whether we're allowed the fastset or not. It isn't supposed
to check if it's necessary or not.

+
+static bool allow_vblank_delay_fastset(const struct intel_crtc_state *old_crtc_state,
+				       const struct intel_crtc_state *new_crtc_state)
  {
  	struct intel_display *display = to_intel_display(old_crtc_state);
/*
-	 * Allow fastboot to fix up vblank delay (handled via LRR
-	 * codepaths), a bit dodgy as the registers aren't
+	 * Allow fastboot to fix up vblank delay, vrr guardband/pipeline_full
+	 * (handled via LRR codepaths), a bit dodgy as the registers aren't
  	 * double buffered but seems to be working more or less...
  	 */
-	return HAS_LRR(display) && old_crtc_state->inherited &&
-		!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI);
+	if (!HAS_LRR(display) || intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DSI))
+		return false;
+
+	return old_crtc_state->inherited ||
+	       allow_vrr_guardband_fastset(old_crtc_state, new_crtc_state);
  }
bool
@@ -5273,7 +5295,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
  	PIPE_CONF_CHECK_I(name.crtc_hsync_start); \
  	PIPE_CONF_CHECK_I(name.crtc_hsync_end); \
  	PIPE_CONF_CHECK_I(name.crtc_vdisplay); \
-	if (!fastset || !allow_vblank_delay_fastset(current_config)) \
+	if (!fastset || !allow_vblank_delay_fastset(current_config, pipe_config)) \
  		PIPE_CONF_CHECK_I(name.crtc_vblank_start); \
  	PIPE_CONF_CHECK_I(name.crtc_vsync_start); \
  	PIPE_CONF_CHECK_I(name.crtc_vsync_end); \
@@ -5569,8 +5591,6 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
  		PIPE_CONF_CHECK_I(vrr.vmin);
  		PIPE_CONF_CHECK_I(vrr.vmax);
  		PIPE_CONF_CHECK_I(vrr.flipline);
-		PIPE_CONF_CHECK_I(vrr.pipeline_full);
-		PIPE_CONF_CHECK_I(vrr.guardband);
  		PIPE_CONF_CHECK_I(vrr.vsync_start);
  		PIPE_CONF_CHECK_I(vrr.vsync_end);
  		PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
@@ -5578,6 +5598,11 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
  		PIPE_CONF_CHECK_BOOL(cmrr.enable);
  	}
+ if (!fastset || !allow_vblank_delay_fastset(current_config, pipe_config)) {
+		PIPE_CONF_CHECK_I(vrr.guardband);
+		PIPE_CONF_CHECK_I(vrr.pipeline_full);
+	}
I'd like to see one patch that just moves these completely out
of the !fastset block, and a second patch that adds the fastboot
exception. That way if we do see any problems from the live
guardband reprogramming we can just revert it.
I have one doubt.

If we remove guardband/pipeline_full from the !fastset block, the VRR-seamless-switch case will start failing.

This is because the test tries a virtual LRR mode, resulting in a change in the VRR guardband.

Since its not in fastset block, we will need a full modeset, but the test expects a seamless switch.

Regards,

Ankit



Hmm, except you haven't changed the vrr_compute_config() yet at
this point in the series, so changing this would break both fastset
and state checker, I think. So we might have to move this patch to be
later in the series.

+
  #undef PIPE_CONF_CHECK_X
  #undef PIPE_CONF_CHECK_I
  #undef PIPE_CONF_CHECK_LLI
@@ -5875,7 +5900,7 @@ static void intel_crtc_check_fastset(const struct intel_crtc_state *old_crtc_sta
  		drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] fastset requirement not met, forcing full modeset\n",
  			    crtc->base.base.id, crtc->base.name);
  	} else {
-		if (allow_vblank_delay_fastset(old_crtc_state))
+		if (allow_vblank_delay_fastset(old_crtc_state, new_crtc_state))
  			new_crtc_state->update_lrr = true;
  		new_crtc_state->uapi.mode_changed = false;
  	}
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 11b7edbdb116..e77f5b483b09 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -284,7 +284,6 @@ int intel_vrr_fixed_rr_flipline(const struct intel_crtc_state *crtc_state)
  	return intel_vrr_fixed_rr_vtotal(crtc_state);
  }
-static
  void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
  {
  	struct intel_display *display = to_intel_display(crtc_state);
@@ -677,3 +676,15 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
  	if (crtc_state->vrr.enable)
  		crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
  }
+
+void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, bool needs_modeset)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	u32 vrr_ctl = trans_vrr_ctl(crtc_state);
+
+	if (intel_vrr_always_use_vrr_tg(display) && !needs_modeset)
+		vrr_ctl |= VRR_CTL_VRR_ENABLE;
+
+	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), vrr_ctl);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
index 514822577e8a..e4e9cadcdf9e 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr.h
@@ -35,5 +35,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state);
  int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state);
  int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state);
  int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state);
+void intel_vrr_update_guardband(const struct intel_crtc_state *crtc_state, bool needs_modeset);
+void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state);
#endif /* __INTEL_VRR_H__ */
--
2.45.2



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux