From: Martin Tůma martin.tuma@digiteqautomotive.com
[ Upstream commit 0750649b528ff18d1d68aecb45b34ec22d5ab778 ]
Compare the whole v4l2_bt_timings struct, not just the width/height when setting new timings. Timings with the same resolution and different pixelclock can now be properly set.
Signed-off-by: Martin Tůma martin.tuma@digiteqautomotive.com Signed-off-by: Hans Verkuil hverkuil+cisco@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes: Prevents a real functional bug where setting DV timings with the same width/height but different pixelclock (or other BT fields) was treated as “no change” and ignored. This blocked valid timing changes (e.g., framerate changes at the same resolution). - Code change: Replaces a narrow equality check on resolution with a full DV timings comparison: - New check: `v4l2_match_dv_timings(timings, &vindev->timings, 0, false)` in `drivers/media/pci/mgb4/mgb4_vin.c:613`. - Old behavior (implicit from the diff): only compared `width` and `height`, causing false “match” for differing pixelclock/porches/polarities/etc. - Correct behavior when busy: With the fix, if the queue is streaming and the requested timings differ in any BT field, `vidioc_s_dv_timings` returns `-EBUSY` instead of silently returning 0 while not applying the change (see `vb2_is_busy` branch right after the match check in `drivers/media/pci/mgb4/mgb4_vin.c:615`). - Scope and risk: Minimal and contained (one-line logic change in a single driver). No API/ABI change, no architectural impact, only affects `VIDIOC_S_DV_TIMINGS` behavior in the MGB4 capture driver. - Uses a proven helper: `v4l2_match_dv_timings` is the standard V4L2 helper that compares the full `v4l2_bt_timings` including width/height, interlaced, polarities, pixelclock (with tolerance), porches, vsync, flags, and interlaced-specific fields; see implementation at `drivers/media/v4l2-core/v4l2-dv-timings.c:267`. This pattern is used across other drivers. - User impact: Enables setting legitimate timings that share resolution but differ in pixelclock (and other BT parameters). Previously such requests were incorrectly treated as no-ops. - Stable criteria fit: - Important bugfix affecting real use (DV timings changes ignored). - Small, localized change with low regression risk. - No new features or interface changes. - Touches only a non-core driver (`drivers/media/pci/mgb4/mgb4_vin.c`). - Backport note: Apply to stable kernels that include the MGB4 driver; the helper `v4l2_match_dv_timings` is long-standing in V4L2 and does not introduce dependencies.
Overall, this is a low-risk, clear bug fix that improves correctness and user experience when changing DV timings; it should be backported.
drivers/media/pci/mgb4/mgb4_vin.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/media/pci/mgb4/mgb4_vin.c b/drivers/media/pci/mgb4/mgb4_vin.c index 989e93f67f75b..42c327bc50e10 100644 --- a/drivers/media/pci/mgb4/mgb4_vin.c +++ b/drivers/media/pci/mgb4/mgb4_vin.c @@ -610,8 +610,7 @@ static int vidioc_s_dv_timings(struct file *file, void *fh, timings->bt.height < video_timings_cap.bt.min_height || timings->bt.height > video_timings_cap.bt.max_height) return -EINVAL; - if (timings->bt.width == vindev->timings.bt.width && - timings->bt.height == vindev->timings.bt.height) + if (v4l2_match_dv_timings(timings, &vindev->timings, 0, false)) return 0; if (vb2_is_busy(&vindev->queue)) return -EBUSY;