Re: [PATCH v2 1/8] drm/i915/dsb: Move the +1 usec adjustment into dsb_wait_usec()

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/8/2025 4:01 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The "wait usec" DSB command doesn't quite seem to able to
> guarantee that it always waits at least the specified
> amount of usecs. Some of that could be just because it
> supposedly just does some kind of dumb timestamp comparison
> internally. But I also see cases where two hardware timestamps
> sampled on each side of the "wait usec" command come out one
> less than expected. So it looks like we always need at least a
> +1 to guarantee that we never wait less than specified. Always
> apply that adjustment in dsb_wait_usec().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


> ---
>   drivers/gpu/drm/i915/display/intel_dsb.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 2f2812c23972..f8bd6fad0c87 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -369,7 +369,8 @@ void intel_dsb_interrupt(struct intel_dsb *dsb)
>   
>   void intel_dsb_wait_usec(struct intel_dsb *dsb, int count)
>   {
> -	intel_dsb_emit(dsb, count,
> +	/* +1 to make sure we never wait less time than asked for */
> +	intel_dsb_emit(dsb, count + 1,
>   		       DSB_OPCODE_WAIT_USEC << DSB_OPCODE_SHIFT);
>   }
>   
> @@ -622,7 +623,7 @@ void intel_dsb_wait_vblank_delay(struct intel_atomic_state *state,
>   	const struct intel_crtc_state *crtc_state =
>   		intel_pre_commit_crtc_state(state, crtc);
>   	int usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
> -					     dsb_vblank_delay(state, crtc)) + 1;
> +					     dsb_vblank_delay(state, crtc));
>   
>   	intel_dsb_wait_usec(dsb, usecs);
>   }