Re: [PATCH 07/22] drm/i915/vrr: Prepare for fixed refresh rate timings

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

 




On 3/5/2025 12:19 AM, Ville Syrjälä wrote:
On Tue, Mar 04, 2025 at 01:49:33PM +0530, Ankit Nautiyal wrote:
Currently we always compute the timings as if vrr is enabled.
With this approach the state checker becomes complicated when we
introduce fixed refresh rate mode with vrr timing generator.

To avoid the complications, instead of always computing vrr timings, we
compute vrr timings based on uapi.vrr_enable knob.
So when the knob is disabled we always compute vmin=flipline=vmax.

v2: Use actual timings without any adjustments while preparing for
fixed timings in compute_config. (Ville)
v3: Avoid setting fixed timings if !vrr_possible().

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> (#v2)
---
  drivers/gpu/drm/i915/display/intel_vrr.c | 73 ++++++++++++++++++++++++
  1 file changed, 73 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index e0573e28014b..0e606dfe4a56 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -246,6 +246,68 @@ void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state)
  	crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
  }
+/*
+ * For fixed refresh rate mode Vmin, Vmax and Flipline all are set to
+ * Vtotal value.
+ */
+static
+int intel_vrr_fixed_rr_vtotal(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	int crtc_vtotal = crtc_state->hw.adjusted_mode.crtc_vtotal;
+
+	if (DISPLAY_VER(display) >= 13)
+		return crtc_vtotal;
+	else
+		return crtc_vtotal -
+			intel_vrr_real_vblank_delay(crtc_state);
+}
+
+static
+int intel_vrr_fixed_rr_vmax(const struct intel_crtc_state *crtc_state)
+{
+	return intel_vrr_fixed_rr_vtotal(crtc_state);
+}
+
+static
+int intel_vrr_fixed_rr_vmin(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+
+	return intel_vrr_fixed_rr_vtotal(crtc_state) -
+		intel_vrr_flipline_offset(display);
+}
+
+static
+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);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+	if (!intel_vrr_possible(crtc_state))
+		return;
+
+	intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_vmin(crtc_state) - 1);
+	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_vmax(crtc_state) - 1);
+	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
+		       intel_vrr_fixed_rr_flipline(crtc_state) - 1);
+}
+
+static
+void intel_vrr_prepare_fixed_timings(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->vrr.vmax = intel_vrr_vmin_flipline(crtc_state);
+	crtc_state->vrr.flipline = intel_vrr_vmin_flipline(crtc_state);
I think it would be cleaner to just use crtc_vtotal during
all this timing compute stuff, and move the magic vmin
adjustment to be done after it's all done.
Alright so IIUC for fixed timings case :
crtc_state->vrr.vmax = crtc_state->hw.adjusted_mode.crtc_vtotal;
crtc_state->vrr.flipline = crtc_state->hw.adjusted_mode.crtc_vtotal;

And will move  crtc_state->vrr.vmin -= intel_vrr_flipline_offset(display); after all compute timings is done.



+}
+
  static
  int intel_vrr_compute_vmin(struct intel_crtc_state *crtc_state)
  {
@@ -325,6 +387,8 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
  		intel_vrr_compute_vrr_timings(crtc_state);
  	else if (is_cmrr_frac_required(crtc_state) && is_edp)
  		intel_vrr_compute_cmrr_timings(crtc_state);
+	else
+		intel_vrr_prepare_fixed_timings(crtc_state);
The "compute" vs. "prepare" difference in terminology is a bit
confusing.
Can change this to `intel_vrr_compute_fixed_rr_timings()`


Regards,

Ankit


if (HAS_AS_SDP(display)) {
  		crtc_state->vrr.vsync_start =
@@ -496,6 +560,13 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
  	if (!crtc_state->vrr.enable)
  		return;
+ intel_de_write(display, TRANS_VRR_VMIN(display, cpu_transcoder),
+		       crtc_state->vrr.vmin - 1);
+	intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder),
+		       crtc_state->vrr.vmax - 1);
+	intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder),
+		       crtc_state->vrr.flipline - 1);
+
  	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder),
  		       TRANS_PUSH_EN);
@@ -523,6 +594,8 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
  				TRANS_VRR_STATUS(display, cpu_transcoder),
  				VRR_STATUS_VRR_EN_LIVE, 1000);
  	intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0);
+
+	intel_vrr_set_fixed_rr_timings(old_crtc_state);
  }
static
--
2.45.2



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

  Powered by Linux