On 9/3/2024 6:55 PM, Ville Syrjälä wrote:
On Mon, Sep 02, 2024 at 01:36:33PM +0530, Ankit Nautiyal wrote:
Currently VRR timing generator is used only when VRR is enabled by
userspace. From XELPD+, gradually move away from older timing
generator and use VRR timing generator for fixed refresh rate also.
In such a case, Flipline VMin and VMax all are set to the Vtotal of the
mode, which effectively makes the VRR timing generator work in
fixed refresh rate mode.
v2: Use VRR Timing Generator from XELPD+ instead of MTL as it needs
Wa_14015406119.
v3: Set vrr.fixed during vrr_get_config (Mitul)
v4: Avoid setting vrr.fixed when vrr.cmrr is enabled.
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_vrr.c | 61 +++++++++++++++---------
1 file changed, 39 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index e01d4b4b8fa7..625728aff5a2 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -172,41 +172,54 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
return;
- crtc_state->vrr.in_range =
- intel_vrr_is_in_range(connector, drm_mode_vrefresh(adjusted_mode));
- if (!crtc_state->vrr.in_range)
- return;
-
if (HAS_LRR(display))
crtc_state->update_lrr = true;
We aren't supposed to do a LRR update unless the refresh rates are
within the VRR range. So this needs to be moved as well.
Noted. Will move this in the other block.
- vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
- adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
- vmax = adjusted_mode->crtc_clock * 1000 /
- (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
+ if (!crtc_state->uapi.vrr_enabled && DISPLAY_VER(display) >= 20) {
+ /*
+ * for XELPD+ always go for VRR timing generator even for
+ * fixed refresh rate.
+ */
+ crtc_state->vrr.vmin = adjusted_mode->crtc_vtotal;
+ crtc_state->vrr.vmax = adjusted_mode->crtc_vtotal;
+ crtc_state->vrr.flipline = adjusted_mode->crtc_vtotal;
Has the "flipline can't be below vmin+1" issue been changed in the hardware?
Need to check this, will get back on this.
+ crtc_state->vrr.fixed_rr = true;
+ } else {
+ crtc_state->vrr.in_range =
+ intel_vrr_is_in_range(connector, drm_mode_vrefresh(adjusted_mode));
- vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
- vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
+ if (!crtc_state->vrr.in_range)
+ return;
- if (vmin >= vmax)
- return;
+ vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
+ adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
+ vmax = adjusted_mode->crtc_clock * 1000 /
+ (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
- /*
- * flipline determines the min vblank length the hardware will
- * generate, and flipline>=vmin+1, hence we reduce vmin by one
- * to make sure we can get the actual min vblank length.
- */
- crtc_state->vrr.vmin = vmin - 1;
- crtc_state->vrr.vmax = vmax;
+ vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
+ vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
+
+ if (vmin >= vmax)
+ return;
+
+ /*
+ * flipline determines the min vblank length the hardware will
+ * generate, and flipline>=vmin+1, hence we reduce vmin by one
+ * to make sure we can get the actual min vblank length.
+ */
+ crtc_state->vrr.vmin = vmin - 1;
+ crtc_state->vrr.vmax = vmax;
- crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
+ crtc_state->vrr.flipline = crtc_state->vrr.vmin + 1;
+ crtc_state->vrr.fixed_rr = false;
+ }
/*
* When panel is VRR capable and userspace has
* not enabled adaptive sync mode then Fixed Average
* Vtotal mode should be enabled.
*/
- if (crtc_state->uapi.vrr_enabled) {
+ if (crtc_state->uapi.vrr_enabled || crtc_state->vrr.fixed_rr) {
crtc_state->vrr.enable = true;
crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
Hmm. This is now a bit of a mess. We need to come up with a sane way to
deal with the vblank timestamping code. Dunno if we want to make it so
that we'd always use VRR timings or the non-VRR timings. Should be
identical from HW POV so technically might not matter, apart from the
software state tracking POV. And from that angle it seems to me that
for the dynamic fixed vs. variable swithcing we might want to keep the
code using the non-VRR timings for fixed refresh rate.
So do we set I915_MODE_FLAG_VRR only when actual vrr is used, (ie. when
the vrr property is set)?
There seems to other stuff amiss still:
- We don't enable/disable the VRR timings generator early/late
in the modeset sequence?
- How do we atomically switch between the fixed vs. variable
timings since we can't temporarily disable the VRR timing generator?
Yeah, with current changes vrr.enable is always true.
Perhaps I should have added a check for disabling(fixed_rr, ..) in
intel_crtc_vrr_disabling.
Will get back on this as well.
MST + VRR is also one of the thing yet to be handled.
Thank you Ville, for the comments and guidance.
Regards,
Ankit
} else if (is_cmrr_frac_required(crtc_state) && is_edp) {
@@ -421,6 +434,10 @@ void intel_vrr_get_config(struct intel_crtc_state *crtc_state)
TRANS_VRR_VMAX(display, cpu_transcoder)) + 1;
crtc_state->vrr.vmin = intel_de_read(display,
TRANS_VRR_VMIN(display, cpu_transcoder)) + 1;
+ if (!crtc_state->cmrr.enable &&
+ crtc_state->vrr.vmax == crtc_state->vrr.flipline &&
+ crtc_state->vrr.vmin == crtc_state->vrr.flipline)
+ crtc_state->vrr.fixed_rr = true;
}
if (crtc_state->vrr.enable) {
--
2.45.2
