Re: [PATCH 3/4] media: uvcvideo: Relax the constrains for interpolating the hw clock

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Mar 23, 2026 at 01:10:30PM +0000, Ricardo Ribalda wrote:
> In the initial version we set the min value to 250msec. Looks like
> 100msec can also provide a good value.

I'd like to know where the value comes from and how it has been tested.

> Now that we are at it, refactor a bit the code to make it cleaner.

Do you mean using a macro ? You can mention that explicitly here.

> Fixes: 6243c83be6ee8 ("media: uvcvideo: Allow hw clock updates with buffers not full")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index c7ebedb3450f..dcbc0941ffe6 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -494,6 +494,13 @@ static int uvc_commit_video(struct uvc_streaming *stream,
>   * Clocks and timestamps
>   */
>  
> +/*
> + * The accuracy of the hardware timestamping depends on having enough data to
> + * interpolate between the different clock domains. This value is sof cycles,
> + * this is, milliseconds.
> + */
> +#define MIN_HW_TIMESTAMP_DIFF 100

UVC prefix.

> +
>  static inline ktime_t uvc_video_get_time(void)
>  {
>  	if (uvc_clock_param == CLOCK_MONOTONIC)
> @@ -834,15 +841,12 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
>  		y2 += 2048 << 16;
>  
>  	/*
> -	 * Have at least 1/4 of a second of timestamps before we
> -	 * try to do any calculation. Otherwise we do not have enough
> -	 * precision. This value was determined by running Android CTS
> -	 * on different devices.
> +	 * Check that we have enough data to do the interpolation.
>  	 *
> -	 * dev_sof runs at 1KHz, and we have a fixed point precision of
> -	 * 16 bits.
> +	 * y1 and y2 are dev_sof with a fixed point precision of 16 bits.
>  	 */
> -	if (clock->size != clock->count && (y2 - y1) < ((1000 / 4) << 16))
> +	if (clock->size != clock->count &&
> +	    (y2 - y1) < (MIN_HW_TIMESTAMP_DIFF << 16))
>  		goto done;
>  
>  	y = (u64)(y2 - y1) * (1ULL << 31) + (u64)y1 * (u64)x2

-- 
Regards,

Laurent Pinchart