On Tue, 28 May 2024, Ville Syrjala <[email protected]> wrote: > From: Ville Syrjälä <[email protected]> > > Currently we switch from out software idea of a scanline > to the hw's idea of a scanline during the commit phase in > _intel_dsb_commit(). While that is slightly easier due to > fastsets fiddling with the timings, we'll also need to > generate proper hw scanline numbers already when emitting > DSB scanline wait instructions. So this approach won't > do in the future. Switch to hw scanline numbers earlier. > > Also intel_dsb_dewake_scanline() itself already makes > some assumptions about VRR that don't take into account > VRR toggling during fastsets, so technically delaying > the sw->hw conversion doesn't even help us. > > The other reason for delaying the conversion was that we > are using intel_get_crtc_scanline() during intel_dsb_commit() > which gives us the current sw scanline. But this is pretty > low level stuff anyway so just using raw PIPEDSL reads seems > fine here, and that of course gives us the hw scanline > directly, reducing the need to do so many conversions.
I'll take your word for the PIPEDSL part, Reviewed-by: Jani Nikula <[email protected]> > > Signed-off-by: Ville Syrjälä <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_dsb.c | 16 +++++++++------- > drivers/gpu/drm/i915/display/intel_vblank.c | 9 ++++----- > drivers/gpu/drm/i915/display/intel_vblank.h | 3 ++- > 3 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c > b/drivers/gpu/drm/i915/display/intel_dsb.c > index 319fbebd7008..63268ed2e53f 100644 > --- a/drivers/gpu/drm/i915/display/intel_dsb.c > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c > @@ -326,14 +326,16 @@ static int intel_dsb_dewake_scanline(const struct > intel_crtc_state *crtc_state) > struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev); > const struct drm_display_mode *adjusted_mode = > &crtc_state->hw.adjusted_mode; > unsigned int latency = skl_watermark_max_latency(i915, 0); > - int vblank_start; > + int vblank_start, dewake_scanline; > > if (crtc_state->vrr.enable) > vblank_start = intel_vrr_vmin_vblank_start(crtc_state); > else > vblank_start = intel_mode_vblank_start(adjusted_mode); > > - return max(0, vblank_start - intel_usecs_to_scanlines(adjusted_mode, > latency)); > + dewake_scanline = max(0, vblank_start - > intel_usecs_to_scanlines(adjusted_mode, latency)); > + > + return intel_crtc_scanline_to_hw(crtc_state, dewake_scanline); > } > > static u32 dsb_chicken(struct intel_crtc *crtc) > @@ -376,19 +378,19 @@ static void _intel_dsb_commit(struct intel_dsb *dsb, > u32 ctrl, > intel_dsb_buffer_ggtt_offset(&dsb->dsb_buf)); > > if (dewake_scanline >= 0) { > - int diff, hw_dewake_scanline; > - > - hw_dewake_scanline = intel_crtc_scanline_to_hw(crtc, > dewake_scanline); > + int diff, position; > > intel_de_write_fw(dev_priv, DSB_PMCTRL(pipe, dsb->id), > DSB_ENABLE_DEWAKE | > - DSB_SCANLINE_FOR_DEWAKE(hw_dewake_scanline)); > + DSB_SCANLINE_FOR_DEWAKE(dewake_scanline)); > > /* > * Force DEwake immediately if we're already past > * or close to racing past the target scanline. > */ > - diff = dewake_scanline - intel_get_crtc_scanline(crtc); > + position = intel_de_read_fw(dev_priv, PIPEDSL(pipe)) & > PIPEDSL_LINE_MASK; > + diff = dewake_scanline - position; > + > intel_de_write_fw(dev_priv, DSB_PMCTRL_2(pipe, dsb->id), > (diff >= 0 && diff < 5 ? DSB_FORCE_DEWAKE : > 0) | > DSB_BLOCK_DEWAKE_EXTENSION); > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c > b/drivers/gpu/drm/i915/display/intel_vblank.c > index eb80952b0cfd..2e3442fe5a5d 100644 > --- a/drivers/gpu/drm/i915/display/intel_vblank.c > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c > @@ -281,13 +281,12 @@ static int __intel_get_crtc_scanline(struct intel_crtc > *crtc) > return (position + vtotal + crtc->scanline_offset) % vtotal; > } > > -int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline) > +int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state, > + int scanline) > { > - const struct drm_vblank_crtc *vblank = > drm_crtc_vblank_crtc(&crtc->base); > - const struct drm_display_mode *mode = &vblank->hwmode; > - int vtotal = intel_mode_vtotal(mode); > + int vtotal = intel_mode_vtotal(&crtc_state->hw.adjusted_mode); > > - return (scanline + vtotal - crtc->scanline_offset) % vtotal; > + return (scanline + vtotal - intel_crtc_scanline_offset(crtc_state)) % > vtotal; > } > > /* > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h > b/drivers/gpu/drm/i915/display/intel_vblank.h > index b51ae2c1039e..b5b8bcbf0bf0 100644 > --- a/drivers/gpu/drm/i915/display/intel_vblank.h > +++ b/drivers/gpu/drm/i915/display/intel_vblank.h > @@ -39,6 +39,7 @@ void intel_wait_for_pipe_scanline_stopped(struct intel_crtc > *crtc); > void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc); > void intel_crtc_update_active_timings(const struct intel_crtc_state > *crtc_state, > bool vrr_enable); > -int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline); > +int intel_crtc_scanline_to_hw(const struct intel_crtc_state *crtc_state, > + int scanline); > > #endif /* __INTEL_VBLANK_H__ */ -- Jani Nikula, Intel
