[Intel-gfx] [PATCH 1/4] drm/i915: Update vblank timestamping stuff on seamless M/N change

[Golani, Mitulkumar Ajitkumar]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: 06 March 2023 20:59
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 1/4] drm/i915: Update vblank timestamping stuff
> on seamless M/N change
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> 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 at linux.intel.com>
> ---
>  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);
> +
Hi Ville,

changes LGTM. Although have a question, are we suspecting any timing param changes after Push bit is sent ?

Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>

Thanks
>  	local_irq_enable();
> 
>  	if (intel_vgpu_active(dev_priv))
> --
> 2.39.2