On Mon, 06 Mar 2023, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> When we change the M/N values seamlessly during a fastset we should
> also update the vblank timestamping stuff to make sure the vblank
> timestamp corrections/guesstimations come out exact.
>
> Note that only crtc_clock and framedur_ns can actually end up
> changing here during fastsets. Everything else we touch can
> only change during full modesets.
>
> Technically we should try to do this exactly at the start of
> vblank, but that would require some kind of double buffering
> scheme. Let's skip that for now and just update things right
> after the commit has been submitted to the hardware. This
> means the information will be properly up to date when the
> vblank irq handler goes to work. Only if someone ends up
> querying some vblanky stuff in between the commit and start
> of vblank may we see a slight discrepancy.
>
> Also this same problem really exists for the DRRS downclocking
> stuff. But as that is supposed to be more or less transparent
> to the user, and it only drops to low gear after a long delay
> (1 sec currently) we probably don't have to worry about it.
> Any time something is actively submitting updates DRRS will
> remain in high gear and so the timestamping constants will
> match the hardware state.
>
> Fixes: e6f29923c048 ("drm/i915: Allow M/N change during fastset on bdw+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_crtc.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index b79a8834559f..41d381bbb57a 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -686,6 +686,14 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
> */
> intel_vrr_send_push(new_crtc_state);
>
> + /*
> + * Seamless M/N update may need to update frame timings.
> + *
> + * FIXME Should be synchronized with the start of vblank somehow...
> + */
> + if (new_crtc_state->seamless_m_n && intel_crtc_needs_fastset(new_crtc_state))
> + intel_crtc_update_active_timings(new_crtc_state);
Side note, do we ensure somewhere that intel_crtc_needs_modeset() &&
intel_crtc_needs_fastset() is never true?
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> +
> local_irq_enable();
>
> if (intel_vgpu_active(dev_priv))
--
Jani Nikula, Intel Open Source Graphics Center
