On Fri, Jan 24, 2025 at 08:29:56PM +0530, Ankit Nautiyal wrote:
> CMRR has a separate logic for computing vrr timings and so it
> overwrites the timings prepared for vrr.
>
> Avoid prepare vrr timings for cmrr. This will help to separate the
> helpers for timings for vrr, cmrr and the forthcoming
> fixed_rr.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_vrr.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 7e69e30b2076..90fd6fe58fce 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -257,8 +257,9 @@ void intel_vrr_compute_cmrr_timings(struct intel_crtc_state *crtc_state)
> }
>
> static
> -void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state)
> +void intel_vrr_compute_vrr_timings(struct intel_crtc_state *crtc_state, int vmin, int vmax)
> {
> + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
> crtc_state->vrr.enable = true;
> crtc_state->mode_flags |= I915_MODE_FLAG_VRR;
> }
> @@ -328,12 +329,12 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> if (vmin >= vmax)
> return;
>
> - intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
> -
> if (crtc_state->uapi.vrr_enabled)
> - intel_vrr_compute_vrr_timings(crtc_state);
> + intel_vrr_compute_vrr_timings(crtc_state, vmin, vmax);
> else if (is_cmrr_frac_required(crtc_state) && is_edp)
> intel_vrr_compute_cmrr_timings(crtc_state);
> + else
> + intel_vrr_prepare_vrr_timings(crtc_state, vmin, vmax);
I don't understand why the caller is calculating the vrr vmin/vmax
and then passing them in to everyone. Looks to me like each of those
guys should just calculate the vmin/vmax on their own.
The
crtc_state->vrr.flipline = crtc_state->vrr.vmin;
crtc_state->vrr.vmin -= intel_vrr_min_flipline_offset(display);
part could stay in the caller since it's always needed regardless
of what kind of timings we use.
>
> if (HAS_AS_SDP(display)) {
> crtc_state->vrr.vsync_start =
> --
> 2.45.2
--
Ville Syrjälä
Intel
