public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Ricardo Ribalda <ribalda@chromium.org>,
	HungNien Chen <hn.chen@sunplusit.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Tomasz Figa <tfiga@chromium.org>, Sasha Levin <sashal@kernel.org>,
	mchehab@kernel.org, linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 6.10 23/34] media: uvcvideo: Ignore empty TS packets
Date: Sun, 28 Jul 2024 11:40:47 -0400	[thread overview]
Message-ID: <20240728154230.2046786-23-sashal@kernel.org> (raw)
In-Reply-To: <20240728154230.2046786-1-sashal@kernel.org>

From: Ricardo Ribalda <ribalda@chromium.org>

[ Upstream commit 5cd7c25f6f0576073b3d03bc4cfb1e8ca63a1195 ]

Some SunplusIT cameras took a borderline interpretation of the UVC 1.5
standard, and fill the PTS and SCR fields with invalid data if the
package does not contain data.

"STC must be captured when the first video data of a video frame is put
on the USB bus."

Some SunplusIT devices send, e.g.,

buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000
buffer: 0xa7755c00 len 000012 header:0x8c stc 00000000 sof 0000 pts 00000000
buffer: 0xa7755c00 len 000668 header:0x8c stc 73779dba sof 070c pts 7376d37a

While the UVC specification meant that the first two packets shouldn't
have had the SCR bit set in the header.

This borderline/buggy interpretation has been implemented in a variety
of devices, from directly SunplusIT and from other OEMs that rebrand
SunplusIT products. So quirking based on VID:PID will be problematic.

All the affected modules have the following extension unit:
VideoControl Interface Descriptor:
  guidExtensionCode         {82066163-7050-ab49-b8cc-b3855e8d221d}

But the vendor plans to use that GUID in the future and fix the bug,
this means that we should use heuristic to figure out the broken
packets.

This patch takes care of this.

lsusb of one of the affected cameras:

Bus 001 Device 003: ID 1bcf:2a01 Sunplus Innovation Technology Inc.
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.01
  bDeviceClass          239 Miscellaneous Device
  bDeviceSubClass         2 ?
  bDeviceProtocol         1 Interface Association
  bMaxPacketSize0        64
  idVendor           0x1bcf Sunplus Innovation Technology Inc.
  idProduct          0x2a01
  bcdDevice            0.02
  iManufacturer           1 SunplusIT Inc
  iProduct                2 HanChen Wise Camera
  iSerial                 3 01.00.00
  bNumConfigurations      1

Tested-by: HungNien Chen <hn.chen@sunplusit.com>
Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Link: https://lore.kernel.org/r/20240323-resend-hwtimestamp-v10-2-b08e590d97c7@chromium.org
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/media/usb/uvc/uvc_video.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 7cbf4692bd875..11ef12d4fb698 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -478,6 +478,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	ktime_t time;
 	u16 host_sof;
 	u16 dev_sof;
