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.nauti...@intel.com>
> ---
>  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

Reply via email to