| Message ID | 20250214121130.1808451-17-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: > During modeset enable sequence, program the fixed timings, > and turn on the VRR Timing Generator (VRR TG) for platforms > that always use VRR TG. > > Later if vrr timings are required, vrr_enable() will switch > to the real VRR timings. > > With this we dont want to reset TRANS_CTL_VRR Enable bit while updating > the vrr transcoder timings. > > v2: Update intel_vrr_set_transcoder_timings for fixed_rr. > v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) > Add TRANS_PUSH while enabling VRR for fixed_rr. > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- > drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- > drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ > 3 files changed, 36 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 66c5ad46bfea..c9d1c091b109 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) > > static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) > { > + struct intel_display *display = to_intel_display(crtc_state); > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta > } > > intel_set_transcoder_timings(crtc_state); > - if (HAS_VRR(dev_priv)) > - intel_vrr_set_transcoder_timings(crtc_state); > - > + if (HAS_VRR(dev_priv)) { > + if (intel_vrr_always_use_vrr_tg(display)) > + intel_vrr_enable_fixed_rr_timings(crtc_state); > + else > + intel_vrr_set_transcoder_timings(crtc_state); I think intel_vrr_set_transcoder_timings() should just program the fixed timings always. And we shouldn't do anything else here. And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true case should be done at some more appropriate spot in the modeset sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for that part. > + } > if (cpu_transcoder != TRANSCODER_EDP) > intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), > crtc_state->pixel_multiplier - 1); > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index e247055bc486..4151fcd0f978 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > crtc_state->vrr.vmin - 1); > intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), > crtc_state->vrr.vmax - 1); > - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > - trans_vrr_ctl(crtc_state)); > + if (intel_vrr_always_use_vrr_tg(display)) > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > + else > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > + trans_vrr_ctl(crtc_state)); We should probably remove this VRR_CTL frobbing from here entirely, and just always do it from the intel_vrr_trancoder_{enable,disable}(). And obviously for the !always_use_vrr_tg case we just skip setting the enable bit there. > intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), > crtc_state->vrr.flipline - 1); > > @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) > return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; > } > > -static > bool intel_vrr_always_use_vrr_tg(struct intel_display *display) > { > if (!HAS_VRR(display)) > @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > intel_vrr_set_fixed_rr_timings(old_crtc_state); > } > > +void intel_vrr_enable_fixed_rr_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; > + > + intel_vrr_set_fixed_rr_timings(crtc_state); > + > + if (HAS_AS_SDP(display)) > + intel_de_write(display, > + TRANS_VRR_VSYNC(display, cpu_transcoder), > + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | > + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); > + > + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), > + TRANS_PUSH_EN); > + > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > +} > + > static > bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) > { > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 514822577e8a..9259964978b1 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -12,6 +12,7 @@ struct drm_connector_state; > struct intel_atomic_state; > struct intel_connector; > struct intel_crtc_state; > +struct intel_display; > struct intel_dsb; > > bool intel_vrr_is_capable(struct intel_connector *connector); > @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); > int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); > int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); > int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); > +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); > +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); > > #endif /* __INTEL_VRR_H__ */ > -- > 2.45.2
On Mon, Feb 17, 2025 at 08:34:39PM +0200, Ville Syrjälä wrote: > On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: > > During modeset enable sequence, program the fixed timings, > > and turn on the VRR Timing Generator (VRR TG) for platforms > > that always use VRR TG. > > > > Later if vrr timings are required, vrr_enable() will switch > > to the real VRR timings. > > > > With this we dont want to reset TRANS_CTL_VRR Enable bit while updating > > the vrr transcoder timings. > > > > v2: Update intel_vrr_set_transcoder_timings for fixed_rr. > > v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) > > Add TRANS_PUSH while enabling VRR for fixed_rr. > > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- > > drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- > > drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ > > 3 files changed, 36 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index 66c5ad46bfea..c9d1c091b109 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) > > > > static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) > > { > > + struct intel_display *display = to_intel_display(crtc_state); > > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta > > } > > > > intel_set_transcoder_timings(crtc_state); > > - if (HAS_VRR(dev_priv)) > > - intel_vrr_set_transcoder_timings(crtc_state); > > - > > + if (HAS_VRR(dev_priv)) { > > + if (intel_vrr_always_use_vrr_tg(display)) > > + intel_vrr_enable_fixed_rr_timings(crtc_state); > > + else > > + intel_vrr_set_transcoder_timings(crtc_state); > > I think intel_vrr_set_transcoder_timings() should just program the > fixed timings always. And we shouldn't do anything else here. > > And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true > case should be done at some more appropriate spot in the modeset > sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for > that part. > > > + } > > if (cpu_transcoder != TRANSCODER_EDP) > > intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), > > crtc_state->pixel_multiplier - 1); > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > > index e247055bc486..4151fcd0f978 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > > @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > > crtc_state->vrr.vmin - 1); > > intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), > > crtc_state->vrr.vmax - 1); > > - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > > - trans_vrr_ctl(crtc_state)); > > + if (intel_vrr_always_use_vrr_tg(display)) > > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > > + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > > + else > > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > > + trans_vrr_ctl(crtc_state)); > > We should probably remove this VRR_CTL frobbing from here entirely, > and just always do it from the intel_vrr_trancoder_{enable,disable}(). > And obviously for the !always_use_vrr_tg case we just skip setting > the enable bit there. Hmm. One thing that might screw us over here is the fastboot vs. vblank_delay stuff. We might have to allow that to reconfigure the guardband... In that case we need to reprogram VRR_CTL from intel_set_transcoder_timings_lrr(), and be careful what we do with the enable bit. So it'd perhaps have to be something like: vrr_ctl = trans_vrr_ctl(); if (always_use_vrr_tg && !needs_modeset) vrr_ctl |= VRR_CTL_VRR_ENABLE; Either way we should move the guardbad and pipeline_full checks out from the pure !fastset block in intel_pipe_config_compare(). And if we do need the fastboot stuff for them then we need to use the allow_vblank_delay_fastset() stuff for them as well. > > > intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), > > crtc_state->vrr.flipline - 1); > > > > @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) > > return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; > > } > > > > -static > > bool intel_vrr_always_use_vrr_tg(struct intel_display *display) > > { > > if (!HAS_VRR(display)) > > @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > > intel_vrr_set_fixed_rr_timings(old_crtc_state); > > } > > > > +void intel_vrr_enable_fixed_rr_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; > > + > > + intel_vrr_set_fixed_rr_timings(crtc_state); > > + > > + if (HAS_AS_SDP(display)) > > + intel_de_write(display, > > + TRANS_VRR_VSYNC(display, cpu_transcoder), > > + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | > > + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); > > + > > + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), > > + TRANS_PUSH_EN); > > + > > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > > + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > > +} > > + > > static > > bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) > > { > > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > > index 514822577e8a..9259964978b1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > > @@ -12,6 +12,7 @@ struct drm_connector_state; > > struct intel_atomic_state; > > struct intel_connector; > > struct intel_crtc_state; > > +struct intel_display; > > struct intel_dsb; > > > > bool intel_vrr_is_capable(struct intel_connector *connector); > > @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); > > int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); > > int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); > > int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); > > +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); > > +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); > > > > #endif /* __INTEL_VRR_H__ */ > > -- > > 2.45.2 > > -- > Ville Syrjälä > Intel
On 2/18/2025 12:04 AM, Ville Syrjälä wrote: > On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: >> During modeset enable sequence, program the fixed timings, >> and turn on the VRR Timing Generator (VRR TG) for platforms >> that always use VRR TG. >> >> Later if vrr timings are required, vrr_enable() will switch >> to the real VRR timings. >> >> With this we dont want to reset TRANS_CTL_VRR Enable bit while updating >> the vrr transcoder timings. >> >> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. >> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) >> Add TRANS_PUSH while enabling VRR for fixed_rr. >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- >> drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- >> drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ >> 3 files changed, 36 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >> index 66c5ad46bfea..c9d1c091b109 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) >> >> static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) >> { >> + struct intel_display *display = to_intel_display(crtc_state); >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; >> @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta >> } >> >> intel_set_transcoder_timings(crtc_state); >> - if (HAS_VRR(dev_priv)) >> - intel_vrr_set_transcoder_timings(crtc_state); >> - >> + if (HAS_VRR(dev_priv)) { >> + if (intel_vrr_always_use_vrr_tg(display)) >> + intel_vrr_enable_fixed_rr_timings(crtc_state); >> + else >> + intel_vrr_set_transcoder_timings(crtc_state); > I think intel_vrr_set_transcoder_timings() should just program the > fixed timings always. And we shouldn't do anything else here. > > And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true > case should be done at some more appropriate spot in the modeset > sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for > that part. Alright. So will do the following changes: In intel_vrr_set_transcoder: -Avoid modifying VRR_CTL, currently, it disables VRR_VTL if flipline is not set, and also writes guardband and other settings in VRR_CTL. -Instead of setting variable timings, just set the fixed timings always. Use separate functions for enabling/disabling VRR CTL: -As suggested will create intel_vrr_transcoder_enable and intel_vrr_transcoder_disable functions. -These functions should handle VRR_CTL settings for platforms where TG is not always used. -I am thinking to call these functions based on vrr.flipline: if (is_enabling(vrr.flipline, old_crtc_state, new_crtc_state)) intel_vrr_transcoder_enable(new_crtc_state) Thinking to add this in commit_pipe_post_planes() just before intel_vrr_enable(). if (is_disabling(vrr.flipline, old_crtc_state, new_crtc_state)) intel_vrr_transcoder_disable(new_crtc_state) Will try adding this in intel_pre_plane_update() after intel_vrr_disable(). Please correct me if I have missed something or got something wrong. Regards, Ankit >> + } >> if (cpu_transcoder != TRANSCODER_EDP) >> intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), >> crtc_state->pixel_multiplier - 1); >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >> index e247055bc486..4151fcd0f978 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) >> crtc_state->vrr.vmin - 1); >> intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), >> crtc_state->vrr.vmax - 1); >> - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >> - trans_vrr_ctl(crtc_state)); >> + if (intel_vrr_always_use_vrr_tg(display)) >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); >> + else >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >> + trans_vrr_ctl(crtc_state)); > We should probably remove this VRR_CTL frobbing from here entirely, > and just always do it from the intel_vrr_trancoder_{enable,disable}(). > And obviously for the !always_use_vrr_tg case we just skip setting > the enable bit there. > >> intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), >> crtc_state->vrr.flipline - 1); >> >> @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) >> return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; >> } >> >> -static >> bool intel_vrr_always_use_vrr_tg(struct intel_display *display) >> { >> if (!HAS_VRR(display)) >> @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) >> intel_vrr_set_fixed_rr_timings(old_crtc_state); >> } >> >> +void intel_vrr_enable_fixed_rr_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; >> + >> + intel_vrr_set_fixed_rr_timings(crtc_state); >> + >> + if (HAS_AS_SDP(display)) >> + intel_de_write(display, >> + TRANS_VRR_VSYNC(display, cpu_transcoder), >> + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | >> + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); >> + >> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), >> + TRANS_PUSH_EN); >> + >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); >> +} >> + >> static >> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) >> { >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h >> index 514822577e8a..9259964978b1 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.h >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h >> @@ -12,6 +12,7 @@ struct drm_connector_state; >> struct intel_atomic_state; >> struct intel_connector; >> struct intel_crtc_state; >> +struct intel_display; >> struct intel_dsb; >> >> bool intel_vrr_is_capable(struct intel_connector *connector); >> @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); >> int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); >> int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); >> int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); >> +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); >> +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); >> >> #endif /* __INTEL_VRR_H__ */ >> -- >> 2.45.2
On 2/18/2025 12:26 AM, Ville Syrjälä wrote: > On Mon, Feb 17, 2025 at 08:34:39PM +0200, Ville Syrjälä wrote: >> On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: >>> During modeset enable sequence, program the fixed timings, >>> and turn on the VRR Timing Generator (VRR TG) for platforms >>> that always use VRR TG. >>> >>> Later if vrr timings are required, vrr_enable() will switch >>> to the real VRR timings. >>> >>> With this we dont want to reset TRANS_CTL_VRR Enable bit while updating >>> the vrr transcoder timings. >>> >>> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. >>> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) >>> Add TRANS_PUSH while enabling VRR for fixed_rr. >>> >>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- >>> drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- >>> drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ >>> 3 files changed, 36 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c >>> index 66c5ad46bfea..c9d1c091b109 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_display.c >>> +++ b/drivers/gpu/drm/i915/display/intel_display.c >>> @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) >>> >>> static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) >>> { >>> + struct intel_display *display = to_intel_display(crtc_state); >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); >>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >>> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; >>> @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta >>> } >>> >>> intel_set_transcoder_timings(crtc_state); >>> - if (HAS_VRR(dev_priv)) >>> - intel_vrr_set_transcoder_timings(crtc_state); >>> - >>> + if (HAS_VRR(dev_priv)) { >>> + if (intel_vrr_always_use_vrr_tg(display)) >>> + intel_vrr_enable_fixed_rr_timings(crtc_state); >>> + else >>> + intel_vrr_set_transcoder_timings(crtc_state); >> I think intel_vrr_set_transcoder_timings() should just program the >> fixed timings always. And we shouldn't do anything else here. >> >> And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true >> case should be done at some more appropriate spot in the modeset >> sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for >> that part. >> >>> + } >>> if (cpu_transcoder != TRANSCODER_EDP) >>> intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), >>> crtc_state->pixel_multiplier - 1); >>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >>> index e247055bc486..4151fcd0f978 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >>> @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) >>> crtc_state->vrr.vmin - 1); >>> intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), >>> crtc_state->vrr.vmax - 1); >>> - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >>> - trans_vrr_ctl(crtc_state)); >>> + if (intel_vrr_always_use_vrr_tg(display)) >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >>> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); >>> + else >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >>> + trans_vrr_ctl(crtc_state)); >> We should probably remove this VRR_CTL frobbing from here entirely, >> and just always do it from the intel_vrr_trancoder_{enable,disable}(). >> And obviously for the !always_use_vrr_tg case we just skip setting >> the enable bit there. > Hmm. One thing that might screw us over here is the fastboot vs. > vblank_delay stuff. We might have to allow that to reconfigure > the guardband... > > In that case we need to reprogram VRR_CTL from > intel_set_transcoder_timings_lrr(), and be careful what > we do with the enable bit. So it'd perhaps have to be > something like: > vrr_ctl = trans_vrr_ctl(); > if (always_use_vrr_tg && !needs_modeset) > vrr_ctl |= VRR_CTL_VRR_ENABLE; I understand while using fastset for platforms that use vrr TG, we dont want to reset the VRR_ENABLE while setting the guardband. We want to use the function intel_vrr_transcoder_enable to handle bits for VRR_CTL. Do you mean to pass need_modeset flag to intel_vrr_transcoder_enable()? Regards, Ankit > > Either way we should move the guardbad and pipeline_full checks > out from the pure !fastset block in intel_pipe_config_compare(). > And if we do need the fastboot stuff for them then we need to use > the allow_vblank_delay_fastset() stuff for them as well. > >>> intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), >>> crtc_state->vrr.flipline - 1); >>> >>> @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) >>> return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; >>> } >>> >>> -static >>> bool intel_vrr_always_use_vrr_tg(struct intel_display *display) >>> { >>> if (!HAS_VRR(display)) >>> @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) >>> intel_vrr_set_fixed_rr_timings(old_crtc_state); >>> } >>> >>> +void intel_vrr_enable_fixed_rr_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; >>> + >>> + intel_vrr_set_fixed_rr_timings(crtc_state); >>> + >>> + if (HAS_AS_SDP(display)) >>> + intel_de_write(display, >>> + TRANS_VRR_VSYNC(display, cpu_transcoder), >>> + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | >>> + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); >>> + >>> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), >>> + TRANS_PUSH_EN); >>> + >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >>> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); >>> +} >>> + >>> static >>> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) >>> { >>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h >>> index 514822577e8a..9259964978b1 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_vrr.h >>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h >>> @@ -12,6 +12,7 @@ struct drm_connector_state; >>> struct intel_atomic_state; >>> struct intel_connector; >>> struct intel_crtc_state; >>> +struct intel_display; >>> struct intel_dsb; >>> >>> bool intel_vrr_is_capable(struct intel_connector *connector); >>> @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); >>> int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); >>> int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); >>> int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); >>> +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); >>> +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); >>> >>> #endif /* __INTEL_VRR_H__ */ >>> -- >>> 2.45.2 >> -- >> Ville Syrjälä >> Intel
On Wed, Feb 19, 2025 at 06:47:20PM +0530, Nautiyal, Ankit K wrote: > > On 2/18/2025 12:04 AM, Ville Syrjälä wrote: > > On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: > >> During modeset enable sequence, program the fixed timings, > >> and turn on the VRR Timing Generator (VRR TG) for platforms > >> that always use VRR TG. > >> > >> Later if vrr timings are required, vrr_enable() will switch > >> to the real VRR timings. > >> > >> With this we dont want to reset TRANS_CTL_VRR Enable bit while updating > >> the vrr transcoder timings. > >> > >> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. > >> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) > >> Add TRANS_PUSH while enabling VRR for fixed_rr. > >> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- > >> drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- > >> drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ > >> 3 files changed, 36 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > >> index 66c5ad46bfea..c9d1c091b109 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >> @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) > >> > >> static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) > >> { > >> + struct intel_display *display = to_intel_display(crtc_state); > >> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >> @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta > >> } > >> > >> intel_set_transcoder_timings(crtc_state); > >> - if (HAS_VRR(dev_priv)) > >> - intel_vrr_set_transcoder_timings(crtc_state); > >> - > >> + if (HAS_VRR(dev_priv)) { > >> + if (intel_vrr_always_use_vrr_tg(display)) > >> + intel_vrr_enable_fixed_rr_timings(crtc_state); > >> + else > >> + intel_vrr_set_transcoder_timings(crtc_state); > > I think intel_vrr_set_transcoder_timings() should just program the > > fixed timings always. And we shouldn't do anything else here. > > > > And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true > > case should be done at some more appropriate spot in the modeset > > sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for > > that part. > > Alright. So will do the following changes: > In intel_vrr_set_transcoder: > -Avoid modifying VRR_CTL, currently, it disables VRR_VTL if flipline is > not set, and also writes guardband and other settings in VRR_CTL. > > -Instead of setting variable timings, just set the fixed timings always. > > Use separate functions for enabling/disabling VRR CTL: > -As suggested will create intel_vrr_transcoder_enable and > intel_vrr_transcoder_disable functions. > -These functions should handle VRR_CTL settings for platforms where TG > is not always used. > -I am thinking to call these functions based on vrr.flipline: > > if (is_enabling(vrr.flipline, old_crtc_state, new_crtc_state)) > intel_vrr_transcoder_enable(new_crtc_state) > > Thinking to add this in commit_pipe_post_planes() just before > intel_vrr_enable(). > > if (is_disabling(vrr.flipline, old_crtc_state, new_crtc_state)) > intel_vrr_transcoder_disable(new_crtc_state) We want them called unconditionally from either intel_{enable,disable}_transcoder() or perhaps from some ddi encoder hook (not sure how early/late this needs to be done). And they should just do nothing for the !always_use_vrr_tg case. > > Will try adding this in intel_pre_plane_update() after intel_vrr_disable(). > > Please correct me if I have missed something or got something wrong. > > Regards, > > Ankit > > >> + } > >> if (cpu_transcoder != TRANSCODER_EDP) > >> intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), > >> crtc_state->pixel_multiplier - 1); > >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > >> index e247055bc486..4151fcd0f978 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c > >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > >> @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > >> crtc_state->vrr.vmin - 1); > >> intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), > >> crtc_state->vrr.vmax - 1); > >> - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >> - trans_vrr_ctl(crtc_state)); > >> + if (intel_vrr_always_use_vrr_tg(display)) > >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > >> + else > >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >> + trans_vrr_ctl(crtc_state)); > > We should probably remove this VRR_CTL frobbing from here entirely, > > and just always do it from the intel_vrr_trancoder_{enable,disable}(). > > And obviously for the !always_use_vrr_tg case we just skip setting > > the enable bit there. > > > >> intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), > >> crtc_state->vrr.flipline - 1); > >> > >> @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) > >> return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; > >> } > >> > >> -static > >> bool intel_vrr_always_use_vrr_tg(struct intel_display *display) > >> { > >> if (!HAS_VRR(display)) > >> @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > >> intel_vrr_set_fixed_rr_timings(old_crtc_state); > >> } > >> > >> +void intel_vrr_enable_fixed_rr_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; > >> + > >> + intel_vrr_set_fixed_rr_timings(crtc_state); > >> + > >> + if (HAS_AS_SDP(display)) > >> + intel_de_write(display, > >> + TRANS_VRR_VSYNC(display, cpu_transcoder), > >> + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | > >> + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); > >> + > >> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), > >> + TRANS_PUSH_EN); > >> + > >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > >> +} > >> + > >> static > >> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) > >> { > >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > >> index 514822577e8a..9259964978b1 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_vrr.h > >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > >> @@ -12,6 +12,7 @@ struct drm_connector_state; > >> struct intel_atomic_state; > >> struct intel_connector; > >> struct intel_crtc_state; > >> +struct intel_display; > >> struct intel_dsb; > >> > >> bool intel_vrr_is_capable(struct intel_connector *connector); > >> @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); > >> int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); > >> int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); > >> int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); > >> +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); > >> +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); > >> > >> #endif /* __INTEL_VRR_H__ */ > >> -- > >> 2.45.2
On Wed, Feb 19, 2025 at 07:27:00PM +0530, Nautiyal, Ankit K wrote: > > On 2/18/2025 12:26 AM, Ville Syrjälä wrote: > > On Mon, Feb 17, 2025 at 08:34:39PM +0200, Ville Syrjälä wrote: > >> On Fri, Feb 14, 2025 at 05:41:26PM +0530, Ankit Nautiyal wrote: > >>> During modeset enable sequence, program the fixed timings, > >>> and turn on the VRR Timing Generator (VRR TG) for platforms > >>> that always use VRR TG. > >>> > >>> Later if vrr timings are required, vrr_enable() will switch > >>> to the real VRR timings. > >>> > >>> With this we dont want to reset TRANS_CTL_VRR Enable bit while updating > >>> the vrr transcoder timings. > >>> > >>> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. > >>> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) > >>> Add TRANS_PUSH while enabling VRR for fixed_rr. > >>> > >>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- > >>> drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- > >>> drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ > >>> 3 files changed, 36 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > >>> index 66c5ad46bfea..c9d1c091b109 100644 > >>> --- a/drivers/gpu/drm/i915/display/intel_display.c > >>> +++ b/drivers/gpu/drm/i915/display/intel_display.c > >>> @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) > >>> > >>> static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) > >>> { > >>> + struct intel_display *display = to_intel_display(crtc_state); > >>> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); > >>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > >>> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >>> @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta > >>> } > >>> > >>> intel_set_transcoder_timings(crtc_state); > >>> - if (HAS_VRR(dev_priv)) > >>> - intel_vrr_set_transcoder_timings(crtc_state); > >>> - > >>> + if (HAS_VRR(dev_priv)) { > >>> + if (intel_vrr_always_use_vrr_tg(display)) > >>> + intel_vrr_enable_fixed_rr_timings(crtc_state); > >>> + else > >>> + intel_vrr_set_transcoder_timings(crtc_state); > >> I think intel_vrr_set_transcoder_timings() should just program the > >> fixed timings always. And we shouldn't do anything else here. > >> > >> And I think the VRR_CTL enable/disable for the always_use_vrr_tg==true > >> case should be done at some more appropriate spot in the modeset > >> sequence. We can add eg. intel_vrr_trancoder_{enable,disable}() for > >> that part. > >> > >>> + } > >>> if (cpu_transcoder != TRANSCODER_EDP) > >>> intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), > >>> crtc_state->pixel_multiplier - 1); > >>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > >>> index e247055bc486..4151fcd0f978 100644 > >>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c > >>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > >>> @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > >>> crtc_state->vrr.vmin - 1); > >>> intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), > >>> crtc_state->vrr.vmax - 1); > >>> - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >>> - trans_vrr_ctl(crtc_state)); > >>> + if (intel_vrr_always_use_vrr_tg(display)) > >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >>> + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); > >>> + else > >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), > >>> + trans_vrr_ctl(crtc_state)); > >> We should probably remove this VRR_CTL frobbing from here entirely, > >> and just always do it from the intel_vrr_trancoder_{enable,disable}(). > >> And obviously for the !always_use_vrr_tg case we just skip setting > >> the enable bit there. > > Hmm. One thing that might screw us over here is the fastboot vs. > > vblank_delay stuff. We might have to allow that to reconfigure > > the guardband... > > > > In that case we need to reprogram VRR_CTL from > > intel_set_transcoder_timings_lrr(), and be careful what > > we do with the enable bit. So it'd perhaps have to be > > something like: > > vrr_ctl = trans_vrr_ctl(); > > if (always_use_vrr_tg && !needs_modeset) > > vrr_ctl |= VRR_CTL_VRR_ENABLE; > > I understand while using fastset for platforms that use vrr TG, we dont > want to reset the VRR_ENABLE while setting the guardband. > > We want to use the function intel_vrr_transcoder_enable to handle bits > for VRR_CTL. > > Do you mean to pass need_modeset flag to intel_vrr_transcoder_enable()? intel_vrr_transcoder_{enable,disable}() should just be intel_vrr_transcoder_enable() { if (!always_use_vrr_tg) return; enable vrr tg } intel_vrr_transcoder_disable() { if (!always_use_vrr_tg) return; disable vrr tg } and they should only be called from the full modeset path.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 66c5ad46bfea..c9d1c091b109 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -1755,6 +1755,7 @@ static void hsw_set_frame_start_delay(const struct intel_crtc_state *crtc_state) static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_state) { + struct intel_display *display = to_intel_display(crtc_state); struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; @@ -1770,9 +1771,12 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta } intel_set_transcoder_timings(crtc_state); - if (HAS_VRR(dev_priv)) - intel_vrr_set_transcoder_timings(crtc_state); - + if (HAS_VRR(dev_priv)) { + if (intel_vrr_always_use_vrr_tg(display)) + intel_vrr_enable_fixed_rr_timings(crtc_state); + else + intel_vrr_set_transcoder_timings(crtc_state); + } if (cpu_transcoder != TRANSCODER_EDP) intel_de_write(dev_priv, TRANS_MULT(dev_priv, cpu_transcoder), crtc_state->pixel_multiplier - 1); diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index e247055bc486..4151fcd0f978 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -456,8 +456,12 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) crtc_state->vrr.vmin - 1); intel_de_write(display, TRANS_VRR_VMAX(display, cpu_transcoder), crtc_state->vrr.vmax - 1); - intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), - trans_vrr_ctl(crtc_state)); + if (intel_vrr_always_use_vrr_tg(display)) + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); + else + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), + trans_vrr_ctl(crtc_state)); intel_de_write(display, TRANS_VRR_FLIPLINE(display, cpu_transcoder), crtc_state->vrr.flipline - 1); @@ -536,7 +540,6 @@ bool intel_vrr_is_push_sent(const struct intel_crtc_state *crtc_state) return intel_de_read(display, TRANS_PUSH(display, cpu_transcoder)) & TRANS_PUSH_SEND; } -static bool intel_vrr_always_use_vrr_tg(struct intel_display *display) { if (!HAS_VRR(display)) @@ -616,6 +619,26 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) intel_vrr_set_fixed_rr_timings(old_crtc_state); } +void intel_vrr_enable_fixed_rr_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; + + intel_vrr_set_fixed_rr_timings(crtc_state); + + if (HAS_AS_SDP(display)) + intel_de_write(display, + TRANS_VRR_VSYNC(display, cpu_transcoder), + VRR_VSYNC_END(crtc_state->vrr.vsync_end) | + VRR_VSYNC_START(crtc_state->vrr.vsync_start)); + + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), + TRANS_PUSH_EN); + + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), + VRR_CTL_VRR_ENABLE | trans_vrr_ctl(crtc_state)); +} + static bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) { diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index 514822577e8a..9259964978b1 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -12,6 +12,7 @@ struct drm_connector_state; struct intel_atomic_state; struct intel_connector; struct intel_crtc_state; +struct intel_display; struct intel_dsb; bool intel_vrr_is_capable(struct intel_connector *connector); @@ -35,5 +36,7 @@ int intel_vrr_vmin_vtotal(const struct intel_crtc_state *crtc_state); int intel_vrr_vmax_vblank_start(const struct intel_crtc_state *crtc_state); int intel_vrr_vmin_vblank_start(const struct intel_crtc_state *crtc_state); int intel_vrr_vblank_delay(const struct intel_crtc_state *crtc_state); +bool intel_vrr_always_use_vrr_tg(struct intel_display *display); +void intel_vrr_enable_fixed_rr_timings(const struct intel_crtc_state *crtc_state); #endif /* __INTEL_VRR_H__ */
During modeset enable sequence, program the fixed timings, and turn on the VRR Timing Generator (VRR TG) for platforms that always use VRR TG. Later if vrr timings are required, vrr_enable() will switch to the real VRR timings. With this we dont want to reset TRANS_CTL_VRR Enable bit while updating the vrr transcoder timings. v2: Update intel_vrr_set_transcoder_timings for fixed_rr. v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) Add TRANS_PUSH while enabling VRR for fixed_rr. Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display.c | 10 +++++-- drivers/gpu/drm/i915/display/intel_vrr.c | 29 ++++++++++++++++++-- drivers/gpu/drm/i915/display/intel_vrr.h | 3 ++ 3 files changed, 36 insertions(+), 6 deletions(-)