[PATCH AUTOSEL 7.0-6.12] media: renesas: vsp1: Initialize format on all pads

[Sasha Levin <sashal@kernel.org>]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

[ Upstream commit 133ac42af0a1b389e8b7b3dc7c1cc8c30ff162b6 ]

The state initialization function vsp1_entity_init_state() incorrectly
leaves the last entity pad out when initializing formats due to an off
by one error. Fix it.

Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> # HiHope RZ/G2M
Link: https://patch.msgid.link/20260318235907.831556-14-laurent.pinchart+renesas@ideasonboard.com
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.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:

Here is my complete analysis following all phases:

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: `media: renesas: vsp1:` (Renesas VSP1 video signal
  processor)
- **Action verb**: "Initialize" (implying incomplete initialization was
  the bug)
- **Summary**: Fix off-by-one error that leaves the last entity pad
  uninitialized during state init.

### Step 1.2: Tags
- **Tested-by**: Lad Prabhakar (Renesas employee, tested on HiHope
  RZ/G2M hardware)
- **Link**: `https://patch.msgid.link/20260318235907.831556-14-
  laurent.pinchart+renesas@ideasonboard.com` — patch 14 of a series
- **Signed-off-by**: Laurent Pinchart (author, VSP1 subsystem
  maintainer), Hans Verkuil (media subsystem maintainer)
- No Fixes: tag (expected for manual review candidates)
- No syzbot or CVE references

### Step 1.3: Commit Body
The message is concise: "The state initialization function
vsp1_entity_init_state() incorrectly leaves the last entity pad out when
initializing formats due to an off by one error. Fix it."

The author explicitly identifies the bug mechanism (off-by-one) and the
consequence (last pad format not initialized).

### Step 1.4: Hidden Bug Fix Detection
Not hidden — explicitly described as an off-by-one error fix.

Record: This is a straightforward initialization bug fix.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **Files**: 1 file changed (`vsp1_entity.c`)
- **Lines**: 1 line changed (`-` → `+`)
- **Function**: `vsp1_entity_init_state()`
- **Scope**: Single-file, single-character surgical fix

### Step 2.2: Code Flow Change
Before: `for (pad = 0; pad < subdev->entity.num_pads - 1; ++pad)` —
iterates pads 0 to num_pads-2, skipping the last pad.
After: `for (pad = 0; pad < subdev->entity.num_pads; ++pad)` — iterates
ALL pads including the last.

### Step 2.3: Bug Mechanism
**Category**: Initialization fix (off-by-one error)

For entities with N pads (N-1 sinks + 1 source), the old code only calls
`set_fmt` on pads 0 to N-2. For 2-pad entities, the source pad is
initialized through propagation in `vsp1_subdev_set_pad_format()`.
However, for BRU/BRS entities with custom `brx_set_format()`, only the
format CODE is propagated to the source pad — width and height remain
zero (uninitialized). This means the source pad of BRU/BRS entities had
0x0 dimensions.

### Step 2.4: Fix Quality
- Obviously correct: The comment says "Initialize all pad formats" but
  the loop skips one
- Minimal: Single character change
- No regression risk: Calling `set_fmt` on the source pad is safe — for
  most entities it returns the current format; for BRU/BRS it applies
  defaults
- No API changes

Record: Fix quality is excellent. Zero regression risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The buggy line was in commit `5755be5f15d9e6` (v6.8, renamed from
`init_cfg` to `init_state`), but the loop condition `num_pads - 1` was
copied from the original `vsp1_entity_init_cfg()`. Tracing back further
with pickaxe search, the pattern dates to commit `0efdf0f5eaaff` ("v4l:
vsp1: Implement and use the subdev pad::init_cfg configuration", v4.6
era, 2015). The off-by-one has been present for ~10 years.

### Step 3.2: Fixes Tag
No explicit Fixes tag. However, the bug trace shows:
- `0efdf0f5eaaff` (v4.6): introduced `vsp1_entity_init_cfg()` with this
  loop
