| Message ID | 20240902080635.2946858-12-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
On Mon, Sep 02, 2024 at 01:36:32PM +0530, 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. > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > 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 5e947465c6e0..e01d4b4b8fa7 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -169,13 +169,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, > const struct drm_display_info *info = &connector->base.display_info; > 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; There's more to this than just sprinkling the secondary checks. Namely, we need to make sure the timing changes happen in the correct spot in the sequence for both primary and secondary pipes. > - > if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) > return; > > @@ -272,6 +265,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 > @@ -313,6 +309,9 @@ void intel_vrr_send_push(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; > + > if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr) > return; > > @@ -336,6 +335,9 @@ void intel_vrr_enable(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; > + > if (!crtc_state->vrr.enable) > return; > > @@ -364,6 +366,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > struct intel_display *display = to_intel_display(old_crtc_state); > enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; > > + if (intel_crtc_is_joiner_secondary(old_crtc_state)) > + return; > + > if (!old_crtc_state->vrr.enable) > return; > > -- > 2.45.2
On 9/3/2024 6:34 PM, Ville Syrjälä wrote: > On Mon, Sep 02, 2024 at 01:36:32PM +0530, 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. >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> 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 5e947465c6e0..e01d4b4b8fa7 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -169,13 +169,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, >> const struct drm_display_info *info = &connector->base.display_info; >> 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; > There's more to this than just sprinkling the secondary checks. > Namely, we need to make sure the timing changes happen in the > correct spot in the sequence for both primary and secondary pipes. Yeah, I was not very confident about this. Whether calling intel_vrr_set_transcoder_timings, intel_vrr_enable, intel_vrr_send_push for each joined pipe in reverse order will work. I think currently they are part of intel_pre_update_crtc, intel_update_crtc and intel_pipe_update_end respectively, but called for each pipe not in reverse order? I am still not very sure though. Regards, Ankit > >> - >> if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) >> return; >> >> @@ -272,6 +265,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 >> @@ -313,6 +309,9 @@ void intel_vrr_send_push(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; >> + >> if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr) >> return; >> >> @@ -336,6 +335,9 @@ void intel_vrr_enable(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; >> + >> if (!crtc_state->vrr.enable) >> return; >> >> @@ -364,6 +366,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) >> struct intel_display *display = to_intel_display(old_crtc_state); >> enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; >> >> + if (intel_crtc_is_joiner_secondary(old_crtc_state)) >> + return; >> + >> if (!old_crtc_state->vrr.enable) >> return; >> >> -- >> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 5e947465c6e0..e01d4b4b8fa7 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -169,13 +169,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state, const struct drm_display_info *info = &connector->base.display_info; 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; @@ -272,6 +265,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 @@ -313,6 +309,9 @@ void intel_vrr_send_push(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; + if (!crtc_state->vrr.enable || crtc_state->vrr.fixed_rr) return; @@ -336,6 +335,9 @@ void intel_vrr_enable(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; + if (!crtc_state->vrr.enable) return; @@ -364,6 +366,9 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) struct intel_display *display = to_intel_display(old_crtc_state); enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder; + if (intel_crtc_is_joiner_secondary(old_crtc_state)) + return; + if (!old_crtc_state->vrr.enable) return;
Do not program transcoder registers for VRR for the secondary pipe of the joiner. Remove check to skip VRR for joiner case. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_vrr.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)