| Message ID | 20240902080635.2946858-13-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
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@intel.com> > --- > 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. > > - 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? > + 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. 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? > } 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
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@intel.com> >> --- >> 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
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; - 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; + 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; } 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) {
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@intel.com> --- drivers/gpu/drm/i915/display/intel_vrr.c | 61 +++++++++++++++--------- 1 file changed, 39 insertions(+), 22 deletions(-)