public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Thorsten Schmelzer" <tschmelzer@topcon.com>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Michael Tretter" <m.tretter@pengutronix.de>,
	"Hans Verkuil" <hverkuil+cisco@kernel.org>,
	"Sasha Levin" <sashal@kernel.org>,
	lars@metafoo.de, linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-5.10] media: adv7180: fix frame interval in progressive mode
Date: Fri, 13 Feb 2026 19:58:13 -0500	[thread overview]
Message-ID: <20260214010245.3671907-13-sashal@kernel.org> (raw)
In-Reply-To: <20260214010245.3671907-1-sashal@kernel.org>

From: Thorsten Schmelzer <tschmelzer@topcon.com>

[ Upstream commit 90289b67c5c1d4c18784059b27460d292e16d208 ]

The ADV7280-M may internally convert interlaced video input to
progressive video. If this mode is enabled, the ADV7280-M delivers
progressive video frames at the field rate of 50 fields per second (PAL)
or 60 fields per second (NTSC).

Fix the reported frame interval if progressive video is enabled.

Signed-off-by: Thorsten Schmelzer <tschmelzer@topcon.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of media: adv7180: fix frame interval in progressive mode

### Commit Message Analysis

The commit message clearly states this is a **fix** for incorrect frame
interval reporting when the ADV7280-M chip's de-interlacer is active.
The chip converts interlaced video to progressive frames at the field
rate (50 fps for PAL, 60 fps for NTSC), but the driver was reporting the
interlaced frame rate (25 fps for PAL, 30 fps for NTSC). This means the
reported frame interval was wrong by a factor of 2.

The commit has been reviewed by Niklas Söderlund and merged by Hans
Verkuil (V4L2 maintainer), which are good trust indicators.

### Code Change Analysis

The change is extremely small and surgical - it adds a single
conditional block:

```c
if (state->field == V4L2_FIELD_NONE)
    fi->interval.denominator *= 2;
```

This checks if the de-interlacer is active (`V4L2_FIELD_NONE` means
progressive/non-interlaced output) and, if so, doubles the denominator
of the frame interval fraction, effectively doubling the reported frame
rate from 25→50 (PAL) or ~30→~60 (NTSC).

The logic is straightforward:
- Previously: PAL reported 1/25, NTSC reported 1001/30000
- With fix when progressive: PAL reports 1/50, NTSC reports 1001/60000

This is mathematically correct - when de-interlacing at the field rate,
you get twice as many frames per second.

### Bug Classification

This is a **correctness bug** in the V4L2 subdev frame interval
reporting. Userspace applications querying the frame interval would get
incorrect information when progressive mode is enabled. This could
cause:
- Video recording at wrong timestamps
- A/V synchronization issues
- Incorrect video playback speed
- Applications making wrong decisions about buffer management based on
  incorrect frame rates

### Scope and Risk Assessment

- **Lines changed**: ~5 lines added (including comments)
- **Files touched**: 1 file (drivers/media/i2c/adv7180.c)
- **Risk**: Very low. The change only affects the reported frame
  interval when `state->field == V4L2_FIELD_NONE`. It cannot affect
  interlaced mode operation at all.
- **Complexity**: Trivial multiplication by 2 of an existing value,
  gated by a clear condition.

### Stable Kernel Criteria Check

1. **Obviously correct and tested**: Yes - the math is straightforward
   and it was reviewed by a domain expert.
2. **Fixes a real bug**: Yes - incorrect frame interval reporting in
   progressive mode.
3. **Important issue**: Moderate - it causes incorrect metadata
   reporting that affects video capture applications. Not a crash or
   security issue, but a functional correctness bug that impacts real
   users of ADV7280-M hardware.
4. **Small and contained**: Yes - 5 lines in one file, one driver.
5. **No new features**: Correct - this fixes existing behavior, doesn't
   add new functionality.
6. **Clean application**: The change is self-contained and should apply
   cleanly to any kernel that has the `adv7180_get_frame_interval`
   function with the `v4l2_subdev_state` parameter.

### User Impact

Users of ADV7280-M video decoder chips with de-interlacing enabled would
get incorrect frame rate information from the V4L2 API. This is a real
hardware used in embedded video capture applications (the author is from
Topcon, a precision measurement/imaging company). The fix is important
for correct video processing pipelines.

### Risk vs Benefit

- **Benefit**: Correct frame interval reporting for ADV7280-M users in
  progressive mode
- **Risk**: Essentially zero - the change is gated by a specific
  condition and only affects metadata reporting, not actual video data
  flow

### Concerns

- The `state->field` and `V4L2_SUBDEV_FORMAT_ACTIVE` API patterns need
  to exist in the target stable tree. This is a relatively modern V4L2
  API pattern, so it should be checked which stable versions have the
  relevant code.
- No dependency on other commits - this is a standalone fix.

### Conclusion

This is a clean, small, obviously correct bug fix for a real issue
affecting hardware users. It meets all stable kernel criteria. The fix
is trivial, well-reviewed, and carries essentially no regression risk.

**YES**

 drivers/media/i2c/adv7180.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 378f4e6af12cb..5cbc973df684d 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -507,6 +507,13 @@ static int adv7180_get_frame_interval(struct v4l2_subdev *sd,
 		fi->interval.denominator = 25;
 	}
 
+	/*
+	 * If the de-interlacer is active, the chip produces full video frames
+	 * at the field rate.
+	 */
+	if (state->field == V4L2_FIELD_NONE)
+		fi->interval.denominator *= 2;
+
 	return 0;
 }
 
-- 
2.51.0


  parent reply	other threads:[~2026-02-14  1:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-14  0:58 [PATCH AUTOSEL 6.19-6.12] media: ipu6: Close firmware streams on streaming enable failure Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] media: chips-media: wave5: Fix conditional in start_streaming Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] media: mt9m114: Avoid a reset low spike during probe() Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.1] media: amphion: Clear last_buffer_dequeued flag for DEC_CMD_START Sasha Levin
2026-02-14  0:58 ` Sasha Levin [this message]
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.6] media: v4l2-async: Fix error handling on steps after finding a match Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.1] media: rkisp1: Fix filter mode register configuration Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] media: ipu6: Ensure stream_mutex is acquired when dealing with node list Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.12] media: mt9m114: Return -EPROBE_DEFER if no endpoint is found Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] media: uvcvideo: Create an ID namespace for streaming output terminals Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.12] media: ipu6: Always close firmware stream Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] media: qcom: camss: Do not enable cpas fast ahb clock for SM8550 VFE lite Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] media: solo6x10: Check for out of bounds chip_id Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] drm/amdgpu: Refactor amdgpu_gem_va_ioctl for Handling Last Fence Update and Timeline Management v4 Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.12] media: chips-media: wave5: Process ready frames when CMD_STOP sent to Encoder Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] media: pvrusb2: fix URB leak in pvr2_send_request_ex Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-5.10] media: dvb-core: dmxdevfilter must always flush bufs 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=20260214010245.3671907-13-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=hverkuil+cisco@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-media@vger.kernel.org \
    --cc=m.tretter@pengutronix.de \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=tschmelzer@topcon.com \
    /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