[PATCH 12/13] drm/i915/vrr: Always use VRR timing generator for XELPD+
Nautiyal, Ankit K
ankit.k.nautiyal at intel.com
Wed Sep 4 13:08:14 UTC 2024
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 at 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
More information about the Intel-gfx
mailing list