On Tue, Mar 04, 2025 at 01:49:45PM +0530, Ankit Nautiyal wrote:
> VRR with joiner is currently disabled as it still needs some work to
> correctly sequence the primary and secondary transcoders. However, we can
> still use VRR Timing generator in fixed refresh rate for joiner and since
> it just need to program vrr timings once and does not involve changing
> timings on the fly. We still need to skip the VRR and LRR for joiner.
>
> To achieve this set vrr.in_range to 0 for joiner case, so that we do not
> try VRR and LRR for the joiner case.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_vrr.c | 38 +++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 0dfe6220ff4a..2b6d022434d2 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -292,6 +292,9 @@ void intel_vrr_set_fixed_rr_timings(const struct intel_crtc_state *crtc_state)
> if (!intel_vrr_possible(crtc_state))
> return;
>
> + if (intel_crtc_is_joiner_secondary(crtc_state))
> + return;
I don't think this gets called on the secondary crtc anyway.
> +
> 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),
> @@ -349,19 +352,23 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> if (!HAS_VRR(display))
> return;
>
> - /*
> - * FIXME all joined pipes share the same transcoder.
> - * Need to account for that during VRR toggle/push/etc.
> - */
> - if (crtc_state->joiner_pipes)
> - return;
> -
> 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));
>
> + /*
> + * Allow fixed refresh rate with VRR Timing Generator.
> + * For now set the vrr.in_range to 0, to allow fixed_rr but skip actual
> + * VRR and LRR.
> + * #TODO For actual VRR with joiner, we need to figure out how to
> + * correctly sequence transcoder level stuff vs. pipe level stuff
> + * in the commit.
> + */
> + if (crtc_state->joiner_pipes)
> + crtc_state->vrr.in_range = 0;
> +
> vmin = intel_vrr_compute_vmin(crtc_state);
>
> if (crtc_state->vrr.in_range) {
> @@ -448,6 +455,8 @@ void intel_vrr_set_transcoder_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_crtc_is_joiner_secondary(crtc_state))
> + return;
Also shouldn't get called on the secondary.
> /*
> * This bit seems to have two meanings depending on the platform:
> * TGL: generate VRR "safe window" for DSB vblank waits
> @@ -486,6 +495,9 @@ void intel_vrr_send_push(struct intel_dsb *dsb,
> if (!crtc_state->vrr.enable)
> return;
>
> + if (intel_crtc_is_joiner_secondary(crtc_state))
> + return;
> +
> if (dsb)
> intel_dsb_nonpost_start(dsb);
>
> @@ -560,6 +572,9 @@ void intel_vrr_enable(const struct intel_crtc_state *crtc_state)
> if (!crtc_state->vrr.enable)
> return;
>
> + if (intel_crtc_is_joiner_secondary(crtc_state))
> + 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),
> @@ -590,6 +605,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state)
> if (!old_crtc_state->vrr.enable)
> return;
>
> + if (intel_crtc_is_joiner_secondary(old_crtc_state))
> + return;
All thosea three cases should be impossible on account of
vrr.enable==false.
> +
> if (!intel_vrr_always_use_vrr_tg(display)) {
> intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
> trans_vrr_ctl(old_crtc_state));
> @@ -613,6 +631,9 @@ void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state)
> if (!intel_vrr_possible(crtc_state))
> return;
>
> + if (intel_crtc_is_joiner_secondary(crtc_state))
> + return;
> +
> if (!intel_vrr_always_use_vrr_tg(display)) {
> intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
> trans_vrr_ctl(crtc_state));
> @@ -637,6 +658,9 @@ void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state)
> if (!intel_vrr_possible(crtc_state))
> return;
>
> + if (intel_crtc_is_joiner_secondary(crtc_state))
> + return;
And these two again shouldn't be called on the secondary crtc.
So I think you should be able to drop all the
intel_crtc_is_joiner_secondary() checks from this patch.
> +
> if (!intel_vrr_always_use_vrr_tg(display)) {
> intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
> trans_vrr_ctl(crtc_state));
> --
> 2.45.2
--
Ville Syrjälä
Intel
