On Tue, 28 May 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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 <jani.nikula@xxxxxxxxx>
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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