- `5755be5f15d9e` (v6.8): renamed to `vsp1_entity_init_state()`,
  preserving the bug
- Both commits exist in all active stable trees

### Step 3.3: File History
Recent commits to the file are from Laurent Pinchart's series adding
color space support. The fix is standalone — it doesn't depend on any
other commits from the series.

### Step 3.4: Author
Laurent Pinchart is THE creator and maintainer of the VSP1 driver. He
has been the sole significant contributor to this subsystem. His fix
carries maximum authority.

### Step 3.5: Dependencies
The fix has no dependencies. The loop body, function signature, and pad
structures are unchanged from stable trees. The only variation across
trees is the function name (`vsp1_entity_init_cfg` in ≤6.6,
`vsp1_entity_init_state` in ≥6.8).

Record: Standalone fix, no prerequisites needed.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Patch Discussion
`b4 dig` could not directly match this commit (likely too new), but
found the related series via nearby commits. The series is Laurent
Pinchart's VSP1 color space and cleanup work. The Link message ID shows
this is patch 14 of a larger series.

### Step 4.2: Reviewers
Laurent Pinchart (VSP1 maintainer) authored it; Hans Verkuil (media
subsystem maintainer) signed off. Lad Prabhakar (Renesas) tested on
actual hardware.

### Step 4.3-4.5: Bug Report / Related Patches / Stable History
No separate bug report — the author discovered the off-by-one during
code review. No prior stable discussion found.

Record: Reviewed and merged through proper maintainer channels.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key Functions
`vsp1_entity_init_state()` modified.

### Step 5.2: Callers
- Called directly from `vsp1_entity_init()` (line 652) during entity
  initialization for ACTIVE format
- Registered as `.init_state` callback in `vsp1_entity_internal_ops` —
  called by V4L2 framework for TRY state initialization

### Step 5.3-5.4: Impact Surface
Every VSP1 entity goes through `vsp1_entity_init()` during probe.
Entities affected by the uninitialized source pad include:
- **BRU**: 5 sinks + 1 source (6 pads) — source pad width/height = 0
- **BRS**: 2 sinks + 1 source (3 pads) — source pad width/height = 0
- All 2-pad entities: Not affected (source pad initialized through
  propagation)

The BRU is a critical component in the display pipeline used by the
DRM/KMS driver for compositing on Renesas R-Car platforms.

Record: Bug affects BRU/BRS entities, which are part of the display
pipeline.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Buggy Code Existence
The buggy loop exists in ALL stable trees (5.15.y, 6.1.y, 6.6.y,
6.12.y). In trees ≤6.6, the function is named `vsp1_entity_init_cfg`; in
≥6.8, it's `vsp1_entity_init_state`.

### Step 6.2: Backport Complications
- For 6.12.y: Should apply cleanly (same function name and structure)
- For 6.6.y and earlier: Needs trivial adaptation (function name
  change), but the fix is the same single-character change

Record: Expected clean apply on 6.12.y; trivial rename needed for older
trees.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Criticality
**Subsystem**: drivers/media (V4L2/media platform driver for Renesas)
**Criticality**: IMPORTANT — Renesas R-Car is widely used in automotive
and embedded systems. The VSP1 is the display compositing hardware used
via DRM/KMS.

### Step 7.2: Activity
Active subsystem with ongoing improvements by the maintainer.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Users of Renesas R-Car SoC platforms (automotive, embedded).
Specifically those using display pipelines with BRU/BRS compositing.

### Step 8.2: Trigger Conditions
Triggered every time a BRU/BRS entity is initialized — this happens
during driver probe (every boot) and when TRY state is initialized.

### Step 8.3: Failure Mode
The source pad has 0x0 dimensions, which could lead to:
- Incorrect display pipeline configuration
- Potential division-by-zero in calculations using the source pad
  dimensions
