Re: [PATCH 13/20] drm/i915/vrr: Handle joiner with vrr

["Nautiyal, Ankit K" <ankit.k.nautiyal@xxxxxxxxx>]

On 2/26/2025 8:44 PM, Ville Syrjälä wrote:
> On Mon, Feb 24, 2025 at 07:17:59PM +0530, Nautiyal, Ankit K wrote:
> > On 2/24/2025 11:47 AM, Ankit Nautiyal wrote:
> > > Do not program transcoder registers for VRR for the secondary pipe of
> > > the joiner. Remove check to skip VRR for joiner case.
> > Missed to drop this patch as mentioned in the last version.
> >
> > Will work on this after the other changes are agreed upon, so this can
> > be ignored for now.
> I think before trying to do full joiner+vrr support we should
> just allow joiner+fixed_rr (and also not allow update_lrr).
> That would avoid the problems with the live VRR reprogramming.

That makes sense.

Will allow joiner + fixed_rr and skip actual vrr and lrr when joiner is 
used.

Regards,

Ankit

> > Regards,
> >
> > Ankit
> >
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> > > ---
> > >    drivers/gpu/drm/i915/display/intel_vrr.c | 19 ++++++++++++-------
> > >    1 file changed, 12 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > index 45efcb57c77a..30fcd1e66a12 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > @@ -345,13 +345,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> > >    	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> > >    	int vmin, vmax;
> > >    
> > > -	/*
> > > -	 * 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;
> > >    
> > > @@ -443,6 +436,9 @@ 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;
> > > +
> > >    	/*
> > >    	 * This bit seems to have two meanings depending on the platform:
> > >    	 * TGL: generate VRR "safe window" for DSB vblank waits
> > > @@ -494,6 +490,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);
> > >    
> > > @@ -558,6 +557,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),
> > > @@ -586,6 +588,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;
> > > +
> > >    	intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder),
> > >    		       trans_vrr_ctl(old_crtc_state));
> > >    	intel_de_wait_for_clear(display,