| Message ID | 20250318073540.2773890-11-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Use VRR timing generator for fixed refresh rate modes | expand |
On Tue, Mar 18, 2025 at 01:05:34PM +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. > > For this intel_vrr_set_transcoder now always programs fixed timings. > Later if vrr timings are required, vrr_enable() will switch > to the real VRR timings. > > For platforms that will always use VRR TG, the VRR_CTL Enable bit is set > and reset in the transcoder enable/disable path. > > v2: Update intel_vrr_set_transcoder_timings for fixed_rr. > v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) > v4: Have separate functions to enable/disable VRR CTL > v5: > -For platforms that do not always have VRRTG on, do write bits other > than enable bit and also use write the TRANS_VRR_PUSH register. (Ville) > -Avoid writing trans_ctl_vrr if !vrr_possible(). > v6: > -Disable VRR just before intel_ddi_disable_transcoder_func(). (Ville) > -Correct the sequence of configuring PUSH and VRR Enable/Disable. (Ville) > v7: Reset trans_vrr_ctl to 0 unconditionally in > intel_vrr_transcoder_disable(). (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 5 ++ > drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++ > drivers/gpu/drm/i915/display/intel_vrr.c | 57 ++++++++++++++++----- > drivers/gpu/drm/i915/display/intel_vrr.h | 2 + > 4 files changed, 54 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index f38c998935b9..44f4465c27e2 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -78,6 +78,7 @@ > #include "intel_tc.h" > #include "intel_vdsc.h" > #include "intel_vdsc_regs.h" > +#include "intel_vrr.h" > #include "skl_scaler.h" > #include "skl_universal_plane.h" > > @@ -3249,6 +3250,8 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, > drm_dp_dpcd_poll_act_handled(&intel_dp->aux, 0); > } > > + intel_vrr_transcoder_disable(old_crtc_state); > + > intel_ddi_disable_transcoder_func(old_crtc_state); > > for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { > @@ -3522,6 +3525,8 @@ static void intel_ddi_enable(struct intel_atomic_state *state, > > intel_ddi_enable_transcoder_func(encoder, crtc_state); > > + intel_vrr_transcoder_enable(crtc_state); > + > /* Enable/Disable DP2.0 SDP split config before transcoder */ > intel_audio_sdp_split_update(crtc_state); > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index bd47cf127b4c..d2988b9a6e7b 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -1065,6 +1065,8 @@ static void mst_stream_post_disable(struct intel_atomic_state *state, > drm_dp_remove_payload_part2(&intel_dp->mst.mgr, new_mst_state, > old_payload, new_payload); > > + intel_vrr_transcoder_disable(old_crtc_state); > + > intel_ddi_disable_transcoder_func(old_crtc_state); > > for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { > @@ -1326,6 +1328,8 @@ static void mst_stream_enable(struct intel_atomic_state *state, > > intel_ddi_enable_transcoder_func(encoder, pipe_config); > > + intel_vrr_transcoder_enable(pipe_config); > + > intel_ddi_clear_act_sent(encoder, pipe_config); > > intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, trans), 0, > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c > index 66afa66c66af..d4912199c3ac 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.c > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c > @@ -460,12 +460,6 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > intel_de_rmw(display, CHICKEN_TRANS(display, cpu_transcoder), > 0, PIPE_VBLANK_WITH_DELAY); > > - if (!intel_vrr_possible(crtc_state)) { > - intel_de_write(display, > - TRANS_VRR_CTL(display, cpu_transcoder), 0); > - return; > - } > - Seems to me that removing this is what causes the state checker woes in BAT. I think we can leave this in, as our own computed state should always have a valid flipline and it's only the GOP that doesn't have one. > if (crtc_state->cmrr.enable) { > intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder), > upper_32_bits(crtc_state->cmrr.cmrr_m)); > @@ -477,14 +471,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) > lower_32_bits(crtc_state->cmrr.cmrr_n)); > } > > - 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), > - crtc_state->vrr.vmax - 1); > - 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); > + intel_vrr_set_fixed_rr_timings(crtc_state); > > if (HAS_AS_SDP(display)) > intel_de_write(display, > @@ -618,6 +605,48 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) > intel_vrr_set_fixed_rr_timings(old_crtc_state); > } > > +void intel_vrr_transcoder_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 (!HAS_VRR(display)) > + return; > + > + if (!intel_vrr_possible(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)); > + return; > + } > + > + 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)); > +} > + > +void intel_vrr_transcoder_disable(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 (!HAS_VRR(display)) > + return; > + > + if (!intel_vrr_possible(crtc_state)) > + return; > + > + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 0); > + > + intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display, cpu_transcoder), > + VRR_STATUS_VRR_EN_LIVE, 1000); > + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); > +} > + > bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) > { > return crtc_state->vrr.flipline && > diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h > index 65d2b0eead51..859f1dc8a6d7 100644 > --- a/drivers/gpu/drm/i915/display/intel_vrr.h > +++ b/drivers/gpu/drm/i915/display/intel_vrr.h > @@ -36,5 +36,7 @@ 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_is_fixed_rr(const struct intel_crtc_state *crtc_state); > +void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state); > +void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state); > > #endif /* __INTEL_VRR_H__ */ > -- > 2.45.2
On 3/19/2025 8:06 PM, Ville Syrjälä wrote: > On Tue, Mar 18, 2025 at 01:05:34PM +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. >> >> For this intel_vrr_set_transcoder now always programs fixed timings. >> Later if vrr timings are required, vrr_enable() will switch >> to the real VRR timings. >> >> For platforms that will always use VRR TG, the VRR_CTL Enable bit is set >> and reset in the transcoder enable/disable path. >> >> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. >> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) >> v4: Have separate functions to enable/disable VRR CTL >> v5: >> -For platforms that do not always have VRRTG on, do write bits other >> than enable bit and also use write the TRANS_VRR_PUSH register. (Ville) >> -Avoid writing trans_ctl_vrr if !vrr_possible(). >> v6: >> -Disable VRR just before intel_ddi_disable_transcoder_func(). (Ville) >> -Correct the sequence of configuring PUSH and VRR Enable/Disable. (Ville) >> v7: Reset trans_vrr_ctl to 0 unconditionally in >> intel_vrr_transcoder_disable(). (Ville) >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_ddi.c | 5 ++ >> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++ >> drivers/gpu/drm/i915/display/intel_vrr.c | 57 ++++++++++++++++----- >> drivers/gpu/drm/i915/display/intel_vrr.h | 2 + >> 4 files changed, 54 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c >> index f38c998935b9..44f4465c27e2 100644 >> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >> @@ -78,6 +78,7 @@ >> #include "intel_tc.h" >> #include "intel_vdsc.h" >> #include "intel_vdsc_regs.h" >> +#include "intel_vrr.h" >> #include "skl_scaler.h" >> #include "skl_universal_plane.h" >> >> @@ -3249,6 +3250,8 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, >> drm_dp_dpcd_poll_act_handled(&intel_dp->aux, 0); >> } >> >> + intel_vrr_transcoder_disable(old_crtc_state); >> + >> intel_ddi_disable_transcoder_func(old_crtc_state); >> >> for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { >> @@ -3522,6 +3525,8 @@ static void intel_ddi_enable(struct intel_atomic_state *state, >> >> intel_ddi_enable_transcoder_func(encoder, crtc_state); >> >> + intel_vrr_transcoder_enable(crtc_state); >> + >> /* Enable/Disable DP2.0 SDP split config before transcoder */ >> intel_audio_sdp_split_update(crtc_state); >> >> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c >> index bd47cf127b4c..d2988b9a6e7b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c >> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c >> @@ -1065,6 +1065,8 @@ static void mst_stream_post_disable(struct intel_atomic_state *state, >> drm_dp_remove_payload_part2(&intel_dp->mst.mgr, new_mst_state, >> old_payload, new_payload); >> >> + intel_vrr_transcoder_disable(old_crtc_state); >> + >> intel_ddi_disable_transcoder_func(old_crtc_state); >> >> for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { >> @@ -1326,6 +1328,8 @@ static void mst_stream_enable(struct intel_atomic_state *state, >> >> intel_ddi_enable_transcoder_func(encoder, pipe_config); >> >> + intel_vrr_transcoder_enable(pipe_config); >> + >> intel_ddi_clear_act_sent(encoder, pipe_config); >> >> intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, trans), 0, >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c >> index 66afa66c66af..d4912199c3ac 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >> @@ -460,12 +460,6 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) >> intel_de_rmw(display, CHICKEN_TRANS(display, cpu_transcoder), >> 0, PIPE_VBLANK_WITH_DELAY); >> >> - if (!intel_vrr_possible(crtc_state)) { >> - intel_de_write(display, >> - TRANS_VRR_CTL(display, cpu_transcoder), 0); >> - return; >> - } >> - > Seems to me that removing this is what causes the state checker woes > in BAT. I think we can leave this in, as our own computed state should > always have a valid flipline and it's only the GOP that doesn't have one. Yeah makes sense. I was wondering what was missing, thanks for spotting this. I have sent revised patch in reply to this patch. With this last patch will not be required, I'll remove the last patch. Regards, Ankit > >> if (crtc_state->cmrr.enable) { >> intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder), >> upper_32_bits(crtc_state->cmrr.cmrr_m)); >> @@ -477,14 +471,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) >> lower_32_bits(crtc_state->cmrr.cmrr_n)); >> } >> >> - 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), >> - crtc_state->vrr.vmax - 1); >> - 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); >> + intel_vrr_set_fixed_rr_timings(crtc_state); >> >> if (HAS_AS_SDP(display)) >> intel_de_write(display, >> @@ -618,6 +605,48 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) >> intel_vrr_set_fixed_rr_timings(old_crtc_state); >> } >> >> +void intel_vrr_transcoder_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 (!HAS_VRR(display)) >> + return; >> + >> + if (!intel_vrr_possible(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)); >> + return; >> + } >> + >> + 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)); >> +} >> + >> +void intel_vrr_transcoder_disable(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 (!HAS_VRR(display)) >> + return; >> + >> + if (!intel_vrr_possible(crtc_state)) >> + return; >> + >> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 0); >> + >> + intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display, cpu_transcoder), >> + VRR_STATUS_VRR_EN_LIVE, 1000); >> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); >> +} >> + >> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) >> { >> return crtc_state->vrr.flipline && >> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h >> index 65d2b0eead51..859f1dc8a6d7 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vrr.h >> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h >> @@ -36,5 +36,7 @@ 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_is_fixed_rr(const struct intel_crtc_state *crtc_state); >> +void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state); >> +void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state); >> >> #endif /* __INTEL_VRR_H__ */ >> -- >> 2.45.2
On 3/20/2025 11:09 AM, Nautiyal, Ankit K wrote: > > On 3/19/2025 8:06 PM, Ville Syrjälä wrote: >> On Tue, Mar 18, 2025 at 01:05:34PM +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. >>> >>> For this intel_vrr_set_transcoder now always programs fixed timings. >>> Later if vrr timings are required, vrr_enable() will switch >>> to the real VRR timings. >>> >>> For platforms that will always use VRR TG, the VRR_CTL Enable bit is >>> set >>> and reset in the transcoder enable/disable path. >>> >>> v2: Update intel_vrr_set_transcoder_timings for fixed_rr. >>> v3: Update intel_set_transcoder_timings_lrr for fixed_rr. (Ville) >>> v4: Have separate functions to enable/disable VRR CTL >>> v5: >>> -For platforms that do not always have VRRTG on, do write bits other >>> than enable bit and also use write the TRANS_VRR_PUSH register. (Ville) >>> -Avoid writing trans_ctl_vrr if !vrr_possible(). >>> v6: >>> -Disable VRR just before intel_ddi_disable_transcoder_func(). (Ville) >>> -Correct the sequence of configuring PUSH and VRR Enable/Disable. >>> (Ville) >>> v7: Reset trans_vrr_ctl to 0 unconditionally in >>> intel_vrr_transcoder_disable(). (Ville) >>> >>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_ddi.c | 5 ++ >>> drivers/gpu/drm/i915/display/intel_dp_mst.c | 4 ++ >>> drivers/gpu/drm/i915/display/intel_vrr.c | 57 >>> ++++++++++++++++----- >>> drivers/gpu/drm/i915/display/intel_vrr.h | 2 + >>> 4 files changed, 54 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c >>> b/drivers/gpu/drm/i915/display/intel_ddi.c >>> index f38c998935b9..44f4465c27e2 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c >>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c >>> @@ -78,6 +78,7 @@ >>> #include "intel_tc.h" >>> #include "intel_vdsc.h" >>> #include "intel_vdsc_regs.h" >>> +#include "intel_vrr.h" >>> #include "skl_scaler.h" >>> #include "skl_universal_plane.h" >>> @@ -3249,6 +3250,8 @@ static void >>> intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, >>> drm_dp_dpcd_poll_act_handled(&intel_dp->aux, 0); >>> } >>> + intel_vrr_transcoder_disable(old_crtc_state); >>> + >>> intel_ddi_disable_transcoder_func(old_crtc_state); >>> for_each_pipe_crtc_modeset_disable(display, pipe_crtc, >>> old_crtc_state, i) { >>> @@ -3522,6 +3525,8 @@ static void intel_ddi_enable(struct >>> intel_atomic_state *state, >>> intel_ddi_enable_transcoder_func(encoder, crtc_state); >>> + intel_vrr_transcoder_enable(crtc_state); >>> + >>> /* Enable/Disable DP2.0 SDP split config before transcoder */ >>> intel_audio_sdp_split_update(crtc_state); >>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> b/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> index bd47cf127b4c..d2988b9a6e7b 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c >>> @@ -1065,6 +1065,8 @@ static void mst_stream_post_disable(struct >>> intel_atomic_state *state, >>> drm_dp_remove_payload_part2(&intel_dp->mst.mgr, new_mst_state, >>> old_payload, new_payload); >>> + intel_vrr_transcoder_disable(old_crtc_state); >>> + >>> intel_ddi_disable_transcoder_func(old_crtc_state); >>> for_each_pipe_crtc_modeset_disable(display, pipe_crtc, >>> old_crtc_state, i) { >>> @@ -1326,6 +1328,8 @@ static void mst_stream_enable(struct >>> intel_atomic_state *state, >>> intel_ddi_enable_transcoder_func(encoder, pipe_config); >>> + intel_vrr_transcoder_enable(pipe_config); >>> + >>> intel_ddi_clear_act_sent(encoder, pipe_config); >>> intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, trans), 0, >>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c >>> b/drivers/gpu/drm/i915/display/intel_vrr.c >>> index 66afa66c66af..d4912199c3ac 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_vrr.c >>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c >>> @@ -460,12 +460,6 @@ void intel_vrr_set_transcoder_timings(const >>> struct intel_crtc_state *crtc_state) >>> intel_de_rmw(display, CHICKEN_TRANS(display, cpu_transcoder), >>> 0, PIPE_VBLANK_WITH_DELAY); >>> - if (!intel_vrr_possible(crtc_state)) { >>> - intel_de_write(display, >>> - TRANS_VRR_CTL(display, cpu_transcoder), 0); >>> - return; >>> - } >>> - >> Seems to me that removing this is what causes the state checker woes >> in BAT. I think we can leave this in, as our own computed state should >> always have a valid flipline and it's only the GOP that doesn't have >> one. > > Yeah makes sense. I was wondering what was missing, thanks for > spotting this. > > I have sent revised patch in reply to this patch. > > With this last patch will not be required, I'll remove the last patch. I guess we still need the last patch without that for case where platforms do not always have VRR TG enabled. For such platforms if full modeset happens, the trans_vrr_ctl gets filled, but if it doesn't happen trans_vrr_ctl is not filled, but VRR timings are all filled. Since we depend on FLIPLINE_EN in trans_vrr to be set before reading VRR.flipline/vmin/vmax, we get a mismatch. E.g: https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_146621v1/bat-rplp-1/igt@i915_module_load@load.html Regards, Ankit > > Regards, > > Ankit > > >> >>> if (crtc_state->cmrr.enable) { >>> intel_de_write(display, TRANS_CMRR_M_HI(display, >>> cpu_transcoder), >>> upper_32_bits(crtc_state->cmrr.cmrr_m)); >>> @@ -477,14 +471,7 @@ void intel_vrr_set_transcoder_timings(const >>> struct intel_crtc_state *crtc_state) >>> lower_32_bits(crtc_state->cmrr.cmrr_n)); >>> } >>> - 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), >>> - crtc_state->vrr.vmax - 1); >>> - 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); >>> + intel_vrr_set_fixed_rr_timings(crtc_state); >>> if (HAS_AS_SDP(display)) >>> intel_de_write(display, >>> @@ -618,6 +605,48 @@ void intel_vrr_disable(const struct >>> intel_crtc_state *old_crtc_state) >>> intel_vrr_set_fixed_rr_timings(old_crtc_state); >>> } >>> +void intel_vrr_transcoder_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 (!HAS_VRR(display)) >>> + return; >>> + >>> + if (!intel_vrr_possible(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)); >>> + return; >>> + } >>> + >>> + 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)); >>> +} >>> + >>> +void intel_vrr_transcoder_disable(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 (!HAS_VRR(display)) >>> + return; >>> + >>> + if (!intel_vrr_possible(crtc_state)) >>> + return; >>> + >>> + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), >>> 0); >>> + >>> + intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display, >>> cpu_transcoder), >>> + VRR_STATUS_VRR_EN_LIVE, 1000); >>> + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); >>> +} >>> + >>> bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) >>> { >>> return crtc_state->vrr.flipline && >>> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h >>> b/drivers/gpu/drm/i915/display/intel_vrr.h >>> index 65d2b0eead51..859f1dc8a6d7 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_vrr.h >>> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h >>> @@ -36,5 +36,7 @@ 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_is_fixed_rr(const struct intel_crtc_state >>> *crtc_state); >>> +void intel_vrr_transcoder_enable(const struct intel_crtc_state >>> *crtc_state); >>> +void intel_vrr_transcoder_disable(const struct intel_crtc_state >>> *crtc_state); >>> #endif /* __INTEL_VRR_H__ */ >>> -- >>> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index f38c998935b9..44f4465c27e2 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -78,6 +78,7 @@ #include "intel_tc.h" #include "intel_vdsc.h" #include "intel_vdsc_regs.h" +#include "intel_vrr.h" #include "skl_scaler.h" #include "skl_universal_plane.h" @@ -3249,6 +3250,8 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state, drm_dp_dpcd_poll_act_handled(&intel_dp->aux, 0); } + intel_vrr_transcoder_disable(old_crtc_state); + intel_ddi_disable_transcoder_func(old_crtc_state); for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { @@ -3522,6 +3525,8 @@ static void intel_ddi_enable(struct intel_atomic_state *state, intel_ddi_enable_transcoder_func(encoder, crtc_state); + intel_vrr_transcoder_enable(crtc_state); + /* Enable/Disable DP2.0 SDP split config before transcoder */ intel_audio_sdp_split_update(crtc_state); diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index bd47cf127b4c..d2988b9a6e7b 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -1065,6 +1065,8 @@ static void mst_stream_post_disable(struct intel_atomic_state *state, drm_dp_remove_payload_part2(&intel_dp->mst.mgr, new_mst_state, old_payload, new_payload); + intel_vrr_transcoder_disable(old_crtc_state); + intel_ddi_disable_transcoder_func(old_crtc_state); for_each_pipe_crtc_modeset_disable(display, pipe_crtc, old_crtc_state, i) { @@ -1326,6 +1328,8 @@ static void mst_stream_enable(struct intel_atomic_state *state, intel_ddi_enable_transcoder_func(encoder, pipe_config); + intel_vrr_transcoder_enable(pipe_config); + intel_ddi_clear_act_sent(encoder, pipe_config); intel_de_rmw(display, TRANS_DDI_FUNC_CTL(display, trans), 0, diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c index 66afa66c66af..d4912199c3ac 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.c +++ b/drivers/gpu/drm/i915/display/intel_vrr.c @@ -460,12 +460,6 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) intel_de_rmw(display, CHICKEN_TRANS(display, cpu_transcoder), 0, PIPE_VBLANK_WITH_DELAY); - if (!intel_vrr_possible(crtc_state)) { - intel_de_write(display, - TRANS_VRR_CTL(display, cpu_transcoder), 0); - return; - } - if (crtc_state->cmrr.enable) { intel_de_write(display, TRANS_CMRR_M_HI(display, cpu_transcoder), upper_32_bits(crtc_state->cmrr.cmrr_m)); @@ -477,14 +471,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state) lower_32_bits(crtc_state->cmrr.cmrr_n)); } - 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), - crtc_state->vrr.vmax - 1); - 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); + intel_vrr_set_fixed_rr_timings(crtc_state); if (HAS_AS_SDP(display)) intel_de_write(display, @@ -618,6 +605,48 @@ void intel_vrr_disable(const struct intel_crtc_state *old_crtc_state) intel_vrr_set_fixed_rr_timings(old_crtc_state); } +void intel_vrr_transcoder_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 (!HAS_VRR(display)) + return; + + if (!intel_vrr_possible(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)); + return; + } + + 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)); +} + +void intel_vrr_transcoder_disable(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 (!HAS_VRR(display)) + return; + + if (!intel_vrr_possible(crtc_state)) + return; + + intel_de_write(display, TRANS_VRR_CTL(display, cpu_transcoder), 0); + + intel_de_wait_for_clear(display, TRANS_VRR_STATUS(display, cpu_transcoder), + VRR_STATUS_VRR_EN_LIVE, 1000); + intel_de_write(display, TRANS_PUSH(display, cpu_transcoder), 0); +} + bool intel_vrr_is_fixed_rr(const struct intel_crtc_state *crtc_state) { return crtc_state->vrr.flipline && diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h index 65d2b0eead51..859f1dc8a6d7 100644 --- a/drivers/gpu/drm/i915/display/intel_vrr.h +++ b/drivers/gpu/drm/i915/display/intel_vrr.h @@ -36,5 +36,7 @@ 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_is_fixed_rr(const struct intel_crtc_state *crtc_state); +void intel_vrr_transcoder_enable(const struct intel_crtc_state *crtc_state); +void intel_vrr_transcoder_disable(const struct intel_crtc_state *crtc_state); #endif /* __INTEL_VRR_H__ */