- Incorrect format negotiation
- Severity: **MEDIUM-HIGH** (functional incorrectness in display
  pipeline)

### Step 8.4: Risk-Benefit
- **Benefit**: Fixes incorrect initialization for display pipeline
  entities on Renesas platforms. HIGH.
- **Risk**: One-character change, extremely low regression risk. Calling
  set_fmt on source pads is safe (returns current format or applies
  defaults). VERY LOW.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes a genuine off-by-one bug that leaves pad formats uninitialized
- One-line, obviously correct fix
- Author is the VSP1 subsystem maintainer (Laurent Pinchart)
- Tested on actual hardware by Renesas engineer
- Bug has existed for ~10 years across all stable trees
- Affects display pipeline initialization on Renesas R-Car
  (automotive/embedded)
- Merged through proper maintainer chain (Hans Verkuil)

**AGAINST backporting:**
- No reported user complaints or crash reports (discovered by code
  review)
- Impact may be mitigated by userspace reconfiguring pads before use

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **YES** — trivial one-line fix, tested
   on HiHope RZ/G2M
2. Fixes a real bug? **YES** — off-by-one leaving pad formats
   uninitialized
3. Important issue? **YES** — affects display pipeline configuration on
   Renesas SoCs
4. Small and contained? **YES** — single character change in one file
5. No new features or APIs? **YES** — no new functionality
6. Can apply to stable? **YES** — clean apply on 6.12.y, trivial rename
   for older

### Step 9.3: Exception Categories
Not applicable — this is a standard bug fix, not an exception category.

### Step 9.4: Decision
This is a textbook stable backport candidate: a one-line off-by-one fix
by the subsystem maintainer, tested on hardware, with no regression
risk.

---

## Verification

- [Phase 1] Parsed tags: found Tested-by from Renesas engineer, SOBs
  from subsystem and media maintainers
- [Phase 2] Diff analysis: single character change in loop condition,
  `num_pads - 1` → `num_pads`
- [Phase 3] git blame: buggy code traced to commit 5755be5f15d9e6 (v6.8,
  rename), originally from 0efdf0f5eaaff (v4.6, initial implementation)
- [Phase 3] git describe: original buggy commit c8663c8e15c95 first
  appeared in v4.12-rc1
- [Phase 3] git log -S: confirmed the `num_pads - 1` pattern existed in
  the original file path before directory reorganization
- [Phase 5] Grep for callers: `vsp1_entity_init_state` called from
  `vsp1_entity_init()` and as `.init_state` callback
- [Phase 5] Grep for all entity init calls: verified BRU (variable
  pads), BRS (3 pads), IIF (3 pads) as multi-pad entities; most others
  are 2-pad
- [Phase 5] Read `brx_set_format()`: confirmed it only propagates format
  CODE to source pad, NOT width/height — source pad left with 0x0
  dimensions
- [Phase 6] git log between stable versions: confirmed the buggy
  function exists in all stable trees (with name variation)
- [Phase 7] Author is Laurent Pinchart, confirmed as the sole major
  contributor and maintainer of VSP1 driver
- UNVERIFIED: Could not fetch lore.kernel.org discussion due to bot
  protection. Does not affect decision.

**YES**

 drivers/media/platform/renesas/vsp1/vsp1_entity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_entity.c b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
index a6680d531872a..e8e65eb19a7d8 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_entity.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_entity.c
@@ -386,7 +386,7 @@ static int vsp1_entity_init_state(struct v4l2_subdev *subdev,
 	unsigned int pad;
 
 	/* Initialize all pad formats with default values. */
-	for (pad = 0; pad < subdev->entity.num_pads - 1; ++pad) {
+	for (pad = 0; pad < subdev->entity.num_pads; ++pad) {
 		struct v4l2_subdev_format format = {
 			.pad = pad,
 			.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY
-- 
2.53.0