+	u32 dev_stc;
 
 	switch (data[1] & (UVC_STREAM_PTS | UVC_STREAM_SCR)) {
 	case UVC_STREAM_PTS | UVC_STREAM_SCR:
@@ -526,6 +527,34 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	if (dev_sof == stream->clock.last_sof)
 		return;
 
+	dev_stc = get_unaligned_le32(&data[header_size - 6]);
+
+	/*
+	 * STC (Source Time Clock) is the clock used by the camera. The UVC 1.5
+	 * standard states that it "must be captured when the first video data
+	 * of a video frame is put on the USB bus". This is generally understood
+	 * as requiring devices to clear the payload header's SCR bit before
+	 * the first packet containing video data.
+	 *
+	 * Most vendors follow that interpretation, but some (namely SunplusIT
+	 * on some devices) always set the `UVC_STREAM_SCR` bit, fill the SCR
+	 * field with 0's,and expect that the driver only processes the SCR if
+	 * there is data in the packet.
+	 *
+	 * Ignore all the hardware timestamp information if we haven't received
+	 * any data for this frame yet, the packet contains no data, and both
+	 * STC and SOF are zero. This heuristics should be safe on compliant
+	 * devices. This should be safe with compliant devices, as in the very
+	 * unlikely case where a UVC 1.1 device would send timing information
+	 * only before the first packet containing data, and both STC and SOF
+	 * happen to be zero for a particular frame, we would only miss one
+	 * clock sample from many and the clock recovery algorithm wouldn't
+	 * suffer from this condition.
+	 */
+	if (buf && buf->bytesused == 0 && len == header_size &&
+	    dev_stc == 0 && dev_sof == 0)
+		return;
+
 	stream->clock.last_sof = dev_sof;
 
 	host_sof = usb_get_current_frame_number(stream->dev->udev);
@@ -564,7 +593,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	spin_lock_irqsave(&stream->clock.lock, flags);
 
 	sample = &stream->clock.samples[stream->clock.head];
-	sample->dev_stc = get_unaligned_le32(&data[header_size - 6]);
+	sample->dev_stc = dev_stc;
 	sample->dev_sof = dev_sof;
 	sample->host_sof = host_sof;
 	sample->host_time = time;
-- 
2.43.0


  parent reply	other threads:[~2024-07-28 15:44 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-28 15:40 [PATCH AUTOSEL 6.10 01/34] drm/xe/preempt_fence: enlarge the fence critical section Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 02/34] drm/amd/display: Handle HPD_IRQ for internal link Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 03/34] drm/amd/display: Add delay to improve LTTPR UHBR interop Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 04/34] drm/amdgpu: fix potential resource leak warning Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 05/34] drm/amdgpu/pm: Fix the param type of set_power_profile_mode Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 06/34] drm/amd/amdkfd: Fix a resource leak in svm_range_validate_and_map() Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 07/34] drm/xe/xe_guc_submit: Fix exec queue stop race condition Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 08/34] drm/amdgpu/pm: Fix the null pointer dereference for smu7 Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 09/34] drm/amdgpu: Fix the null pointer dereference to ras_manager Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 10/34] drm/amdgpu/pm: Fix the null pointer dereference in apply_state_adjust_rules Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 11/34] drm/admgpu: fix dereferencing null pointer context Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 12/34] drm/amdgpu: Add lock around VF RLCG interface Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 13/34] drm/amd/pm: Fix the null pointer dereference for vega10_hwmgr Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 14/34] drm/amd/display: Add null checks for 'stream' and 'plane' before dereferencing Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 15/34] media: amphion: Remove lock in s_ctrl callback Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 16/34] drm/amd/display: Add NULL check for 'afb' before dereferencing in amdgpu_dm_plane_handle_cursor_update Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 17/34] drm/amd/display: Wake DMCUB before sending a command for replay feature Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 18/34] drm/amd/display: reduce ODM slice count to initial new dc state only when needed Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 19/34] drm/amd/display: Don't refer to dc_sink in is_dsc_need_re_compute Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 20/34] drm/amd/display: remove dpp pipes on failure to update pipe params Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 21/34] drm/amd/display: Add null checker before passing variables Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 22/34] media: i2c: ov5647: replacing of_node_put with __free(device_node) Sasha Levin
2024-07-28 15:40 ` Sasha Levin [this message]
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 24/34] media: uvcvideo: Fix the bandwdith quirk on USB 3.x Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 25/34] media: uvcvideo: Remove mappings form uvc_device_info Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 26/34] drm/panic: depends on !VT_CONSOLE Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 27/34] drm/amd/display: Fix NULL pointer dereference for DTN log in DCN401 Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 28/34] media: xc2028: avoid use-after-free in load_firmware_cb() Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 29/34] ext4: fix uninitialized variable in ext4_inlinedir_to_tree Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 30/34] jbd2: avoid memleak in jbd2_journal_write_metadata_buffer Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 31/34] drm/amd/display: Fix null pointer deref in dcn20_resource.c Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 32/34] s390/sclp: Prevent release of buffer in I/O Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 33/34] ext4: sanity check for NULL pointer after ext4_force_shutdown Sasha Levin
2024-07-28 15:40 ` [PATCH AUTOSEL 6.10 34/34] SUNRPC: Fix a race to wake a sync task Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240728154230.2046786-23-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=hn.chen@sunplusit.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=ribalda@chromium.org \
    --cc=senozhatsky@chromium.org \
    --cc=stable@vger.kernel.org \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox