* [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: fix CPER ring header parsing
[not found] <20260428104133.2858589-1-sashal@kernel.org>
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Pass min page size from SOC BB to dml2_1 plane config Sasha Levin
` (6 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Xiang Liu, Tao Zhou, Alex Deucher, Sasha Levin, christian.koenig,
Xinhui.Pan, airlied, daniel, amd-gfx, dri-devel, linux-kernel
From: Xiang Liu <xiang.liu@amd.com>
[ Upstream commit b8939bd764c9c8bf6488dc0d71d9c718c25d8cfc ]
amdgpu_cper_ring_get_ent_sz() parses CPER headers directly from the
circular ring buffer to determine the current entry size. When the ring
is full and the write pointer lands near the end of the buffer, the
header can wrap across the ring boundary.
The existing code treats the 4-byte CPER signature as a C string and
uses strcmp() on in-ring binary data, then reads record_length through a
direct struct pointer cast. Both assumptions are unsafe for wrapped
entries and can read past the end of the ring mapping.
Fix the parser by comparing the signature as raw bytes and by copying
the header into a local buffer before reading record_length, handling
wraparound explicitly in both cases. This avoids out-of-bounds reads in
amdgpu_cper_ring_get_ent_sz() when the CPER ring is full or the current
entry starts at the tail of the ring.
Signed-off-by: Xiang Liu <xiang.liu@amd.com>
Reviewed-by: Tao Zhou <tao.zhou1@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough context to make a comprehensive analysis. Let me
complete the structured investigation.
## Analysis
### PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: SUBJECT LINE PARSING**
- Subsystem prefix: `drm/amdgpu:`
- Action verb: "fix"
- Record: `[drm/amdgpu]` `[fix]` Fix CPER ring header parsing - handle
wrap-around and avoid OOB reads when entries straddle ring boundary.
**Step 1.2: COMMIT MESSAGE TAGS**
- No `Fixes:` tag (the Fixes target would be `4d614ce8ffd75 "drm/amdgpu:
add RAS CPER ring buffer"`)
- No `Reported-by:`, `Tested-by:`, `Link:` to bug report
- `Reviewed-by: Tao Zhou <tao.zhou1@amd.com>` — same Tao Zhou who
originally added the CPER ring buffer code (subsystem expert)
- `Signed-off-by:` chain: Xiang Liu (author) -> Alex Deucher (AMD GPU
maintainer)
- No `Cc: stable@vger.kernel.org`
- Record: Limited tags, but reviewed by subsystem expert and signed off
by maintainer.
**Step 1.3: COMMIT BODY**
- Bug description: `amdgpu_cper_ring_get_ent_sz()` parses CPER headers
directly from a circular ring buffer. When the ring is full and write
pointer lands near the end of the buffer, the header can wrap across
the ring boundary. The existing code uses `strcmp()` on in-ring binary
data (signature is 4-byte non-null-terminated) and reads
`record_length` through a direct struct pointer cast, which can read
past the end of the ring buffer mapping for wrapped entries.
- Failure mode: "out-of-bounds reads in `amdgpu_cper_ring_get_ent_sz()`
when the CPER ring is full or the current entry starts at the tail of
the ring."
- Root cause: Lack of wrap-around handling in ring header parsing.
- Record: Clear description of an out-of-bounds read bug in ring buffer
parsing logic.
**Step 1.4: HIDDEN BUG FIX DETECTION**
- This commit is explicitly a fix ("fix CPER ring header parsing").
- It addresses two issues: (a) using `strcmp()` on non-null-terminated
binary data, (b) struct pointer cast reading past ring end.
- Record: Not hidden - clearly a defensive fix for OOB reads.
### PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY**
- 1 file modified: `drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c`
- ~25 lines added, ~9 lines removed
- Functions modified: `amdgpu_cper_is_hdr()`,
`amdgpu_cper_ring_get_ent_sz()`
- Record: Single file, surgical fix to two static functions.
**Step 2.2: CODE FLOW CHANGE**
- Before: `chdr = (struct cper_hdr *)&(ring->ring[pos])` cast,
`strcmp(chdr->signature, "CPER")` - assumes linear reads beyond `pos`.
- After: Uses `memcpy()` with explicit bounds check for `(pos << 2) >=
ring->ring_size`, splits reads when wrapping the ring boundary, uses
`memcmp()` on bytes (no null-termination assumption). For
`record_length`, copies the header to a local `struct cper_hdr chdr`
first.
- Record: Changes from unsafe pointer cast/strcmp to bounded
memcpy/memcmp with wrap handling.
**Step 2.3: BUG MECHANISM**
- Category (d) Memory safety + (g) Logic correctness:
- OOB read: When `pos << 2` is near `ring->ring_size`, casting to
`struct cper_hdr *` and reading 128 bytes (size of struct) reads
past the allocated ring memory.
- Wrap-around: When CPER entries wrap the ring boundary, the old code
reads contiguous memory (which is past the buffer end) instead of
reading the wrapped portion from the start of the ring.
- The `strcmp()` on a 4-byte non-null-terminated `signature` field
happens to work in unwrapped cases because the next byte
(`revision`'s low byte for `CPER_HDR_REV_1=0x100`) is zero in
little-endian, but the wrap-around case is genuinely broken.
- Record: OOB read on heap allocation + incorrect handling of ring wrap-
around.
**Step 2.4: FIX QUALITY**
- Bounds checks before access; correct memcpy splitting at ring boundary
- Localizes the buffer (struct cper_hdr chdr on stack vs. pointer to
ring memory)
- Reuses `amdgpu_cper_is_hdr()` for the search loop (DRY)
- Risk: low - no locking changes, no API changes, surgical
- Record: Correct, minimal, well-contained.
### PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME**
- The buggy code was introduced in `4d614ce8ffd75 "drm/amdgpu: add RAS
CPER ring buffer"` (Jan 22, 2025)
- This commit is included in v6.15 (verified via `git tag --contains
4d614ce8ffd75`)
- Record: Buggy code introduced in v6.15 timeframe.
**Step 3.2: FIXES TARGET**
- No explicit Fixes tag, but the buggy code is clearly `4d614ce8ffd75`
(and subsequent additions in same series)
- Target exists in v6.15+ (mainline), v6.16, v6.17, v6.18 (LTS), v7.0
stable trees
- NOT in older LTS (5.10, 5.15, 6.1, 6.6, 6.12) - those don't have CPER
ring code
- Record: Bug exists in v6.15+ stable trees only.
**Step 3.3: FILE HISTORY**
- The CPER ring buffer infrastructure has been actively developed since
Jan 2025
- Multiple subsequent fixes: `d6f9bbce18762`, `8e0d1edb5c167` (the
latter has explicit `Cc: stable@vger.kernel.org`)
- No hard prerequisites identified for this specific patch
- Record: Standalone fix; no dependencies needed.
**Step 3.4: AUTHOR**
- Xiang Liu is a regular AMD contributor with many CPER-related commits
- Tao Zhou is the original author of the CPER ring buffer code (highly
knowledgeable about it)
- Alex Deucher is the AMD GPU maintainer
- Record: Strong subsystem expertise.
**Step 3.5: DEPENDENCIES**
- No prerequisites; the fix is self-contained
- Record: Self-contained, applies cleanly.
### PHASE 4: MAILING LIST RESEARCH
**Step 4.1: PATCH DISCUSSION**
- `b4 dig` found the original submission: `https://lore.kernel.org/all/2
0260409092403.572319-1-xiang.liu@amd.com/`
- Only one revision (v1) was sent
- Reviewer Tao Zhou suggested defining a `CPER_SIGNATURE_SZ` macro -
this was incorporated in the committed version
- No NAK or stability concerns raised
- No explicit `Cc: stable` request in the discussion
- Record: One revision; minor cosmetic feedback incorporated; no
concerns raised.
**Step 4.2: REVIEWERS**
- CC list: Hawking Zhang, Tao Zhou, amd-gfx mailing list
- Reviewed by Tao Zhou (the original author of the buggy CPER ring code)
- Record: Reviewed by the right subsystem experts.
**Step 4.3: BUG REPORT**
- No bug report referenced - appears to be developer-found via code
review/audit
- Record: No external bug report - found by AMD developers themselves.
**Step 4.4-4.5: RELATED PATCHES / STABLE HISTORY**
- Single-patch series; no related patches in series
- Earlier CPER fix `8e0d1edb5c167` had explicit `Cc: stable` - shows
pattern of CPER fixes being sent to stable
- Record: Consistent with other CPER fixes that went to stable.
### PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1-5.4: Functions and call sites**
- `amdgpu_cper_is_hdr()` - called by `amdgpu_cper_ring_write()` (line
516) and `amdgpu_cper_ring_get_ent_sz()` (after fix)
- `amdgpu_cper_ring_get_ent_sz()` - called by `amdgpu_cper_ring_write()`
(lines 488, 509)
- `amdgpu_cper_ring_write()` - called from `amdgpu_cper_generate_*()` (3
sites in amdgpu_cper.c) and `amdgpu_virt.c` (1 site for SR-IOV)
- Trigger path: AMD GPU error reporting (RAS/ACA) -> generate CPER entry
-> write to ring -> parse headers when ring is full
- Reachability: User triggered indirectly when GPU experiences error
events; CPER ring fills over time
- Record: Path is reachable on systems with RAS-enabled enterprise AMD
GPUs that experience errors.
**Step 5.5: Similar patterns**
- The fix uses the standard pattern of bounds-checking + memcpy for
reading from circular buffers
- Record: Standard defensive programming pattern.
### PHASE 6: CROSS-REFERENCING
**Step 6.1: Code in stable**
- The CPER ring code was introduced in v6.15 (commit `4d614ce8ffd75`)
- Buggy code present in: v6.15, v6.16, v6.17, v6.18 (LTS), v7.0
- NOT present in: v6.12 (LTS), v6.6 (LTS), v6.1 (LTS), v5.15 (LTS),
v5.10 (LTS)
- Record: Only newer stable trees affected.
**Step 6.2: Backport complications**
- Fix applies cleanly against current `linux-7.0.y` HEAD (verified via
`git diff HEAD..b8939bd764c9c`)
- Record: Clean apply on 7.0 stable; should also apply cleanly to
6.18.y, 6.17.y, 6.16.y.
**Step 6.3: Related fixes in stable**
- Other CPER fixes (e.g., `8e0d1edb5c167`) went to stable - this is
consistent treatment
- Record: Pattern of CPER fixes going to stable.
### PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1: Subsystem**
- `drivers/gpu/drm/amd/amdgpu/` - AMD GPU driver, RAS error reporting
subsystem
- Criticality: PERIPHERAL-to-IMPORTANT (specific hardware, but
datacenter relevance)
- Record: Affects users of AMD enterprise GPUs (MI series) with RAS
enabled.
**Step 7.2: Activity**
- CPER subsystem is actively developed (~16 commits since Jan 2025)
- Record: Actively maintained.
### PHASE 8: IMPACT/RISK
**Step 8.1: Affected users**
- AMD GPU users with RAS enabled (datacenter/enterprise GPUs primarily,
MI200/MI300 etc.)
- SR-IOV virtualized GPU environments also affected
- Record: Smaller but real user population.
**Step 8.2: Trigger conditions**
- Requires CPER ring to become full (many error events recorded)
- AND the CPER entry to start near the end of the ring buffer (wrap
condition)
- Cannot be triggered by unprivileged users directly
- Record: Realistic but not common trigger; happens on hardware
experiencing errors.
**Step 8.3: Failure mode**
- OOB read on heap allocation (KASAN-detectable)
- Could read garbage data leading to incorrect ring management
- In worst case: kernel oops if page after ring is unmapped (rare since
ring is page-aligned)
- More likely: misidentified headers causing wrong rptr advancement,
dropped CPER entries, or incorrect entry size calculation
- Severity: MEDIUM-HIGH (OOB read is memory safety; ring corruption
affects RAS data integrity)
- Record: Memory safety bug + correctness bug.
**Step 8.4: Risk-benefit**
- Benefit: Fixes real OOB read on affected systems; fixes incorrect wrap
handling
- Risk: Very low - small fix to two static functions, no API/lock
changes, reviewed by subsystem expert
- Record: Good benefit-to-risk ratio.
### PHASE 9: SYNTHESIS
**Step 9.1-9.3: Evidence**
- FOR: Real OOB read bug, real wrap-around logic bug, small contained
fix, reviewed by subsystem experts, applies cleanly, signed off by
maintainer
- AGAINST: No Cc:stable, narrow trigger condition, smaller user
population (enterprise GPU users only), code only in v6.15+ trees
- Stable rules: 1) obviously correct ✓ 2) fixes real bug ✓ 3) memory
safety / data integrity ✓ 4) small ✓ 5) no new features ✓ 6) applies
cleanly ✓
## Verification
- [Phase 1] Parsed tags: No Fixes:, no Cc:stable, Reviewed-by Tao Zhou,
Signed-off-by chain Xiang Liu -> Alex Deucher (verified via `git
show`)
- [Phase 1] Commit message describes OOB read in
`amdgpu_cper_ring_get_ent_sz()` for wrapped entries (verified)
- [Phase 2] Diff analysis: 25 added, 9 removed in single file
`amdgpu_cper.c`; modifies `amdgpu_cper_is_hdr()` and
`amdgpu_cper_ring_get_ent_sz()` (verified via `git show
b8939bd764c9c`)
- [Phase 2] Verified `struct cper_hdr` is 128 bytes with 4-byte non-
null-terminated signature followed by uint16_t revision (verified in
`drivers/gpu/drm/amd/include/amd_cper.h`)
- [Phase 2] Verified ring is allocated as GTT BO with size
`ring->ring_size + extra_bytes` (verified in
`drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c` line 381)
- [Phase 3] git log: buggy code introduced by `4d614ce8ffd75` (verified
- "drm/amdgpu: add RAS CPER ring buffer", Jan 22, 2025)
- [Phase 3] Tag containment: `4d614ce8ffd75` first appears in v6.15
(verified via `git tag --contains`)
- [Phase 3] Older LTS trees (5.10, 5.15, 6.1, 6.6, 6.12) do NOT contain
the buggy code (verified)
- [Phase 4] b4 dig found patch at `https://lore.kernel.org/all/202604090
92403.572319-1-xiang.liu@amd.com/` (verified)
- [Phase 4] b4 dig -a: only v1 of the patch (verified)
- [Phase 4] Mailing thread shows Tao Zhou's review with Reviewed-by;
suggested CPER_SIGNATURE_SZ macro was incorporated (verified by
reading `/tmp/cper_thread.mbox`)
- [Phase 4] No NAKs, no stability concerns raised (verified from mbox)
- [Phase 5] `amdgpu_cper_ring_write()` callers:
`amdgpu_cper_generate_ue_record`,
`amdgpu_cper_generate_bp_threshold_record`,
`amdgpu_cper_generate_ce_records`, plus SR-IOV path in `amdgpu_virt.c`
(verified via Grep)
- [Phase 6] Patch applies cleanly to current `stable/linux-7.0.y` HEAD
(verified via `git diff HEAD..b8939bd764c9c`)
- [Phase 6] Earlier CPER fix `8e0d1edb5c167` had explicit `Cc:
stable@vger.kernel.org`, indicating pattern of CPER fixes going to
stable (verified via `git show`)
- [Phase 8] Trigger requires ring full + entry near end of buffer;
affects RAS-enabled AMD enterprise GPU users (verified by reading code
paths and CPER ring init in `amdgpu_cper.c`)
- UNVERIFIED: The actual frequency of the trigger condition in
production - depends on workload and error rates; likely rare but not
theoretical.
- UNVERIFIED: Whether KASAN has reported this specific issue - no
syzbot/KASAN report referenced in commit message.
## Conclusion
This commit fixes a real out-of-bounds read bug and an incorrect ring
wrap-around handling in the AMD GPU CPER (Common Platform Error Record)
ring buffer parser. When the ring is full and a CPER entry's header
straddles the ring boundary, the previous code would:
1. Cast a pointer past the end of the ring buffer to `struct cper_hdr *`
and read 128 bytes (the struct size), causing OOB reads on the heap
allocation
2. Use `strcmp()` on non-null-terminated 4-byte binary data
3. Fail to handle the wrap-around case correctly, leading to incorrect
record_length reads and wrong ring pointer management
The fix is small (~25/9 lines), surgical (single file, two static
functions), reviewed by the original CPER ring author (Tao Zhou), and
signed off by the AMD GPU maintainer. It applies cleanly to current
stable trees that contain the CPER ring code (v6.15+). The bug affects
users with RAS-enabled AMD enterprise GPUs (MI series datacenter cards)
- a smaller but real user population where memory safety and CPER data
integrity matter for reliability.
The fix meets all stable kernel criteria: obviously correct, fixes a
real bug (memory safety + data integrity), small and contained, no new
features, applies cleanly. The pattern of similar CPER fixes (e.g.,
`8e0d1edb5c167`) being sent to stable supports this as a stable
candidate.
**YES**
drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c | 36 ++++++++++++++++++------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
index c72c345334d01..4e6e390854e6e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
@@ -32,6 +32,8 @@ static const guid_t BOOT = BOOT_TYPE;
static const guid_t CRASHDUMP = AMD_CRASHDUMP;
static const guid_t RUNTIME = AMD_GPU_NONSTANDARD_ERROR;
+#define CPER_SIGNATURE_SZ (sizeof(((struct cper_hdr *)0)->signature))
+
static void __inc_entry_length(struct cper_hdr *hdr, uint32_t size)
{
hdr->record_length += size;
@@ -425,23 +427,40 @@ int amdgpu_cper_generate_ce_records(struct amdgpu_device *adev,
static bool amdgpu_cper_is_hdr(struct amdgpu_ring *ring, u64 pos)
{
- struct cper_hdr *chdr;
+ char signature[CPER_SIGNATURE_SZ];
+
+ if ((pos << 2) >= ring->ring_size)
+ return false;
- chdr = (struct cper_hdr *)&(ring->ring[pos]);
- return strcmp(chdr->signature, "CPER") ? false : true;
+ if ((pos << 2) + CPER_SIGNATURE_SZ <= ring->ring_size) {
+ memcpy(signature, &ring->ring[pos], CPER_SIGNATURE_SZ);
+ } else {
+ u32 chunk = ring->ring_size - (pos << 2);
+
+ memcpy(signature, &ring->ring[pos], chunk);
+ memcpy(signature + chunk, ring->ring, CPER_SIGNATURE_SZ - chunk);
+ }
+
+ return !memcmp(signature, "CPER", CPER_SIGNATURE_SZ);
}
static u32 amdgpu_cper_ring_get_ent_sz(struct amdgpu_ring *ring, u64 pos)
{
- struct cper_hdr *chdr;
+ struct cper_hdr chdr;
u64 p;
u32 chunk, rec_len = 0;
- chdr = (struct cper_hdr *)&(ring->ring[pos]);
chunk = ring->ring_size - (pos << 2);
- if (!strcmp(chdr->signature, "CPER")) {
- rec_len = chdr->record_length;
+ if (amdgpu_cper_is_hdr(ring, pos)) {
+ if (chunk >= sizeof(chdr)) {
+ memcpy(&chdr, &ring->ring[pos], sizeof(chdr));
+ } else {
+ memcpy(&chdr, &ring->ring[pos], chunk);
+ memcpy((u8 *)&chdr + chunk, ring->ring, sizeof(chdr) - chunk);
+ }
+
+ rec_len = chdr.record_length;
goto calc;
}
@@ -450,8 +469,7 @@ static u32 amdgpu_cper_ring_get_ent_sz(struct amdgpu_ring *ring, u64 pos)
goto calc;
for (p = pos + 1; p <= ring->buf_mask; p++) {
- chdr = (struct cper_hdr *)&(ring->ring[p]);
- if (!strcmp(chdr->signature, "CPER")) {
+ if (amdgpu_cper_is_hdr(ring, p)) {
rec_len = (p - pos) << 2;
goto calc;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Pass min page size from SOC BB to dml2_1 plane config
[not found] <20260428104133.2858589-1-sashal@kernel.org>
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: fix CPER ring header parsing Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: drop userq fence driver refs out of fence process() Sasha Levin
` (5 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Nicholas Kazlauskas, Dillon Varone, Roman Li, Dan Wheeler,
Alex Deucher, Sasha Levin, harry.wentland, sunpeng.li,
Rodrigo.Siqueira, christian.koenig, Xinhui.Pan, airlied, daniel,
amd-gfx, dri-devel, linux-kernel
From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
[ Upstream commit 07ac59230d5fd603d56af2363dae80d3e973e4bc ]
[Why]
Like dml2_0 this isn't guaranteed to be constant for every ASIC.
This can cause corruption or underflow for linear surfaces due to a
wrong PTE_ROW_HEIGHT_LINEAR value if not correctly specified.
[How]
Like dml2_0 pass in the SOC bb into the plane configuration population
functions.
Set both GPUVM and HostVM page sizes in the overrides.
Reviewed-by: Dillon Varone <dillon.varone@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Signed-off-by: Roman Li <roman.li@amd.com>
Tested-by: Dan Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Based on my forensic investigation, here is my detailed analysis:
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem prefix: `drm/amd/display:`
- Action verb: "Pass" (descriptive of the fix; effectively a bug fix)
- Record: drm/amd/display, action verb "Pass" (corrective), summary:
ensures DML2.1 plane configuration uses SOC bounding box values for
min page size instead of hardcoded 256
**Step 1.2: Commit Message Tags**
- Reviewed-by: Dillon Varone <dillon.varone@amd.com>
- Signed-off-by: Nicholas Kazlauskas (author, AMD)
- Signed-off-by: Roman Li (AMD display submaintainer)
- Tested-by: Dan Wheeler <daniel.wheeler@amd.com> (AMD QA tester)
- Signed-off-by: Alex Deucher (AMD GPU maintainer)
- No Fixes:/Cc:stable tags (expected per instructions)
- Record: Strong AMD internal review chain - reviewed, tested by AMD QA,
signed by maintainers
**Step 1.3: Commit Body Analysis**
- Bug: `gpuvm_min_page_size_kbytes` is hardcoded to 256, but SOC-
provided values can differ per ASIC
- Symptom: "corruption or underflow for linear surfaces due to a wrong
PTE_ROW_HEIGHT_LINEAR value if not correctly specified"
- Mechanism: Wrong page size causes wrong PTE row height, which causes
incorrect PTE prefetching
- Author understands root cause and explicitly notes this mirrors the
dml2_0 fix
- Record: Hardware corruption/underflow on linear surfaces; explicit
reference to prior dml2_0 fix
**Step 1.4: Hidden Bug Fix Detection**
- "Pass min page size from SOC BB" is corrective phrasing
- Commit explicitly says "can cause corruption or underflow"
- Record: This IS a bug fix despite verb-only-language ("Pass")
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file: `drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_translation
_helper.c`
- 15 insertions, 6 deletions
- 3 functions modified: `populate_dml21_dummy_plane_cfg`,
`populate_dml21_plane_config_from_plane_state`,
`dml21_map_dc_state_into_dml_display_cfg`
- Record: Single-file surgical fix, very small scope
**Step 2.2: Code Flow Change**
- BEFORE: `plane->overrides.gpuvm_min_page_size_kbytes = 256;`
(hardcoded)
- AFTER: `plane->overrides.gpuvm_min_page_size_kbytes =
soc_bb->gpuvm_min_page_size_kbytes;` (from SOC bb)
- Also adds: `plane->overrides.hostvm_min_page_size_kbytes =
soc_bb->hostvm_min_page_size_kbytes;`
- Function signatures extended to accept `struct dml2_soc_bb *soc_bb`
parameter
- Caller updated to pass `&dml_ctx->v21.dml_init.soc_bb`
- Record: Replaces hardcoded values with SOC-provided values; added
missing hostvm setting
**Step 2.3: Bug Mechanism**
- Category: Logic/correctness fix (hardware programming)
- Root cause: hardcoded constant where ASIC-specific value should be
used
- Specific impact: Wrong gpuvm_min_page_size affects
PTE_ROW_HEIGHT_LINEAR HW register programming on DCN401 hardware
- Record: Hardware programming correctness bug; can cause display
corruption
**Step 2.4: Fix Quality**
- Obviously correct: just propagates existing SOC bb values
- Minimal/surgical: 21-line diff, no unrelated changes
- Regression risk: very low - just replaces hardcoded values with
structured access; for DCN401 default SOC bb, values are identical
(256/0)
- Record: High quality, low-risk fix
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
- Hardcoded `= 256` lines have been present since the dml21 directory
was first added
- The dml21 file itself was renamed from `dml2/dml21/` to
`dml2_0/dml21/` in commit `e6a8a000cfe6a` (v6.19)
- Original creation: commit `70839da636050` ("Add new DCN401 sources")
from April 2024, first appeared in v6.11
- Record: Buggy code present since v6.11
**Step 3.2: Fixes: Tag**
- No Fixes: tag, but the commit explicitly references "Like dml2_0"
referring to commit `31663521ede2e` ("Use gpuvm_min_page_size_kbytes
for DML2 surfaces", July 2024)
- The dml2_0 fix WAS selected for stable trees: backported to 6.10.y
(54877301a7551), 6.11.y (291c87fd3abe1), 6.12.y, 6.18.y, 6.19.y
- Record: Direct precedent for backporting this class of fix
**Step 3.3: Related Recent Changes**
- Adjacent commit `5721b5b9c9c79` (Mar 24, 2026): "Fix HostVMMinPageSize
unit mismatch in DML2.1" - related but independent (fixes core
calculation, not override population)
- Adjacent commit `5a89553231833` (Mar 24, 2026): DCN42 SOC bb
correction
- Record: Part of a series of DML2.1 hardening fixes; this commit is
self-contained
**Step 3.4: Author Context**
- Nicholas Kazlauskas: AMD display engineer, primary author of DML logic
- Roman Li: AMD display maintainer
- Alex Deucher: AMD GPU subsystem maintainer
- Record: Author has full subsystem authority
**Step 3.5: Dependencies**
- The override field `hostvm_min_page_size_kbytes` was added to the
`plane->overrides` struct in commit `76468055069ce` ("DML21
Reintegration"), first appearing in v6.16
- For stable trees < 6.16, the hostvm field doesn't exist in the
override struct → backport adjustment needed
- The gpuvm portion can apply to all stable trees with the dml21
directory
- Record: Partial dependency on field availability; gpuvm portion
universally applicable
## PHASE 4: MAILING LIST INVESTIGATION
**Step 4.1: b4 dig**
- `b4 dig -c 07ac59230d5fd`: returned "Could not find anything matching"
- typical for AMD display patches that go through the internal `amd-
staging-drm-next` tree before mainline (not posted directly to lkml)
- Record: Patch went through AMD internal pipeline; no public list
discussion to investigate
**Step 4.2: Reviewers**
- Verified through commit message: AMD internal review (Dillon Varone,
Roman Li, Alex Deucher all involved)
- Tested by AMD QA (Dan Wheeler)
**Step 4.3-4.5: External Research**
- No bug report links; no Reported-by tags
- No syzbot involvement
- The commit was developed proactively after dml2_0 fix to address
parallel bug
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1-5.4: Functions and Reachability**
- `populate_dml21_dummy_plane_cfg`: called when stream has no planes
(e.g., display blanked/initial state)
- `populate_dml21_plane_config_from_plane_state`: called for every plane
on every mode-set
- Caller: `dml21_map_dc_state_into_dml_display_cfg` invoked from
`dml21_validate`/`dml21_compute_subvp_state`
- Reachable from: every atomic commit / mode-set on DCN401 hardware
- Record: HIGHLY reachable - any display configuration change on DCN401
**Step 5.5: Similar Patterns**
- The same fix was already done for dml2_0
(`populate_dummy_dml_plane_cfg`,
`populate_dml_plane_cfg_from_plane_state`)
- DCN401 uses `using_dml21 = true` (verified in `dcn401_resource.c`), so
dml2_1 path is the active one for this hardware
- Record: Direct parallel to previously-fixed dml2_0 bug
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code Existence**
- 6.6.y: file does NOT exist (no DCN401 support, dml21 dir absent)
- 6.11.y - 6.18.y: file exists at
`drivers/gpu/drm/amd/display/dc/dml2/dml21/dml21_translation_helper.c`
- 6.19.y - 7.0.y: file exists at `drivers/gpu/drm/amd/display/dc/dml2_0/
dml21/dml21_translation_helper.c` (renamed)
- Record: Bug exists in 6.11.y onward; not applicable to 6.6.y and
earlier
**Step 6.2: Backport Difficulty**
- 7.0.y, 6.19.y: clean apply
- 6.18.y: needs path adjustment (dml2 vs dml2_0)
- 6.16.y - 6.17.y: needs path adjustment; both fields available
- 6.12.y, 6.15.y: needs path adjustment AND hostvm field doesn't exist
in override struct → drop the hostvm override line
- 6.11.y: similar to 6.12.y (needs adjustment)
- Record: Trivial path adjustment for older trees; hostvm portion may
need dropping for 6.15.y and earlier
**Step 6.3: Related Fixes Already in Stable**
- The dml2_0 equivalent IS already in stable from 6.10.y onward
- The dml2_1 specific fix is NOT yet in any stable tree
- Record: This commit fills a gap left by the prior dml2_0 fix
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1: Subsystem**
- `drivers/gpu/drm/amd/display/` - AMD DC display driver
- Affects: DCN401 hardware (RX 9000 / RDNA4 GPUs, gfx12.0.0/12.0.1)
- Criticality: IMPORTANT - affects users of new AMD GPUs
**Step 7.2: Activity**
- Highly active subsystem; frequent fixes flow to stable
- Record: Active; AMD regularly submits display fixes to stable
## PHASE 8: IMPACT AND RISK
**Step 8.1: Affected Users**
- DCN401 hardware users (AMD RX 9000 / RDNA 4)
- Triggered on every mode-set/atomic commit
- Record: Driver-specific (DCN401), but on every display config change
**Step 8.2: Trigger Conditions**
- Per-ASIC dependent: only triggers visible corruption when SOC bb has
non-default values
- For DCN401 default SOC bb, values are identical (gpuvm=256, hostvm=0),
so the fix is a no-op functionally
- For DCN42 (in master, not yet stable) and other future ASICs with
different values, this prevents corruption
- Record: Latent bug; impact varies per ASIC
**Step 8.3: Failure Mode Severity**
- Per commit message: "corruption or underflow for linear surfaces"
- HIGH severity: visible display corruption is user-visible
- Record: HIGH for affected ASICs
**Step 8.4: Risk-Benefit**
- Benefit: prevents a documented hardware programming bug; mirrors
approved dml2_0 fix
- Risk: very low - structural change replacing constant with same-value
lookup; for current DCN401 default, no behavior change
- Record: Favorable risk-benefit ratio
## PHASE 9: SYNTHESIS
**Evidence FOR backport:**
- Direct precedent: the dml2_0 equivalent was selected for stable
(6.10-6.19+)
- Small surgical fix (~15 lines)
- Low regression risk; for default DCN401 SOC bb the values are
identical
- Reviewed and tested by AMD
- Hardware corruption potential explicitly stated by author
- Affects DCN401 users on every mode-set
**Evidence AGAINST backport:**
- For older stable trees (<6.16), the hostvm override field doesn't
exist - needs backport adjustment
- For DCN401's default SOC bb, this is essentially a structural fix
without functional change in practice
- Bug is latent for default configurations on current hardware
**Stable Rules Checklist:**
1. Obviously correct and tested? YES (Reviewed-by, Tested-by AMD QA)
2. Real bug? YES (per commit message: corruption/underflow potential)
3. Important issue? YES (display corruption is HIGH severity)
4. Small and contained? YES (1 file, 21 lines)
5. No new features? YES (just propagates existing SOC bb values)
6. Applies cleanly? PARTIALLY - clean for 6.19+, needs path/feature
adjustments for older
This commit follows the established pattern: the dml2_0 equivalent fix
was an AUTOSEL stable backport, and this fixes the parallel bug in
dml2_1. While the impact on the current DCN401 default configuration is
minimal (values match), it is the correct structural fix that prevents
corruption on ASICs with different SOC bb values, including upcoming
DCN42 hardware. The fix is small, well-reviewed, and low-risk.
## Verification
- [Phase 1] Parsed tags: confirmed Reviewed-by Dillon Varone, Tested-by
Dan Wheeler, signed by Roman Li and Alex Deucher
- [Phase 2] Diff analysis: 15 insertions, 6 deletions in single file;
verified function signature changes via diff
- [Phase 3] git log on file: dml21 dir created in commit `70839da636050`
(April 2024); confirmed via `git tag --contains` it first appeared in
v6.11
- [Phase 3] git log/show `31663521ede2e`: confirmed dml2_0 equivalent
exists, was authored July 2024 by same author Nicholas Kazlauskas
- [Phase 3] Confirmed dml2_0 fix backported to stable: 54877301a7551
(6.10.y), 291c87fd3abe1 (6.11.y), 31663521ede2e (6.12.y, 6.18.y,
6.19.y)
- [Phase 3] git show `5721b5b9c9c79`: verified adjacent independent fix
for HostVMMinPageSize unit mismatch
- [Phase 4] b4 dig -c 07ac59230d5fd: returned "Could not find anything
matching" - typical for AMD internal pipeline
- [Phase 5] grep verified `using_dml21 = true` in dcn401_resource.c
(line 731), confirming dml2_1 path is active for DCN401
- [Phase 5] grep verified callers of populate functions trace back to
mode-set/atomic commit paths
- [Phase 6] git ls-tree on stable trees: confirmed file presence per
branch (6.6.y absent, 6.11+ present, renamed at 6.19)
- [Phase 6] git show on 6.12.y soc_parameter_types.h: confirmed
`hostvm_min_page_size_kbytes` exists in dml2_soc_bb but verified the
dml_top_display_cfg_types.h plane override only had
`gpuvm_min_page_size_kbytes` (not hostvm) until v6.16
- [Phase 6] git show on 6.18.y/6.19.y override struct: confirmed both
fields present
- [Phase 7] Verified subsystem activity via recent commit log
- [Phase 8] Verified default DCN4 SOC bb values:
`gpuvm_min_page_size_kbytes = 256`, `hostvm_min_page_size_kbytes = 0`
in `dcn4_soc_bb.h` - identical to previous hardcoding for default case
- [Phase 8] Verified `display_config` is memset to 0 before population
in `dml21_wrapper.c` (line 210), so hostvm starts at 0 effectively
even before fix
- UNVERIFIED: Could not find the patch on lore.kernel.org (b4 dig and
direct lore search both failed); typical for AMD internal pipeline
patches that don't get posted to public lists before merge
The fix matches a previously-approved stable backport pattern, is small
and surgical, addresses a documented hardware programming concern
(PTE_ROW_HEIGHT_LINEAR causing corruption/underflow), and has negligible
regression risk.
**YES**
.../dml2_0/dml21/dml21_translation_helper.c | 21 +++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_translation_helper.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_translation_helper.c
index bf5e7f4e04167..a64c0407ad515 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/dml21_translation_helper.c
@@ -381,7 +381,9 @@ static void populate_dml21_dummy_surface_cfg(struct dml2_surface_cfg *surface, c
surface->tiling = dml2_sw_64kb_2d;
}
-static void populate_dml21_dummy_plane_cfg(struct dml2_plane_parameters *plane, const struct dc_stream_state *stream)
+static void populate_dml21_dummy_plane_cfg(struct dml2_plane_parameters *plane,
+ const struct dc_stream_state *stream,
+ const struct dml2_soc_bb *soc_bb)
{
unsigned int width, height;
@@ -425,7 +427,8 @@ static void populate_dml21_dummy_plane_cfg(struct dml2_plane_parameters *plane,
plane->pixel_format = dml2_444_32;
plane->dynamic_meta_data.enable = false;
- plane->overrides.gpuvm_min_page_size_kbytes = 256;
+ plane->overrides.gpuvm_min_page_size_kbytes = soc_bb->gpuvm_min_page_size_kbytes;
+ plane->overrides.hostvm_min_page_size_kbytes = soc_bb->hostvm_min_page_size_kbytes;
}
static void populate_dml21_surface_config_from_plane_state(
@@ -495,7 +498,7 @@ static const struct scaler_data *get_scaler_data_for_plane(
static void populate_dml21_plane_config_from_plane_state(struct dml2_context *dml_ctx,
struct dml2_plane_parameters *plane, const struct dc_plane_state *plane_state,
- const struct dc_state *context, unsigned int stream_index)
+ const struct dc_state *context, unsigned int stream_index, const struct dml2_soc_bb *soc_bb)
{
const struct scaler_data *scaler_data = get_scaler_data_for_plane(dml_ctx, plane_state, context);
struct dc_stream_state *stream = context->streams[stream_index];
@@ -631,7 +634,8 @@ static void populate_dml21_plane_config_from_plane_state(struct dml2_context *dm
plane->composition.rotation_angle = (enum dml2_rotation_angle) plane_state->rotation;
plane->stream_index = stream_index;
- plane->overrides.gpuvm_min_page_size_kbytes = 256;
+ plane->overrides.gpuvm_min_page_size_kbytes = soc_bb->gpuvm_min_page_size_kbytes;
+ plane->overrides.hostvm_min_page_size_kbytes = soc_bb->hostvm_min_page_size_kbytes;
plane->immediate_flip = plane_state->flip_immediate;
@@ -765,7 +769,9 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
if (context->stream_status[stream_index].plane_count == 0) {
disp_cfg_plane_location = dml_dispcfg->num_planes++;
populate_dml21_dummy_surface_cfg(&dml_dispcfg->plane_descriptors[disp_cfg_plane_location].surface, context->streams[stream_index]);
- populate_dml21_dummy_plane_cfg(&dml_dispcfg->plane_descriptors[disp_cfg_plane_location], context->streams[stream_index]);
+ populate_dml21_dummy_plane_cfg(
+ &dml_dispcfg->plane_descriptors[disp_cfg_plane_location],
+ context->streams[stream_index], &dml_ctx->v21.dml_init.soc_bb);
dml_dispcfg->plane_descriptors[disp_cfg_plane_location].stream_index = disp_cfg_stream_location;
} else {
for (plane_index = 0; plane_index < context->stream_status[stream_index].plane_count; plane_index++) {
@@ -777,7 +783,10 @@ bool dml21_map_dc_state_into_dml_display_cfg(const struct dc *in_dc, struct dc_s
ASSERT(disp_cfg_plane_location >= 0 && disp_cfg_plane_location < __DML2_WRAPPER_MAX_STREAMS_PLANES__);
populate_dml21_surface_config_from_plane_state(in_dc, &dml_dispcfg->plane_descriptors[disp_cfg_plane_location].surface, context->stream_status[stream_index].plane_states[plane_index]);
- populate_dml21_plane_config_from_plane_state(dml_ctx, &dml_dispcfg->plane_descriptors[disp_cfg_plane_location], context->stream_status[stream_index].plane_states[plane_index], context, stream_index);
+ populate_dml21_plane_config_from_plane_state(
+ dml_ctx, &dml_dispcfg->plane_descriptors[disp_cfg_plane_location],
+ context->stream_status[stream_index].plane_states[plane_index],
+ context, stream_index, &dml_ctx->v21.dml_init.soc_bb);
dml_dispcfg->plane_descriptors[disp_cfg_plane_location].stream_index = disp_cfg_stream_location;
if (dml21_wrapper_get_plane_id(context, context->streams[stream_index]->stream_id, context->stream_status[stream_index].plane_states[plane_index], &dml_ctx->v21.dml_to_dc_pipe_mapping.disp_cfg_to_plane_id[disp_cfg_plane_location]))
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: drop userq fence driver refs out of fence process()
[not found] <20260428104133.2858589-1-sashal@kernel.org>
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: fix CPER ring header parsing Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Pass min page size from SOC BB to dml2_1 plane config Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] fbdev: savage: fix probe-path EDID cleanup leaks Sasha Levin
` (4 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Prike Liang, Alex Deucher, Christian König, Sasha Levin,
Xinhui.Pan, airlied, daniel, amd-gfx, dri-devel, linux-kernel
From: Prike Liang <Prike.Liang@amd.com>
[ Upstream commit dd88d42d9ca0dd7a4ed327dd33f6ead76cedf726 ]
amdgpu_userq_wait_ioctl() takes extra references on waited-on fence
drivers and stores them in waitq->fence_drv_xa. When a new userq fence is
created, those references are transferred into userq_fence->fence_drv_array
so they can be released when the fence completes.
However, those inherited references are currently only dropped from
amdgpu_userq_fence_driver_process(). If a fence never reaches that path,
such as it is already signaled when created, so we need to explicitly release
those fences in that case.
v2: use a list(list_cut_before) for managing the signal userq driver fences.(Christian)
Link: https://patchwork.freedesktop.org/patch/718078/?series=164763&rev=2
v3: Doesn't cache the userq first unsignaled fence and use the cut before list
head directly.(Christian)
Cc: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Prike Liang <Prike.Liang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me write the final analysis.
## Analysis
### PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1:** Subsystem: `drm/amdgpu`. Action verb: "drop"
(release/cleanup). One-line summary: drop userq fence driver references
out of the fence-process path so they get released even when a fence is
already signaled at creation.
Record: drm/amdgpu drop - releases fence driver refs on the signaled-at-
create path.
**Step 1.2:** Tags found:
- `Cc: Alex Deucher` - co-author/maintainer notification
- `Signed-off-by: Prike Liang` - author at AMD
- `Reviewed-by: Christian König` - AMD subsystem maintainer
- `Signed-off-by: Alex Deucher` - applied via Alex's tree
- `Link:
https://patchwork.freedesktop.org/patch/718078/?series=164763&rev=2` -
v2 reference
- v3 noted, no Cc: stable, no Fixes:, no Reported-by
Record: Reviewed and signed by maintainers; no syzbot/stable tags;
passed through 3 review iterations.
**Step 1.3:** Bug description: `amdgpu_userq_wait_ioctl()` takes
references on each waited-on fence driver and stores them in
`waitq->fence_drv_xa`. When a new fence is later created via
`amdgpu_userq_fence_create()`, those references are transferred into
`userq_fence->fence_drv_array`. The releases of those refs happen
exclusively in `amdgpu_userq_fence_driver_process()`. If the fence is
already signaled at creation time (HW already advanced past wptr), the
fence is never linked into `fence_drv->fences` and therefore never goes
through `amdgpu_userq_fence_driver_process()`, so the inherited
`fence_drv` references are leaked.
**Step 1.4:** This is clearly described as a leak fix; not disguised as
cleanup.
### PHASE 2: DIFF ANALYSIS
**Step 2.1:** Single file:
`drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c`, +33/-14. Functions
changed: new helper `amdgpu_userq_fence_put_fence_drv_array()`, modifies
`amdgpu_userq_fence_driver_process()` and `amdgpu_userq_fence_create()`.
Surgical single-file fix.
**Step 2.2:** Three change clusters:
1. New helper `amdgpu_userq_fence_put_fence_drv_array()` to put each
entry of the inherited fence_drv array.
2. `amdgpu_userq_fence_driver_process()` reworked to: walk under
spinlock to find the boundary, `list_cut_before()` to splice signaled
entries to a local `to_be_signaled` list, drop the spinlock, then
signal & put refs outside the lock. This avoids dropping `fence_drv`
refs (which can call destroy → take various locks) while holding
`fence_list_lock`.
3. `amdgpu_userq_fence_create()`: when the fence is already signaled at
creation, set a `signaled = true` flag and call the new helper after
releasing the spinlock to drop the inherited refs.
**Step 2.3:** Bug category: (b) reference counting / resource leak fix.
Specific mechanism: The inherited `fence_drv` references in
`userq_fence->fence_drv_array` were only released in the fence-list
processing path (signal+remove). When the fence was already signaled at
creation, the inherited refs leaked. Each leaked ref pins an
`amdgpu_userq_fence_driver` (which holds GPU memory via seq64). Plus a
structural improvement: putting refs outside `fence_list_lock` is needed
because the put can chain into destroy callbacks.
**Step 2.4:** Fix is logically correct (verified `list_cut_before`
semantics: when iterator points at head after the loop completes,
`cut_before(head)` moves all entries; when iterator is the first non-
signaled entry, `cut_before(entry)` moves correct prefix; empty list is
no-op). Minor risk: changes locking discipline in
`fence_driver_process()` - now releases & signals outside the lock. This
is safer wrt deadlock but is a behavioral change that could expose new
races if any caller assumes the function holds the lock through signal.
### PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1:** The buggy mechanism (`fence_drv_array` transfer + only-
release-in-process) was introduced in `e7cf21fbb2773` (Oct 2024) and the
userq feature itself in `a292fdecd7283` (Oct 2024), both first appearing
in v6.16. Bug present in all v6.16+ kernels.
**Step 3.2:** No `Fixes:` tag. Underlying buggy code is in v6.16,
present in stable trees 6.18.y and 7.0.y (6.16/6.17 are EOL).
**Step 3.3:** Many related fixes recently:
- `8e051e38a8d45 drm/amdgpu/userq: Fix fence reference leak on queue
teardown v2` — already in 6.18.y/7.0.y stable
- `48c33af0b62d8 drm/amdgpu: make userq fence_drv drop explicit in queue
destroy` — Mar 2026, NOT in stable
- `34f31fe40f3a1 drm/amdgpu: rework userq fence driver alloc/destroy` —
Mar 2026, NOT in stable
- `a1371d9f0e611 drm/amdgpu: rework amdgpu_userq_wait_ioctl v4` — 582
lines, NOT in stable
20 commits to this file have landed in master since the 7.0.y branch
point.
**Step 3.4:** Author Prike Liang is an AMD engineer with multiple recent
userq commits. Reviewer Christian König is the dma-fence/amdgpu
maintainer.
**Step 3.5:** The diff context shows references to `userq->last_fence =
NULL;` and a comment "Drop the queue's ownership reference to fence_drv
explicitly" that come from earlier reworks NOT in stable, but those are
*context lines* only - the actual hunks don't depend on them.
### PHASE 4: MAILING LIST RESEARCH
**Step 4.1:** `b4 dig -c dd88d42d9ca0d` could not find the patch on lore
(likely hosted on amd-gfx archives at lists.freedesktop.org rather than
indexed on lore). The commit message points to patchwork.freedesktop.org
(Anubis-protected from automated fetches). Web searches didn't surface
explicit stable nominations or NAKs for v3.
**Step 4.2/4.3/4.4/4.5:** Could not directly fetch the discussion due to
access restrictions. The commit went through 3 revisions with explicit
review feedback from Christian König incorporated each iteration.
### PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1:** Functions: `amdgpu_userq_fence_driver_process`,
`amdgpu_userq_fence_create`, new
`amdgpu_userq_fence_put_fence_drv_array`.
**Step 5.2:** Callers:
- `amdgpu_userq_fence_driver_process` — called from interrupt handlers
(`gfx_v11_0_eop_irq`, `gfx_v12_0_eop_irq`) and from
`amdgpu_userq_fence_driver_force_completion` (process context). Hot
path.
- `amdgpu_userq_fence_create` — called from `amdgpu_userq_signal_ioctl`
(userspace ioctl).
**Step 5.3:** The functions interact with `dma_fence`, the per-userq
fence list, and the per-userq `fence_drv_xa` xarray.
**Step 5.4:** Reachable from userspace via
`DRM_IOCTL_AMDGPU_USERQ_SIGNAL` and `DRM_IOCTL_AMDGPU_USERQ_WAIT`
ioctls. Any application using user-mode queues on RDNA3+/Navi3X+ AMD
GPUs can hit it.
**Step 5.5:** The leak pattern — only releasing inherited refs in one
specific path — is unique to this code; no sibling pattern needs the
same fix.
### PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
**Step 6.1:** Buggy code in 6.18.y, 6.19.y, 7.0.y. Not in 6.12.y or
earlier (userq feature didn't exist).
**Step 6.2:** Verified the patch applies cleanly with a 4-line offset to
both `stable/linux-6.18.y` and `stable/linux-7.0.y`:
```text
Hunk #1 succeeded at 151 (offset 6 lines).
Hunk #2 succeeded at 174 (offset 6 lines).
Hunk #3 succeeded at 256 (offset 14 lines).
Hunk #4 succeeded at 303-304 (offset 14-15 lines).
```
**Step 6.3:** No earlier/different fix for this exact leak in stable.
Related leak fix `8e051e38a8d45` (last_fence leak on teardown) is
already in stable.
### PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1:** drm/amdgpu, IMPORTANT level - widely deployed driver, but
the userq feature is gated to specific newer GPUs.
**Step 7.2:** Highly active subsystem; userq is in heavy flux (over 20
commits to this file since the 7.0 branch point).
### PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1:** Affected: users of AMD RDNA3+ GPUs (Navi3X family and
newer) using user-mode queues - this includes Mesa with the new UMD
path. CONFIG_DRM_AMDGPU_NAVI3X_USERQ was removed in v6.16, so it's
unconditionally available.
**Step 8.2:** Trigger conditions:
- App uses userq wait+signal ioctl pattern (typical of Mesa with UMD)
- HW completes the new fence's seqno before the create path checks
`dma_fence_is_signaled()`
- This is realistic for fast HW - the wptr is read, then a fence is
constructed, and HW can advance during that window.
- Each occurrence leaks one or more fence_drv references (depending on
how many fences the wait-on set had).
**Step 8.3:** Failure mode: kernel + GPU memory leak. Not a crash, not a
security issue. Severity MEDIUM — accumulates over long-running graphics
workloads. Each leaked `fence_drv` retains:
- The fence_drv struct itself
- A seq64 GPU memory allocation
- An entry in `adev->userq_xa`
**Step 8.4:** Benefit: MEDIUM (real leak in user-reachable path on
common workloads for this hardware). Risk: MEDIUM-LOW — 47-line change
but well-contained, reviewed through 3 iterations, applies cleanly. The
locking restructure also fixes a latent concern (dropping refs inside
the spinlock could chain into destroy callbacks).
### PHASE 9: FINAL SYNTHESIS
**Step 9.1:** Evidence FOR: real reachable memory leak (kernel + GPU
memory), reviewed by subsystem maintainer through v1/v2/v3, applies
cleanly to active stable trees containing the bug, sibling fix
`8e051e38a8d45` was backported, the affected feature ships
unconditionally in 6.18+. Evidence AGAINST: no `Fixes:` or `Cc: stable`
tag, structural change rather than minimal one-liner, userq feature
still in heavy flux.
**Step 9.2:** Stable rules:
1. Obviously correct/tested — Yes, reviewed by maintainer, multi-version
2. Real bug — Yes, reference/memory leak
3. Important — Medium: accumulating leak, not crash
4. Small — Borderline: 47 lines, single file, three logical changes
5. No new features — Correct
6. Apply cleanly — Yes, verified for 6.18 and 7.0
**Step 9.3:** No exception category needed - this is a straightforward
bug fix.
**Step 9.4:** Decision: The fix targets a real leak in user-reachable
code (userq signal/wait ioctl on AMD GPUs), is properly reviewed,
applies cleanly to active stable trees, and follows the same pattern as
previously-backported sibling fixes. The structural rework is necessary
to drop refs safely outside the spinlock. The userq subsystem is
volatile, but this change is self-contained and addresses an unambiguous
defect. Tilts toward backport-worthy.
### Verification
- [Phase 1] Parsed tags: Reviewed-by Christian König (subsystem
maintainer), no Cc:stable, no Fixes:, v1→v3 evolution noted in commit
message.
- [Phase 2] Diff inspected with `git show dd88d42d9ca0d`: 4 hunks, 33
added / 14 removed, single file. Manually traced `list_cut_before`
semantics including empty list and full-iterate-without-break corner
cases - logic correct.
- [Phase 3] `git tag --contains
a292fdecd72834b3bec380baa5db1e69e7f70679` — userq feature first in
v6.16. `git tag --contains e7cf21fbb2773` — fence_drv_array transfer
mechanism first in v6.16.
- [Phase 3] `git log --oneline stable/linux-7.0.y..origin/master --
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c` shows 20 commits not
in 7.0.y stable; surveyed 8e051e38a8d45 (sibling backported fix),
48c33af0b62d8, 34f31fe40f3a1 (not yet in stable, but our patch's hunks
don't depend on them).
- [Phase 4] `b4 dig -c dd88d42d9ca0d` - no lore match (commit was on
amd-gfx fdo list, points to patchwork). Web searches confirmed there's
a v2 and v3 with Christian's review feedback.
- [Phase 5] Verified callers via Grep on the file:
`amdgpu_userq_fence_driver_process` is called from
`gfx_v11_0_eop_irq`/`gfx_v12_0_eop_irq` (irq) and
`amdgpu_userq_fence_driver_force_completion`.
`amdgpu_userq_fence_create` only from `amdgpu_userq_signal_ioctl`.
- [Phase 6] Read content of file in `stable/linux-7.0.y` and
`stable/linux-6.18.y`: same vulnerable pattern
(`fence_drv_array_count` only released in `fence_driver_process`).
- [Phase 6] `git apply --check` against stable 7.0.y and 6.18.y file
contents - applies cleanly with offsets 6/6/14/14-15.
- [Phase 8] Failure mode: persistent memory leak (kernel struct + GPU
seq64 + userq_xa entry) per occurrence. Not crash; not security.
- UNVERIFIED: Could not directly fetch the original lore/freedesktop
discussion due to Anubis bot protection; relied on patchwork link in
commit message and prior commits' patterns.
- UNVERIFIED: Could not run the patched code; correctness of the rework
was reasoned from kernel `list_cut_before` semantics in source.
The fix addresses a real, user-reachable, ongoing GPU+kernel memory leak
with proper review, applies cleanly to active stable trees with the bug,
and matches the pattern of previously-backported sibling fixes in the
same file.
**YES**
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 47 +++++++++++++------
1 file changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 5239b06b9ab03..147d3cbab7a88 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -151,13 +151,22 @@ amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq)
amdgpu_userq_fence_driver_put(userq->fence_drv);
}
+static void
+amdgpu_userq_fence_put_fence_drv_array(struct amdgpu_userq_fence *userq_fence)
+{
+ unsigned long i;
+ for (i = 0; i < userq_fence->fence_drv_array_count; i++)
+ amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
+ userq_fence->fence_drv_array_count = 0;
+}
+
void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv)
{
struct amdgpu_userq_fence *userq_fence, *tmp;
+ LIST_HEAD(to_be_signaled);
struct dma_fence *fence;
unsigned long flags;
u64 rptr;
- int i;
if (!fence_drv)
return;
@@ -165,21 +174,26 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
spin_lock_irqsave(&fence_drv->fence_list_lock, flags);
rptr = amdgpu_userq_fence_read(fence_drv);
- list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences, link) {
- fence = &userq_fence->base;
-
- if (rptr < fence->seqno)
+ list_for_each_entry(userq_fence, &fence_drv->fences, link) {
+ if (rptr < userq_fence->base.seqno)
break;
+ }
- dma_fence_signal(fence);
-
- for (i = 0; i < userq_fence->fence_drv_array_count; i++)
- amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
+ list_cut_before(&to_be_signaled, &fence_drv->fences,
+ &userq_fence->link);
+ spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
- list_del(&userq_fence->link);
+ list_for_each_entry_safe(userq_fence, tmp, &to_be_signaled, link) {
+ fence = &userq_fence->base;
+ list_del_init(&userq_fence->link);
+ dma_fence_signal(fence);
+ /* Drop fence_drv_array outside fence_list_lock
+ * to avoid the recursion lock.
+ */
+ amdgpu_userq_fence_put_fence_drv_array(userq_fence);
dma_fence_put(fence);
}
- spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
+
}
void amdgpu_userq_fence_driver_destroy(struct kref *ref)
@@ -242,6 +256,7 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
struct amdgpu_userq_fence_driver *fence_drv;
struct dma_fence *fence;
unsigned long flags;
+ bool signaled = false;
fence_drv = userq->fence_drv;
if (!fence_drv)
@@ -288,13 +303,17 @@ static int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
/* Check if hardware has already processed the job */
spin_lock_irqsave(&fence_drv->fence_list_lock, flags);
- if (!dma_fence_is_signaled(fence))
+ if (!dma_fence_is_signaled(fence)) {
list_add_tail(&userq_fence->link, &fence_drv->fences);
- else
+ } else {
+ signaled = true;
dma_fence_put(fence);
-
+ }
spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
+ if (signaled)
+ amdgpu_userq_fence_put_fence_drv_array(userq_fence);
+
*f = fence;
return 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0-5.10] fbdev: savage: fix probe-path EDID cleanup leaks
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: drop userq fence driver refs out of fence process() Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Fix HostVMMinPageSize unit mismatch in DML2.1 Sasha Levin
` (3 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Yuho Choi, Myeonghun Pak, Ijae Kim, Taegyu Kim, Helge Deller,
Sasha Levin, adaplas, linux-fbdev, dri-devel, linux-kernel
From: Yuho Choi <dbgh9129@gmail.com>
[ Upstream commit 9b8a9a3a6f57edd02b7c8db14a316e6fab7fa772 ]
When CONFIG_FB_SAVAGE_I2C is enabled, savagefb_probe() can build both an
EDID-derived monspecs.modedb and a modelist from it before later failing.
The normal success path frees monspecs.modedb after the initial mode selection,
but the probe error path only deletes the I2C busses and misses the
EDID-derived allocations.
Free both the modelist and monspecs.modedb on the failed: unwind path.
Co-developed-by: Myeonghun Pak <mhun512@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Co-developed-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Taegyu Kim <tmk5904@psu.edu>
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: subsystem `fbdev: savage`; action verb `fix`; claimed
intent is to free EDID-derived probe-path allocations on
`savagefb_probe()` failure.
Step 1.2 Record: tags present: `Co-developed-by` Myeonghun Pak, Ijae
Kim, Taegyu Kim; `Signed-off-by` Myeonghun Pak, Ijae Kim, Taegyu Kim,
Yuho Choi, Helge Deller. No `Fixes:`, `Reported-by:`, `Tested-by:`,
`Reviewed-by:`, `Acked-by:`, `Link:`, or `Cc: stable@vger.kernel.org`
tag in the supplied message.
Step 1.3 Record: the body describes a real resource leak when
`CONFIG_FB_SAVAGE_I2C=y`, EDID parsing creates `info->monspecs.modedb`
and modelist entries, and later probe failure reaches `failed:` without
freeing those allocations. Symptom is leaked kernel memory on failed
probe. No explicit affected kernel versions or user report are provided.
Step 1.4 Record: not hidden; this is explicitly a probe error-path
cleanup leak fix.
## Phase 2: Diff Analysis
Step 2.1 Record: one file changed,
`drivers/video/fbdev/savage/savagefb_driver.c`; 2 lines added, 0
removed; function modified: `savagefb_probe()`; scope is a single-file
surgical error-path fix.
Step 2.2 Record: before, `failed:` under `CONFIG_FB_SAVAGE_I2C` only
deleted I2C busses. After, it also calls
`fb_destroy_modelist(&info->modelist)` and
`fb_destroy_modedb(info->monspecs.modedb)`. This affects probe unwind
paths after EDID/modelist setup.
Step 2.3 Record: bug category is resource leak. Verified allocation
sources: `fb_edid_to_monspecs()` stores `specs->modedb =
fb_create_modedb(...)`; `fb_create_modedb()` allocates with
`kzalloc_objs()`/`kmalloc_objs()`; `fb_videomode_to_modelist()` calls
`fb_add_videomode()`, which allocates `struct fb_modelist`. Verified
cleanup helpers free those objects.
Step 2.4 Record: fix quality is good: minimal, uses existing fbdev
cleanup APIs, no new feature/API. Regression risk is very low.
`fb_destroy_modedb(NULL)` is just `kfree(NULL)`, and
`fb_destroy_modelist()` safely iterates an initialized empty list.
## Phase 3: Git History Investigation
Step 3.1 Record: `git blame` shows the EDID/modelist setup and missing
`failed:` cleanup originate from very old code, much of it from the
initial imported history; the local EDID pointer handling was adjusted
by `0f8a1cae923670` in v5.18-rc1, but the leak pattern existed before
that with `par->edid`.
Step 3.2 Record: no `Fixes:` tag is present, so no target commit to
follow.
Step 3.3 Record: recent file history includes related probe fixes:
`e8d35898a78e3` fixed a savage probe leak in 2020, `04e5eac8f3ab`
handled zero pixclock, and `6ad959b6703e` fixed error handling for
`savagefb_check_var()`. No prerequisite was found for this cleanup,
because the failed label and cleanup helpers exist independently.
Step 3.4 Record: local history has no commits by Yuho Choi under
`drivers/video/fbdev`; Helge Deller signed off the supplied commit and
is verified in `MAINTAINERS` as framebuffer layer maintainer. The S3
Savage driver entry lists Antonino Daplas as maintainer.
Step 3.5 Record: dependency risk is low. The patch only uses
`fb_destroy_modelist()` and `fb_destroy_modedb()`, both verified present
in v5.15, v6.1, and v6.6 tags.
## Phase 4: Mailing List And External Research
Step 4.1 Record: no local commit hash was found with `git log --grep`,
so `b4 dig -c <hash>` could not be performed on a real commit object.
Attempts to use `b4 dig` with the subject failed: “Cannot find a commit
matching ...”. Lore `WebFetch` searches were blocked by Anubis; web
search found no exact subject match.
Step 4.2 Record: `b4 dig -w` could not identify recipients for the same
reason: no commit object found.
Step 4.3 Record: no `Link:` or `Reported-by:` tags were supplied; no
external bug report was verified.
Step 4.4 Record: no patch series context was verified. Local git history
suggests this is standalone.
Step 4.5 Record: stable-specific lore search could not be verified
because lore fetch was blocked; web search found no exact stable
discussion.
## Phase 5: Code Semantic Analysis
Step 5.1 Record: modified function: `savagefb_probe()`.
Step 5.2 Record: `savagefb_probe()` is assigned as `.probe` in
`savagefb_driver`; `savagefb_init()` calls
`pci_register_driver(&savagefb_driver)`; `pci_register_driver` maps to
`__pci_register_driver()`, which registers the driver with the PCI core.
Impact is limited to S3 Savage PCI/AGP devices.
Step 5.3 Record: relevant callees are `savagefb_create_i2c_busses()`,
`savagefb_probe_i2c_connector()`, `fb_edid_to_monspecs()`,
`fb_videomode_to_modelist()`, `register_framebuffer()`, and the cleanup
helpers. Verified `savagefb_probe_i2c_connector()` can obtain EDID via
DDC or firmware copy.
Step 5.4 Record: reachable during PCI device probe at boot, module load,
hotplug, or driver bind. I did not verify an unprivileged direct
trigger; this appears hardware/config/probe-path reachable, not syscall-
hot-path reachable.
Step 5.5 Record: similar cleanup patterns exist in other fbdev drivers:
`udlfb`, `smscufx`, and `uvesafb` free both `monspecs.modedb` and
`modelist` on teardown/error paths.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: buggy pattern verified in v4.14, v4.19, v5.10, v5.15,
v6.1, v6.6, v6.10, and v6.12 tags: EDID/modelist are created, normal
path destroys `monspecs.modedb`, but `failed:` only deletes I2C busses.
Step 6.2 Record: expected backport difficulty is clean or minor line-
offset adjustment. For older trees, EDID is stored as `par->edid`, but
the new cleanup lines only reference `info`, so no semantic dependency
on the v5.18 local-variable cleanup.
Step 6.3 Record: no related fix for this exact EDID/modelist failed-path
leak found in local history.
## Phase 7: Subsystem Context
Step 7.1 Record: subsystem is fbdev driver code, specifically S3 Savage
framebuffer. Criticality is peripheral/driver-specific, but kernel
memory leaks in probe error paths are still real bugs for affected
hardware/configurations.
Step 7.2 Record: `drivers/video/fbdev` is active, but
`drivers/video/fbdev/savage` has low churn. Recent changes are mostly
small fixes and cleanups.
## Phase 8: Impact And Risk
Step 8.1 Record: affected users are systems with `CONFIG_FB_SAVAGE` and
`CONFIG_FB_SAVAGE_I2C` using S3 Savage hardware or matching devices.
Step 8.2 Record: trigger requires successful EDID-derived allocation
followed by later `savagefb_probe()` failure, such as mode
validation/geometry failure or framebuffer registration failure.
Repeated privileged probe attempts could accumulate leaks; no
unprivileged trigger was verified.
Step 8.3 Record: failure mode is kernel memory/resource leak, severity
medium: not a crash or corruption, but real unreclaimed kernel memory on
failed probe.
Step 8.4 Record: benefit is moderate for affected systems because it
fixes a verified leak across many stable versions. Risk is very low: two
cleanup calls in an existing unwind path, using established helpers.
## Phase 9: Final Synthesis
Step 9.1 Record: evidence for backporting: verified real leak, tiny
scoped patch, existing helper APIs, same buggy pattern present in many
stable tags, no feature/API change, low regression risk. Evidence
against: driver-specific old hardware, only a probe failure path, no
verified user report/test/lore review. Unresolved: exact upstream commit
hash and mailing-list discussion could not be verified.
Step 9.2 Record: stable rules checklist: obviously correct by code
inspection: yes; fixes a real bug: yes, leaked `modedb` and modelist
allocations; important issue: medium, resource leak rather than crash;
small and contained: yes, 2 lines in one function; no new features/APIs:
yes; can apply to stable: likely yes, helpers and buggy code verified in
stable tags.
Step 9.3 Record: not a device-ID, quirk, DT, build, or documentation
exception. It is a standard resource-leak fix.
Step 9.4 Decision: backport. The issue is not high severity, but the fix
is minimal, correct, uses existing cleanup APIs, and addresses a
verified kernel memory leak present across stable trees.
Verification:
- [Phase 1] Parsed supplied subject/body/tags; confirmed no
Fixes/Reported/Tested/Reviewed/Acked/Link/Cc stable tags in the
supplied message.
- [Phase 2] Read `savagefb_probe()` and helper implementations;
confirmed missing failed-path cleanup and verified allocation/free
behavior.
- [Phase 3] Ran `git blame`, `git log`, `git show`, and ancestry checks
without `--all`; found related savage probe/error-path history and no
local candidate commit.
- [Phase 4] Ran `b4 dig` attempts, web search, and lore fetch attempts;
no exact commit/thread verified, lore fetch blocked by Anubis.
- [Phase 5] Traced `savagefb_probe()` registration through the PCI
driver structure and `pci_register_driver()`.
- [Phase 6] Used tag-scoped `git grep` on v4.14, v4.19, v5.10, v5.15,
v6.1, v6.6, v6.10, and v6.12; confirmed the buggy pattern exists.
- [Phase 7] Checked `MAINTAINERS`; verified fbdev and S3 Savage
maintainer entries.
- [Phase 8] Verified trigger and severity from code paths; unprivileged
trigger remains unverified and did not drive the decision.
**YES**
drivers/video/fbdev/savage/savagefb_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/video/fbdev/savage/savagefb_driver.c b/drivers/video/fbdev/savage/savagefb_driver.c
index ac41f8f37589f..c2f79357c8da0 100644
--- a/drivers/video/fbdev/savage/savagefb_driver.c
+++ b/drivers/video/fbdev/savage/savagefb_driver.c
@@ -2322,6 +2322,8 @@ static int savagefb_probe(struct pci_dev *dev, const struct pci_device_id *id)
failed:
#ifdef CONFIG_FB_SAVAGE_I2C
savagefb_delete_i2c_busses(info);
+ fb_destroy_modelist(&info->modelist);
+ fb_destroy_modedb(info->monspecs.modedb);
#endif
fb_alloc_cmap(&info->cmap, 0, 0);
savage_unmap_video(info);
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Fix HostVMMinPageSize unit mismatch in DML2.1
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] fbdev: savage: fix probe-path EDID cleanup leaks Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amdgpu: OR init_pte_flags into invalid leaf PTE updates Sasha Levin
` (2 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Nicholas Kazlauskas, Austin Zheng, Roman Li, Dan Wheeler,
Alex Deucher, Sasha Levin, harry.wentland, sunpeng.li,
Rodrigo.Siqueira, christian.koenig, Xinhui.Pan, airlied, daniel,
amd-gfx, dri-devel, linux-kernel
From: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
[ Upstream commit 5721b5b9c9c792233d7817239bd81925fb3ad9d1 ]
[Why]
This was found back on DML2 but was missed when creating DML2.1.
The bottom layer calculation (CalculateHostVMDynamicLevels) expects
a value in bytes, not KB, but we pass in the value in KB (eg. 4).
This causes an extra page table level to be required in the prefetch
bytes which can be significant overhead - preventing some modes
from being supported that should otherwise be.
[How]
Correct the units by multiplying the input and override values by 1024.
Reviewed-by: Austin Zheng <austin.zheng@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Signed-off-by: Roman Li <roman.li@amd.com>
Tested-by: Dan Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Based on my comprehensive analysis, I have enough information to make a
decision. Let me compile my findings.
## Phase 1: Commit Message Forensics
**Step 1.1:** Subject parsing
Record: [drm/amd/display] [Fix] [HostVMMinPageSize unit mismatch in
DML2.1]
**Step 1.2:** Tags
Record:
- Reviewed-by: Austin Zheng (AMD)
- Signed-off-by: Nicholas Kazlauskas (author, AMD), Roman Li (AMD DC
submaintainer), Alex Deucher (AMD GPU maintainer)
- Tested-by: Dan Wheeler (AMD test engineer)
- No Fixes: tag, no Cc: stable tag (expected for review candidates)
**Step 1.3:** Body analysis
Record: The bug is that `CalculateHostVMDynamicLevels` expects
HostVMMinPageSize in bytes (thresholds 2048 and 1048576 = 2KB and 1MB),
but DML2.1 passes the value in KB (e.g., 4 for 4KB). This causes wrong
branch selection and adds an extra page table level to prefetch
overhead, "preventing some modes from being supported that should
otherwise be." Failure mode = display mode unnecessarily rejected by
validator.
**Step 1.4:** Hidden bug fix detection
Record: Not hidden - clearly described as a fix for a unit mismatch. The
verb "Fix" is explicit.
## Phase 2: Diff Analysis
**Step 2.1:** Inventory
Record: Single file `dml2_core_dcn4_calcs.c`, 6 lines changed (+6/-6), 6
hunks. All in `dml_core_ms_prefetch_check`, `dml_core_mode_support`,
`dml_core_mode_programming`. Scope: surgical single-file fix.
**Step 2.2:** Code flow
Record: Each hunk replaces `hostvm_min_page_size_kbytes` (a value in KB)
with `hostvm_min_page_size_kbytes * 1024` (converting to bytes). Affects
calls to `CalculateExtraLatency`,
`CalculatePrefetchSchedule_params->HostVMMinPageSize`, and
`CalculateVMRowAndSwath_params->HostVMMinPageSize`.
**Step 2.3:** Bug mechanism
Record: Type/unit bug. The receiving function checks `< 2048`, `>= 2048
&& < 1048576`, `>= 1048576` (bytes thresholds). With KB input (e.g., 4),
every value falls into the first branch, causing maximum page table
levels to be added incorrectly, which inflates prefetch bandwidth
requirements.
**Step 2.4:** Fix quality
Record: Trivially correct - just multiplying by a constant. No
regression risk from the fix itself. Same fix pattern was historically
applied to DML2.0 (commit 22136ff27c4e0/dcf6cd7f35de5) with `Cc:
stable`.
## Phase 3: Git History Investigation
**Step 3.1:** File history
Record: File introduced in commit `70839da636050` (2024-04-19, v6.11)
"drm/amd/display: Add new DCN401 sources". Bug present since v6.11.
**Step 3.2:** Fixes: tag follow-up
Record: No Fixes: tag, but commit message references DML2 history. Found
related history:
- `22136ff27c4e0`/`dcf6cd7f35de5` (Nov 2023): Original DML2 fix with Cc:
stable - did exactly this multiplication
- `d0f639c586939`/`a409c053b0b0c` (Dec 2023): Reverted, claimed spec
said KB
- `bf282eb92b8` (Dec 2023): Re-applied the *1024 fix because revert
"causes failure to light up for 1080p eDP + 8k HDMI panel combo"
This proves the *1024 IS the correct value.
**Step 3.3:** File history for related changes
Record: Related patches in same April 2 patch series include:
- Patch 13: `df9228624afde` "Pass min page size from SOC BB to dml2_1
plane config" - related fix but independent
- Patch 14: `90b05672b7f0e` "Fix DCN42 gpuvm_min_page_size_kbytes in SOC
BB" - related but independent
This patch (11) is self-contained.
**Step 3.4:** Author context
Record: Nicholas Kazlauskas is a regular DC contributor and authored the
related DCN35/DCN401 fixes. Reviewer Austin Zheng is also DC
contributor. Submitter Roman Li is DC submaintainer.
**Step 3.5:** Dependencies
Record: Standalone fix. Multiplication by 1024 is purely a numeric
correction at call sites. No dependencies.
## Phase 4: Mailing List Research
**Step 4.1:** b4 dig
Record: b4 dig could not find a match (commit too recent / not yet
indexed). Found via direct lore search at
`https://lists.freedesktop.org/archives/amd-gfx/2026-April/142246.html`.
Posted as PATCH 11/22 of "DC Patches April 02, 2026" by Roman Li on Thu
Apr 2 18:33:03 UTC 2026.
**Step 4.2:** Reviewers
Record: Reviewed by Austin Zheng (AMD DC). Sent to amd-gfx list with
appropriate maintainer CC.
**Step 4.3:** Bug reports
Record: No specific Reported-by, no syzbot link, no bugzilla link. Bug
found internally by AMD when reviewing DML2.1 vs DML2 differences.
**Step 4.4:** Series context
Record: Part of "DC Patches April 02, 2026" with 22 patches. The
Nicholas Kazlauskas DML2.1 cluster (patches 11-15) addresses related but
independent issues. This patch (11) does not depend on the others.
**Step 4.5:** Stable history
Record: No discussion on stable@vger.kernel.org. Original DML2 fix was
Cc'd to stable; this DML2.1 version was not.
## Phase 5: Code Semantic Analysis
**Step 5.1:** Functions modified
Record: 3 functions: `dml_core_ms_prefetch_check`,
`dml_core_mode_support`, `dml_core_mode_programming`. All are core mode
validation/programming entry points called from DML2.1.
**Step 5.2:** Callers
Record: Called from `dml21_create`/`dml21_reinit`, which are called when
`using_dml21=true && dce_version >= DCN_VERSION_4_01`. This means:
DCN401 (RDNA4 / RX 9000 series GPUs) and DCN42 hardware. Reachable from
every display mode validation.
**Step 5.3:** Callees
Record: `CalculateExtraLatency` and via params,
`CalculateHostVMDynamicLevels` (line 1565) which has the byte-threshold
checks (`< 2048`, `< 1048576`).
**Step 5.4:** Reachability
Record: Every kernel modeset path on DCN401/DCN42 hardware. Highly
reachable from userspace via DRM modeset ioctls.
**Step 5.5:** Similar patterns
Record: Same fix pattern was previously applied to DML2.0 in current
mainline (`drivers/gpu/drm/amd/display/dc/dml2_0/display_mode_core.c`
has `* 1024` at the same kind of call sites).
## Phase 6: Cross-Referencing
**Step 6.1:** Code in stable trees
Record: Buggy code present in v6.11 through v6.18 (and v7.0). Verified
with `git show v6.18:drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_
core/dml2_core_dcn4_calcs.c | grep "soc.hostvm_min_page_size_kbytes,"` -
bug exists.
**Step 6.2:** Backport complications
Record: Path was renamed from `dml2/dml21/` to `dml2_0/dml21/` in commit
`e6a8a000cfe6a` (2025-10-21). For stable trees v6.11-v6.18, the file is
at `drivers/gpu/drm/amd/display/dc/dml2/dml21/src/dml2_core/dml2_core_dc
n4_calcs.c`. Each `* 1024` change applies cleanly with path translation
- line numbers vary by tree but contexts are stable. Minor manual rework
needed for path.
**Step 6.3:** Related fixes already in stable
Record: No, the DML2.1 version of this fix has not been backported to
any stable tree.
## Phase 7: Subsystem Context
**Step 7.1:** Subsystem criticality
Record: drivers/gpu/drm/amd/display - PERIPHERAL (driver-specific) but
affects display output, which is user-visible. Users of DCN401 (Navi 4x
discrete GPUs) and DCN42 (newer APUs) can lose display mode
availability.
**Step 7.2:** Subsystem activity
Record: Highly active subsystem with frequent DC patch series.
## Phase 8: Impact and Risk
**Step 8.1:** Affected population
Record: DRIVER-SPECIFIC: Users with AMD DCN401 (RX 9070, RX 9060XT etc.)
or DCN42 hardware running v6.11+. As DCN401 is the Navi 4x architecture
(recent consumer GPU), this is a meaningful but smaller user base than
core fixes.
**Step 8.2:** Trigger conditions
Record: Triggered on every display mode validation when
`using_dml21=true` (default). Bug manifests as "mode rejected" only when
the actual page table level overhead matters, i.e., for high-bandwidth
modes (high resolution + high refresh rate, multi-display). The DML2
history shows real-world failure with "1080p eDP + 8k HDMI" combo.
**Step 8.3:** Failure mode severity
Record: MEDIUM-HIGH. Failure mode is display modes being rejected that
should work. Not a crash or data corruption, but user-visible feature
loss (e.g., user cannot enable their monitor's native resolution/refresh
rate). On laptops with eDP + external display, may prevent multi-monitor
configurations.
**Step 8.4:** Risk-benefit
Record:
- BENEFIT: enables previously-rejected display modes for DCN401/DCN42
users (real-world impact demonstrated in DML2 history)
- RISK: very low - 6 lines of arithmetic correction, well-tested in
mainline, identical pattern proven correct in DML2.0
- Ratio: clearly favors backport
## Phase 9: Final Synthesis
**Step 9.1:** Evidence
FOR backporting:
- Small, surgical, obviously correct fix (just `* 1024`)
- Bug present in stable trees v6.11+
- Same fix pattern proven correct via DML2.0 history (revert/re-apply
cycle confirmed `*1024` is right)
- Tested-by, Reviewed-by from AMD team
- Real user-visible impact demonstrated historically (modes failing to
light up)
- Bug already in mainline since April 2026
- Fix is mathematical - no behavioral risk
AGAINST:
- No Cc: stable tag from authors
- Smaller user base (DCN401/DCN42 specific hardware)
- Path requires translation for older stable trees (`dml2_0/dml21/` ->
`dml2/dml21/`)
- The DML2 version of this fix had a turbulent revert history (though
ultimately confirmed correct)
UNRESOLVED:
- Could not run b4 dig successfully (patch too recent for indexing)
- Could not verify whether AMD will explicitly nominate this for stable
in a follow-up
**Step 9.2:** Stable rules checklist
1. Obviously correct? YES - simple multiplication, function clearly
expects bytes (thresholds in bytes)
2. Fixes real bug affecting users? YES - mode validation incorrectly
rejects valid modes
3. Important issue? MEDIUM - prevents some display modes from working,
similar pattern caused 1080p+8k failure in DML2
4. Small and contained? YES - 6 lines, single file
5. No new features/APIs? YES - pure fix
6. Applies to stable? YES with minor path translation for v6.11-v6.18
**Step 9.3:** Exception categories
Not in special exception categories, but qualifies as a regular bug fix.
**Step 9.4:** Decision
The fix is small, surgical, obviously correct, well-tested, and
addresses a real bug that affects users of recent AMD GPU/APU hardware.
The historical precedent in DML2.0 (where the same `*1024` correction
was eventually applied to stable after extensive testing confirmed it
was right) strengthens confidence. The risk of regression is minimal
given the trivial nature of the change.
## Verification
- [Phase 1] Parsed tags: Reviewed-by (Austin Zheng), Tested-by (Dan
Wheeler), no Fixes/Cc:stable - verified by reading commit message
- [Phase 2] Diff analysis: confirmed 6 hunks, all `* 1024` multiplier
additions in single file - verified by reading patch
- [Phase 3] git log + git show: bug code present since `70839da636050`
(v6.11, 2024-04-19) - verified
- [Phase 3] git show 22136ff27c4e0: original DML2 fix with `Cc:
stable@vger.kernel.org` did same `*1024` change - verified
- [Phase 3] git show bf282eb92b8: revert-of-revert for DML2 because
*1024 absence broke "1080p eDP + 8k HDMI" - verified
- [Phase 3] git show e6a8a000cfe6a: path renamed `dml2/dml21/` ->
`dml2_0/dml21/` in 2025-10-21 - verified
- [Phase 3] Local file read at line 1565: `CalculateHostVMDynamicLevels`
checks `< 2048`, `< 1048576` (bytes thresholds) - verified
- [Phase 4] WebFetch lore archive: found patch 11/22 of "DC Patches
April 02, 2026" thread - verified
- [Phase 4] b4 dig -c bd8001acfa62b: failed to find (patch too recent) -
acknowledged
- [Phase 5] Grep for using_dml21: confirmed DML2.1 active for
DCN401/DCN42 with `dce_version >= DCN_VERSION_4_01` - verified
- [Phase 6] git show
v6.18:.../dml2/dml21/src/.../dml2_core_dcn4_calcs.c: bug present in
v6.18 - verified
- [Phase 6] git show
v6.12:.../dml2/dml21/src/.../dml2_core_dcn4_calcs.c: bug present in
v6.12 - verified
- [Phase 6] dml2_0/display_mode_core.c grep: confirmed `*1024` already
applied for DML2.0 - verified
- [Phase 7] DCN_VERSION_4_01 grep: confirmed dce_version usage gates
DML2.1 to DCN401+ hardware - verified
- UNVERIFIED: Exact mailing list discussion URL - the patch is indexed
at lists.freedesktop.org (got the patch text), but lore.kernel.org has
not yet indexed it (b4 dig failure)
- UNVERIFIED: Whether reviewers explicitly suggested stable backport -
did not find direct discussion
The fix meets all stable kernel criteria: it is obviously correct, fixes
a real bug affecting users of recent AMD hardware, is small and
contained (6 lines, one file), introduces no new features, and applies
to stable with only minor path translation. The historical DML2
precedent strongly supports correctness and the user-visible impact.
**YES**
.../dml21/src/dml2_core/dml2_core_dcn4_calcs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
index ca5ac3c0deb56..cdd7c771afff9 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2_0/dml21/src/dml2_core/dml2_core_dcn4_calcs.c
@@ -7381,7 +7381,7 @@ static noinline_for_stack void dml_core_ms_prefetch_check(struct dml2_core_inter
s->tdlut_bytes_per_group,
s->HostVMInefficiencyFactor,
s->HostVMInefficiencyFactorPrefetch,
- mode_lib->soc.hostvm_min_page_size_kbytes,
+ mode_lib->soc.hostvm_min_page_size_kbytes * 1024,
mode_lib->soc.qos_parameters.qos_type,
!(display_cfg->overrides.max_outstanding_when_urgent_expected_disable),
mode_lib->soc.max_outstanding_reqs,
@@ -7477,7 +7477,7 @@ static noinline_for_stack void dml_core_ms_prefetch_check(struct dml2_core_inter
CalculatePrefetchSchedule_params->OutputFormat = display_cfg->stream_descriptors[display_cfg->plane_descriptors[k].stream_index].output.output_format;
CalculatePrefetchSchedule_params->MaxInterDCNTileRepeaters = mode_lib->ip.max_inter_dcn_tile_repeaters;
CalculatePrefetchSchedule_params->VStartup = s->MaximumVStartup[k];
- CalculatePrefetchSchedule_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes;
+ CalculatePrefetchSchedule_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes * 1024;
CalculatePrefetchSchedule_params->DynamicMetadataEnable = display_cfg->plane_descriptors[k].dynamic_meta_data.enable;
CalculatePrefetchSchedule_params->DynamicMetadataVMEnabled = mode_lib->ip.dynamic_metadata_vm_enabled;
CalculatePrefetchSchedule_params->DynamicMetadataLinesBeforeActiveRequired = display_cfg->plane_descriptors[k].dynamic_meta_data.lines_before_active_required;
@@ -8965,7 +8965,7 @@ static bool dml_core_mode_support(struct dml2_core_calcs_mode_support_ex *in_out
CalculateVMRowAndSwath_params->MALLAllocatedForDCN = mode_lib->soc.mall_allocated_for_dcn_mbytes;
CalculateVMRowAndSwath_params->SwathWidthY = mode_lib->ms.SwathWidthY;
CalculateVMRowAndSwath_params->SwathWidthC = mode_lib->ms.SwathWidthC;
- CalculateVMRowAndSwath_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes;
+ CalculateVMRowAndSwath_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes * 1024;
CalculateVMRowAndSwath_params->DCCMetaBufferSizeBytes = mode_lib->ip.dcc_meta_buffer_size_bytes;
CalculateVMRowAndSwath_params->mrq_present = mode_lib->ip.dcn_mrq_present;
@@ -10755,7 +10755,7 @@ static bool dml_core_mode_programming(struct dml2_core_calcs_mode_programming_ex
CalculateVMRowAndSwath_params->MALLAllocatedForDCN = mode_lib->soc.mall_allocated_for_dcn_mbytes;
CalculateVMRowAndSwath_params->SwathWidthY = mode_lib->mp.SwathWidthY;
CalculateVMRowAndSwath_params->SwathWidthC = mode_lib->mp.SwathWidthC;
- CalculateVMRowAndSwath_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes;
+ CalculateVMRowAndSwath_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes * 1024;
CalculateVMRowAndSwath_params->DCCMetaBufferSizeBytes = mode_lib->ip.dcc_meta_buffer_size_bytes;
CalculateVMRowAndSwath_params->mrq_present = mode_lib->ip.dcn_mrq_present;
@@ -10971,7 +10971,7 @@ static bool dml_core_mode_programming(struct dml2_core_calcs_mode_programming_ex
s->tdlut_bytes_per_group,
s->HostVMInefficiencyFactor,
s->HostVMInefficiencyFactorPrefetch,
- mode_lib->soc.hostvm_min_page_size_kbytes,
+ mode_lib->soc.hostvm_min_page_size_kbytes * 1024,
mode_lib->soc.qos_parameters.qos_type,
!(display_cfg->overrides.max_outstanding_when_urgent_expected_disable),
mode_lib->soc.max_outstanding_reqs,
@@ -11264,7 +11264,7 @@ static bool dml_core_mode_programming(struct dml2_core_calcs_mode_programming_ex
CalculatePrefetchSchedule_params->OutputFormat = display_cfg->stream_descriptors[display_cfg->plane_descriptors[k].stream_index].output.output_format;
CalculatePrefetchSchedule_params->MaxInterDCNTileRepeaters = mode_lib->ip.max_inter_dcn_tile_repeaters;
CalculatePrefetchSchedule_params->VStartup = s->MaxVStartupLines[k];
- CalculatePrefetchSchedule_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes;
+ CalculatePrefetchSchedule_params->HostVMMinPageSize = mode_lib->soc.hostvm_min_page_size_kbytes * 1024;
CalculatePrefetchSchedule_params->DynamicMetadataEnable = display_cfg->plane_descriptors[k].dynamic_meta_data.enable;
CalculatePrefetchSchedule_params->DynamicMetadataVMEnabled = mode_lib->ip.dynamic_metadata_vm_enabled;
CalculatePrefetchSchedule_params->DynamicMetadataLinesBeforeActiveRequired = display_cfg->plane_descriptors[k].dynamic_meta_data.lines_before_active_required;
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0] drm/amdgpu: OR init_pte_flags into invalid leaf PTE updates
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Fix HostVMMinPageSize unit mismatch in DML2.1 Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amd/display: Use overlay cursor when color pipeline is active Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] staging: fbtft: fix unchecked write return value in fb_agm1264k-fl Sasha Levin
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Siwei He, Philip Yang, Alex Deucher, Sasha Levin,
christian.koenig, Xinhui.Pan, airlied, daniel, amd-gfx, dri-devel,
linux-kernel
From: Siwei He <siwei.he@amd.com>
[ Upstream commit 778bf584f2fb0a2b09594f568faf400bf6858091 ]
Invalid leaf clears that only set AMDGPU_PTE_EXECUTABLE match the old
GMC9 fault-priority workaround but omit adev->gmc.init_pte_flags.
On GFX12 that includes AMDGPU_PTE_IS_PTE; without it, some cleared
PTEs can fault as no-retry and bypass the SVM/XNACK handler when a
VA is reused after a BO unmap.
Apply init_pte_flags in amdgpu_vm_pte_update_flags() alongside
EXECUTABLE so range-driven clears (e.g. amdgpu_vm_clear_freed) match
amdgpu_vm_pt_clear() for leaf templates.
Signed-off-by: Siwei He <siwei.he@amd.com>
Reviewed-by: Philip Yang <philip.yang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit 9d47b2c36b9a6c6b844c33cab407a5d7ad102234)
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a thorough analysis.
## Phase 1: Commit Message Forensics
**Step 1.1 - Subject Line Parse**
- Subsystem: `drm/amdgpu` (AMD GPU driver, virtual memory page table
code)
- Action verb: "OR" (operator), implies a missing flag combination → bug
fix
- Record: `drm/amdgpu` adds `init_pte_flags` to invalid leaf PTE updates
to match `amdgpu_vm_pt_clear()` behavior
**Step 1.2 - Tags Inventory**
- Signed-off-by: Siwei He (author)
- Reviewed-by: Philip Yang (AMD/amdkfd maintainer)
- Signed-off-by: Alex Deucher (DRM/amdgpu maintainer)
- `(cherry picked from commit 9d47b2c36b9a6c6b844c33cab407a5d7ad102234)`
— already merged upstream
- No Fixes:, no Cc: stable, no syzbot/Reported-by (expected per pipeline
rules)
- Record: Reviewed and signed by relevant subsystem maintainers; cherry
pick from upstream
**Step 1.3 - Commit Body Analysis**
- Bug description: leaf PTE clears that only set `AMDGPU_PTE_EXECUTABLE`
omit `adev->gmc.init_pte_flags`
- Affected hardware: GFX12 (where `init_pte_flags` includes
`AMDGPU_PTE_IS_PTE`)
- Symptom: cleared PTEs can fault as no-retry and bypass SVM/XNACK
handler when VA is reused after BO unmap
- Root cause: code template inconsistency between `amdgpu_vm_pt_clear()`
(already updated) and `amdgpu_vm_pte_update_flags()` (range-driven
clear path used by e.g., `amdgpu_vm_clear_freed`)
- Record: clearly states bug mechanism; affects real GFX12 hardware
running SVM/XNACK after BO unmap → VA reuse
**Step 1.4 - Hidden Bug Fix Detection**
- Although not titled "fix", the body explicitly describes a fault-
handler bypass on GFX12 — this IS a bug fix
- Record: explicit bug fix (not hidden)
## Phase 2: Diff Analysis
**Step 2.1 - Inventory**
- Single file: `drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c`
- Hunks: 1; lines: net +1 (statement extended), comment expanded
- Function modified: `amdgpu_vm_pte_update_flags()`
- Scope: surgical, single-line semantic change in one function
- Record: tiny single-file fix, ~2 lines of logic change
**Step 2.2 - Code Flow Change**
- Before: when handling an invalid leaf clear (`level==PTB && !VALID &&
!PRT`), `flags |= AMDGPU_PTE_EXECUTABLE`
- After: `flags |= AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags`
- On GFX12.1, `init_pte_flags = AMDGPU_PTE_IS_PTE`; on GMC9 it's 0 (no
behavior change there)
- Record: aligns the leaf-clear template with `amdgpu_vm_pt_clear()`
(line 416 already does the same)
**Step 2.3 - Bug Mechanism**
- Category: Logic/correctness fix → consistency between two clear paths
- Mechanism: template mismatch on GFX12 page table clears caused PTEs to
be marked without `IS_PTE`, leading to no-retry faults that bypass the
SVM/XNACK fault handler
- Record: same template now used in both leaf-clear sites; this is a
hardware-correctness fix
**Step 2.4 - Fix Quality**
- Obviously correct: mirrors existing pattern at line 416 of same file
(`amdgpu_vm_pt_clear`)
- Minimal/surgical: a single OR with a per-ASIC field that defaults to 0
- Regression risk: very low — on non-GFX12 hardware `init_pte_flags ==
0`, so behavior is unchanged
- Record: high quality, low regression risk
## Phase 3: Git History Investigation
**Step 3.1 - Blame**
- The buggy line existed before, but the *omission* was created by
`db29ddf6505f3` ("drm/amdgpu: Add per-ASIC PTE init flag", Apr 24,
2025) which added `init_pte_flags` and applied it in
`amdgpu_vm_pt_clear()` only — not in `amdgpu_vm_pte_update_flags()`
- Record: bug introduced by db29ddf6505f3
**Step 3.2 - Fixes Target**
- No explicit `Fixes:` tag in this commit, but the underlying fault-
handler-bypass bug requires `init_pte_flags` to exist, which only
appeared in `db29ddf6505f3`
- That commit lands in v7.0-rc1 (verified: `git tag --contains
db29ddf6505f3` shows v7.0-rc1+ only)
- Record: bug only exists in v7.0+; older stable trees do not have
`init_pte_flags`
**Step 3.3 - File History**
- Last commit on `amdgpu_vm_pt.c`: `db29ddf6505f3` (the very commit that
introduced the inconsistency)
- Record: this fix immediately follows the bug-introducing commit;
standalone, no prerequisite missing
**Step 3.4 - Author**
- Siwei He (AMD developer) — the upstream cherry-pick is reviewed by
Philip Yang and signed by Alex Deucher (amdgpu maintainer)
- Record: properly vetted by amdgpu maintainership
**Step 3.5 - Dependencies**
- Requires `adev->gmc.init_pte_flags` — present in v7.0+ via
`db29ddf6505f3`
- No other dependency
- Record: standalone fix in v7.0; not applicable to pre-v7.0 stable
trees
## Phase 4: Mailing List / External Research
- `b4 dig -c 9d47b2c36b9a6c6b844c33cab407a5d7ad102234` — SHA not present
in local repo (cherry-pick hash from a tree this repo doesn't have)
- `b4 dig -c db29ddf6505f3` — no lore match found
- `lore.kernel.org` direct fetch blocked by Anubis bot challenge / 403
from raw curl — could not retrieve discussion
- Record: UNVERIFIED — could not retrieve the original lore discussion
thread; relying on the in-tree review trail (Reviewed-by Philip Yang,
Signed-off-by Alex Deucher)
## Phase 5: Code Semantic Analysis
**Step 5.1 - Functions in Diff**
- `amdgpu_vm_pte_update_flags()`
**Step 5.2 - Callers**
- Called from `amdgpu_vm_update_ptes()` (line 909 in the same file)
- That is called from `amdgpu_vm_update_range()` in `amdgpu_vm.c`
- `amdgpu_vm_update_range()` is called from many sites:
`amdgpu_vm_clear_freed` (line 1573, with flags=0 → exact bug path),
`amdgpu_vm_bo_update`, `amdgpu_gem_va_ioctl`, etc.
- Record: the buggy path is reached on EVERY BO unmap that places
mappings on `vm->freed`
**Step 5.3 - Callees**
- Calls `update_funcs->update()` to write PTEs
- Record: writes the actual page table entries — direct hardware effect
**Step 5.4 - Reachability**
- `amdgpu_vm_clear_freed` runs from normal GEM unmap/CS paths and from
KFD memory paths
- Triggerable by any user/process unmapping a GPU buffer with a VA that
gets reused
- Record: trivially reachable from userspace via standard amdgpu/KFD
ioctls
**Step 5.5 - Similar Patterns**
- Only two leaf-clear template sites; the other one
(`amdgpu_vm_pt_clear` line 416) was already updated to use
`init_pte_flags`. This patch makes the second site consistent.
- Record: closes the only remaining inconsistent site
## Phase 6: Cross-Referencing & Stable Tree Analysis
**Step 6.1 - Bug Presence in Stable Trees**
- `init_pte_flags` field exists only in v7.0+ — verified by `git tag
--contains db29ddf6505f3` showing earliest tag `v7.0-rc1`
- Pre-v7.0 stable trees (6.6.y, 6.1.y, 5.15.y, 5.10.y) do NOT have this
field, so this fix does not apply there and the specific bug being
addressed does not exist in that form there
- Record: target stable tree for this fix is v7.0.y (matches workspace
path `linux-autosel-7.0`)
**Step 6.2 - Backport Difficulty**
- `amdgpu_vm_pte_update_flags()` exists unchanged in v7.0.y; the diff
applies cleanly
- Record: clean apply to 7.0.y
**Step 6.3 - Related Fixes Already In Stable**
- The companion fix at `amdgpu_vm_pt_clear()` (line 416) is part of
`db29ddf6505f3` which is in v7.0
- This commit is the second half of that fix
- Record: 7.0.y already has half of the pattern; this patch completes it
## Phase 7: Subsystem Context
**Step 7.1 - Subsystem Criticality**
- `drivers/gpu/drm/amd/amdgpu` — important driver subsystem (large user
base for AMD GPUs)
- This specific code path: GFX12 (RDNA4 / RX 9000 series) SVM/XNACK —
real shipping consumer hardware
- Record: IMPORTANT (driver-specific, affects current AMD hardware)
**Step 7.2 - Activity**
- amdgpu is a very active subsystem with frequent fixes
- Record: highly active; fixes-quickly-integrated subsystem
## Phase 8: Impact and Risk Assessment
**Step 8.1 - Affected Users**
- GFX12.1 (RDNA4 / RX 9000 series) hardware users running compute
workloads with SVM/XNACK enabled (ROCm, HIP, OpenCL, etc.)
- Record: driver-specific (GFX12 only with SVM); on other ASICs
`init_pte_flags == 0` so no behavior change
**Step 8.2 - Trigger**
- Trigger: any unmap of a GPU buffer where the VA later gets reused
- Reachable from unprivileged user code via standard amdgpu/KFD ioctls
- Common in compute workloads that allocate/free buffers
- Record: easily triggered from userspace; common in real workloads
**Step 8.3 - Severity**
- Failure mode: PTEs faulting as no-retry that bypass the SVM/XNACK
handler — the handler is what makes SVM-on-demand actually work, so
its bypass leads to incorrect fault behavior on GPU memory accesses
- Severity: HIGH for affected users (broken SVM/XNACK semantics on new
HW); MEDIUM-CRITICAL depending on workload (silent incorrect access
vs. application failure)
- Record: HIGH severity for GFX12 SVM users
**Step 8.4 - Risk/Benefit**
- Benefit: high — fixes broken SVM on current shipping AMD hardware
- Risk: very low — single OR with a field that is 0 on every other ASIC;
mirrors a sibling site already in the tree
- Record: clear net positive for backport
## Phase 9: Final Synthesis
**Evidence FOR backport**
- Real bug on real shipping GFX12 hardware (RX 9000 / RDNA4) with
SVM/XNACK
- Trivial 2-line fix; mirrors existing pattern (`amdgpu_vm_pt_clear`) at
line 416 of the same file
- Reviewed by Philip Yang (amdkfd) and signed by Alex Deucher (amdgpu
maintainer)
- Buggy template inconsistency was introduced in v7.0-rc1 by
`db29ddf6505f3`; lands cleanly in v7.0.y
- On non-GFX12 ASICs `init_pte_flags == 0` → guaranteed no behavior
change there
- Reachable via standard userspace ioctls (BO unmap → VA reuse)
**Evidence AGAINST backport**
- No `Fixes:` or `Cc: stable` tag (expected per pipeline rules; not a
negative signal)
- Lore discussion could not be fetched for further verification
(UNVERIFIED)
- Does NOT apply to pre-v7.0 stable trees (no `init_pte_flags` field
there); only relevant to 7.0.y
**Stable Rules Checklist**
1. Obviously correct & tested: yes — mirrors a sibling site, very small
change, signed off by maintainers
2. Fixes a real bug: yes — fault-handler bypass on GFX12 SVM/XNACK
3. Important issue: yes — broken VA-reuse semantics on shipping hardware
4. Small and contained: yes — single hunk, ~2 lines logic
5. No new features/APIs: yes — pure consistency fix
6. Applies to stable: yes for 7.0.y (clean), N/A for older
**Decision**
This is a small, surgical, maintainer-reviewed fix that closes a
hardware-correctness gap on shipping AMD GPUs (GFX12 SVM/XNACK) by
mirroring an already-applied pattern in the same file. The matching
companion fix is already in 7.0, so this completes that work. Risk is
essentially nil on non-GFX12 hardware (zero OR'd in).
## Verification
- [Phase 1] Parsed commit message tags and body: identified Reviewed-by
Philip Yang, Signed-off-by Alex Deucher; no syzbot, no Fixes
(expected)
- [Phase 2] Read diff and surrounding code at `amdgpu_vm_pt.c:679-721`:
confirmed single hunk in `amdgpu_vm_pte_update_flags()`, change is
`flags |= AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags`
- [Phase 2] Read `amdgpu_vm_pt.c:361-418` to verify the sibling site
`amdgpu_vm_pt_clear()` already uses the same template at line 416
(`flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags`)
- [Phase 3] `git log --oneline --grep="PTE init flag" master` → found
`db29ddf6505f3`
- [Phase 3] `git show db29ddf6505f3` → confirmed it added
`init_pte_flags` field, set `init_pte_flags = AMDGPU_PTE_IS_PTE` for
GFX12.1, and updated only `amdgpu_vm_pt_clear()` (not
`amdgpu_vm_pte_update_flags()`)
- [Phase 3] `git tag --contains db29ddf6505f3` → earliest tag `v7.0-rc1`
— confirms bug lives in v7.0+ only
- [Phase 3] `git log --oneline --
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c` → no other intermediate
fixes between `db29ddf6505f3` and HEAD
- [Phase 4] `b4 dig -c 9d47b2c36b9a6c6b844c33cab407a5d7ad102234` → SHA
not in this repo (cherry-pick hash from elsewhere)
- [Phase 4] `b4 dig -c db29ddf6505f3` → no lore match found
- [Phase 4] WebFetch / curl to lore.kernel.org → blocked by Anubis bot
challenge / 403 — UNVERIFIED for original mailing list discussion
content
- [Phase 5] `Grep AMDGPU_PTE_EXECUTABLE` and `Grep AMDGPU_PTE_IS_PTE`
and `Grep init_pte_flags` across `drivers/gpu/drm/amd` → confirmed
only two leaf-clear template sites; `init_pte_flags` is only set non-
zero for GFX12.1 (`gmc_v12_0.c:643`)
- [Phase 5] Read `amdgpu_vm_pt.c:880-921` → verified caller chain:
`amdgpu_vm_update_ptes` → `amdgpu_vm_pte_update_flags`
- [Phase 5] `Grep amdgpu_vm_clear_freed` and read
`amdgpu_vm.c:1548-1577` → confirmed `amdgpu_vm_clear_freed` calls
`amdgpu_vm_update_range` with flags=0, hitting the patched `else if`
branch
- [Phase 6] `git tag --contains db29ddf6505f3` → confirms
`init_pte_flags` only in v7.0+; older stable trees lack the field, fix
is not applicable to them
- [Phase 6] Read of HEAD source confirms `amdgpu_vm_pte_update_flags`
exists unchanged in v7.0.1 → diff applies cleanly
- [Phase 8] Bug mechanism verified via commit body + code: leaf-clear
template inconsistency on GFX12 → no-retry faults bypass SVM/XNACK
handler on VA reuse
- UNVERIFIED: original lore discussion (lore blocked by anti-bot)
- UNVERIFIED: independent reproduction reports / bug-report links (none
in commit body)
The fix is small, obviously correct, addresses a real hardware-
correctness bug on current AMD GPUs, mirrors a sibling site already in
v7.0, and is essentially a no-op on non-GFX12 ASICs. It is appropriate
for the v7.0.y stable tree.
**YES**
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 31a437ce95704..a930f1522f962 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -693,8 +693,11 @@ static void amdgpu_vm_pte_update_flags(struct amdgpu_vm_update_params *params,
!(flags & AMDGPU_PTE_VALID) &&
!(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
- /* Workaround for fault priority problem on GMC9 */
- flags |= AMDGPU_PTE_EXECUTABLE;
+ /* Workaround for fault priority problem on GMC9 and GFX12,
+ * EXECUTABLE for GMC9 fault priority and init_pte_flags
+ * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
+ */
+ flags |= AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
}
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0] drm/amd/display: Use overlay cursor when color pipeline is active
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amdgpu: OR init_pte_flags into invalid leaf PTE updates Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] staging: fbtft: fix unchecked write return value in fb_agm1264k-fl Sasha Levin
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Alex Hung, Sun peng (Leo) Li, Aurabindo Pillai, Dan Wheeler,
Alex Deucher, Sasha Levin, harry.wentland, Rodrigo.Siqueira,
christian.koenig, Xinhui.Pan, airlied, daniel, amd-gfx, dri-devel,
linux-kernel
From: Alex Hung <alex.hung@amd.com>
[ Upstream commit d3a549f4df7864bca8612c8bcfce1ec72b2874fb ]
Force overlay cursor mode when an underlying plane has a non-bypassed
color pipeline to avoid incorrect cursor transformation.
Reviewed-by: Sun peng (Leo) Li <sunpeng.li@amd.com>
Signed-off-by: Alex Hung <alex.hung@amd.com>
Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
Tested-by: Dan Wheeler <daniel.wheeler@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information for a comprehensive analysis.
## Phase 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject line parsing**
- Subsystem: `drm/amd/display`
- Action verb: "Use" (implicit "fix" through "to avoid incorrect cursor
transformation")
- Record: drm/amd/display - changes cursor mode selection logic when
underlying plane has active color pipeline
**Step 1.2: Tag parsing**
- Reviewed-by: Sun peng (Leo) Li <sunpeng.li@amd.com> (AMD display
maintainer)
- Signed-off-by: Alex Hung <alex.hung@amd.com> (author)
- Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com> (AMD)
- Tested-by: Dan Wheeler <daniel.wheeler@amd.com> (AMD)
- Signed-off-by: Alex Deucher <alexander.deucher@amd.com> (subsystem
maintainer)
- NO Fixes: tag, NO Cc: stable, NO Reported-by, NO Link
- Record: Standard AMD development chain, reviewed and tested
internally, no external bug report references
**Step 1.3: Commit body analysis**
- Bug described: Cursor in NATIVE mode is subjected to color pipeline
transformation of underlying plane, causing "incorrect cursor
transformation"
- Solution: Force OVERLAY cursor mode (which uses dedicated DCN pipe)
when underlying plane has non-bypassed color pipeline
- No version info, no stack traces, no reproduction steps
- Record: Bug = visual incorrectness in cursor color rendering when
color pipeline is active. Symptom is described conceptually, not
concretely.
**Step 1.4: Hidden bug fix detection**
- "Use overlay cursor when..." -> "to avoid incorrect cursor
transformation" reveals this IS a fix, but described as enhancement
- Record: Fixes a real correctness issue, but framed as adding new code
path rather than emergency fix.
## Phase 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- Single file: `drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c`
- ~49 lines added, ~4 lines deleted
- Functions modified: `dm_crtc_get_cursor_mode()`,
`amdgpu_dm_atomic_check()` (debug message)
- New helper: `dm_plane_color_pipeline_active()` (static)
- Adds `#include <drm/drm_colorop.h>`
- Record: Surgical, single-file fix, contained.
**Step 2.2: Code flow change**
- Before: `dm_crtc_get_cursor_mode()` only checked YUV format and scale
differences for overlay cursor decision
- After: Also checks if any plane in z-order has an active (non-
bypassed) color pipeline
- Two checks added: (1) trigger consider_mode_change if color pipeline
activity changes between old/new, (2) force OVERLAY mode when
underlying plane has active pipeline
- Record: Adds new condition for selecting OVERLAY cursor mode.
**Step 2.3: Bug mechanism**
- Category: Logic/correctness fix - missing condition check
- The native cursor (integrated in DCN hw plane) was being subjected to
color pipeline transformations meant for the underlying plane; this
corrupts cursor visual rendering
- Fix forces overlay (separate DCN pipe) which is not subject to
underlying plane's color pipeline
- Record: Visual correctness bug; not a crash/UAF/race/leak
**Step 2.4: Fix quality**
- Obviously correct: Yes, mirrors existing YUV/scaling check pattern
- Minimal/surgical: Yes
- Regression risk: Low - only changes cursor mode selection on a
specific narrow condition (active color pipeline)
- Record: High quality, well-contained.
## Phase 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame analysis**
- The `dm_crtc_get_cursor_mode()` function was introduced in commit
`1b04dcca4fb1` (drm/amd/display: Introduce overlay cursor mode),
pre-v6.18
- The drm_colorop infrastructure was introduced in commit
`cfc27680ee208` (Nov 26, 2025), present from v6.19
- Record: The function exists since pre-6.18, but the bug only triggers
when color pipeline (v6.19+) is active.
**Step 3.2: Fixes: tag - N/A, no Fixes: tag**
**Step 3.3: Recent file history**
- Active development on color pipeline in AMD display driver
- Several recent color pipeline fixes: `b49814033cb52` (Fix gamma 2.2
colorop TFs), `a4fa2355e0add` (Enable DEGAMMA and reject
COLOR_PIPELINE+DEGAMMA_LUT), `18a4127e93156` (Disable CRTC degamma
when color pipeline is enabled)
- Standalone fix; not part of an explicit X/Y series
- Record: Standalone correctness fix in actively developed area
**Step 3.4: Author**
- Alex Hung is a regular AMD display contributor; numerous recent
commits in this area
- Record: Trusted developer, area maintainer chain present
**Step 3.5: Dependencies**
- Requires: `<drm/drm_colorop.h>`, `for_each_oldnew_colorop_in_state`
macro, `drm_colorop_state` struct with `colorop`/`bypass` fields,
`drm_colorop` struct with `plane` field
- All present in v6.19.14 and v7.0.1 stable trees - verified by direct
inspection
- Record: All dependencies present in v6.19.y and v7.0.y; will apply
cleanly.
## Phase 4: MAILING LIST RESEARCH
**Step 4.1: b4 dig results**
- `b4 dig -c d3a549f4df786`: "Could not find anything matching commit"
- `b4 dig -c 5d09aac12d5be`: "Could not find anything matching commit"
- Manual lore search via search engine: did not find direct submission
of this exact patch
- BUT found relevant prior discussion: lists.freedesktop.org dri-devel
April 2025 thread - Harry Wentland confirmed: "Yes, AMD driver is
using the overlay cursor (entire dedicated HW pipe) for the cursor
when the cursor scaling doesn't match the underlying plane. **The same
thing can be done for color operations but it's not implemented
yet.**"
- Record: This commit IMPLEMENTS the missing functionality identified
during the original color pipeline patch series review.
**Step 4.2: Reviewers**
- Reviewed-by: Leo Li (AMD display maintainer)
- Tested-by: Dan Wheeler (AMD QA)
- Record: Properly reviewed by relevant maintainer.
**Step 4.3: Bug report - N/A** (no Reported-by, no Link)
**Step 4.4: Series context**
- Standalone patch (not part of X/Y series)
- Builds upon entire color pipeline infrastructure already in v6.19+
- Record: Self-contained; depends only on v6.19+ infrastructure.
**Step 4.5: Stable list - no specific discussion found**
## Phase 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Key functions**
- New: `dm_plane_color_pipeline_active(state, plane, use_old)` - checks
for non-bypassed colorops on a plane
- Modified: `dm_crtc_get_cursor_mode()` - cursor mode selection
- Modified: debug message in `amdgpu_dm_atomic_check()`
**Step 5.2: Callers**
- `dm_crtc_get_cursor_mode()` is called from `amdgpu_dm_atomic_check()`
for every atomic commit when cursor configuration may change on AMD
DCN hardware
- Affects: Every modeset/cursor update path on supported AMD DCN
hardware
- Record: Reachable from userspace via DRM atomic commit syscalls
**Step 5.3: Callees**
- `for_each_oldnew_colorop_in_state` (DRM core macro from v6.19+)
- `drm_atomic_get_plane_state`, `drm_atomic_plane_enabling/disabling`
- Record: Standard DRM atomic helpers
**Step 5.4: Reachability**
- User triggers: opt-in to `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE` AND
configure non-bypassed colorop on a primary plane
- Modern Wayland compositors are adopting color pipeline API
- Record: Reachable but requires opt-in to relatively new API
**Step 5.5: Similar patterns**
- Existing YUV format check and scaling check follow same pattern
- The fix is the third "underlying plane property" check, parallel to
the existing two
- Record: Consistent with established pattern.
## Phase 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
**Step 6.1: Code in stable trees**
- `drm_colorop` infrastructure: NOT in v6.18 or earlier; PRESENT in
v6.19.14 and v7.0.1 (verified by `git cat-file -e`)
- `dm_crtc_get_cursor_mode()`: present in v6.18, v6.19.14, v7.0.1
(verified by direct inspection)
- The bug only manifests in v6.19+ (where colorop is operational on AMD)
- Record: Stable trees affected: v6.19.y and v7.0.y only. Older stables
(v6.18, v6.12, v6.6, v6.1, v5.15, v5.10) DO NOT have the buggy code
path because color pipeline didn't exist.
**Step 6.2: Backport difficulty**
- `dm_crtc_get_cursor_mode()` structure identical between mainline and
v6.19.14/v7.0.1
- All required infrastructure (`for_each_oldnew_colorop_in_state`,
`drm_colorop_state.colorop`, `drm_colorop_state.bypass`,
`drm_colorop.plane`) is present in v6.19.14 - verified
- Record: Expected clean apply to v6.19.y and v7.0.y stable trees.
**Step 6.3: Related fixes already in stable**
- `e180b2af2725c` (drm/amd/display: Fix gamma 2.2 colorop TFs)
backported to 6.19.y
- `083f1f71a9291` (drm/amd/display: Enable DEGAMMA and reject
COLOR_PIPELINE+DEGAMMA_LUT) backported to 6.19.y
- `0b26c7e819c40` (drm/atomic: convert drm_atomic_get_{old,
new}_colorop_state() into proper functions) backported to 6.19.y
- Record: Strong precedent of color pipeline correctness fixes
backported to 6.19.y stable.
## Phase 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: Subsystem**
- DRM/AMD display - graphics driver
- Criticality: PERIPHERAL/IMPORTANT - affects many AMD GPU users but
only those using new color pipeline feature
- Record: AMD display driver - impacts user-visible rendering for users
who opt-in to color pipeline
**Step 7.2: Activity**
- Heavy activity on color pipeline area; multiple recent fixes
- Record: Very active subsystem; AMD display team actively maintaining
color pipeline
## Phase 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Affected users**
- AMD GPU users with DCN hardware (DCN1+, except DCN401/420 which are
exempt)
- Who use compositors that opt-in to
`DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`
- Modern Wayland compositors increasingly use the new API
- Record: Driver-specific (AMD) AND opt-in feature dependent
**Step 8.2: Trigger conditions**
- Userspace must enable `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`
- A non-cursor plane must have an active (non-bypassed) colorop
- Cursor must be enabled on the same CRTC
- Triggerable by unprivileged users via DRM atomic commit
- Record: Relatively narrow but reachable from userspace
**Step 8.3: Failure mode severity**
- Cursor undergoes incorrect color transformation -> cursor renders with
wrong colors/visual artifacts
- No crash, no data corruption, no security implications, no system hang
- Severity: MEDIUM (visual correctness only)
- Record: Visual bug only - severity MEDIUM
**Step 8.4: Risk-benefit**
- BENEFIT: Correct cursor rendering for users of color pipeline; small
but real correctness improvement
- RISK: Very low - single file, ~50 lines, well-contained, reviewed by
AMD maintainer, tested
- Ratio: Low-risk, moderate-benefit fix
- Record: Favorable risk/benefit for a correctness fix
## Phase 9: FINAL SYNTHESIS
**Step 9.1: Evidence compilation**
Evidence FOR backporting:
- Real bug fix (incorrect cursor color rendering when color pipeline
active)
- Small (~49 lines), single file, surgical change
- Reviewed-by AMD display maintainer (Leo Li)
- Tested-by AMD QA (Dan Wheeler)
- Clean apply to v6.19.y and v7.0.y stable (all dependencies verified
present)
- Strong precedent: similar color pipeline correctness fixes already
backported to 6.19.y
- Reachable from userspace via standard DRM atomic API
- Implements functionality explicitly identified as missing during
original color pipeline review
Evidence AGAINST backporting:
- No Fixes: tag, no Cc: stable
- No Reported-by from external users (only AMD internal Tested-by)
- Color pipeline is documented under Documentation/gpu/rfc/ (RFC status)
- Opt-in feature requiring `DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE`
- Bug is visual-only (no crash, corruption, security)
- Commit reads as feature enhancement rather than fix
**Step 9.2: Stable rules checklist**
1. Obviously correct and tested? YES (Reviewed/Tested by AMD)
2. Fixes a real bug affecting users? YES (visual correctness)
3. Important issue? BORDERLINE (visual only, but real)
4. Small and contained? YES (single file, ~50 lines)
5. No new features/APIs? YES (uses existing APIs)
6. Can apply to stable trees? YES (verified for v6.19.y and v7.0.y)
**Step 9.3: Exception categories - N/A**
**Step 9.4: Decision**
The patch represents a small, well-reviewed correctness fix for cursor
rendering on AMD hardware when the color pipeline feature is in use.
While the bug is visual-only (no crashes/corruption/security), the fix
is exactly the type that has been backported to v6.19.y stable for the
same color pipeline area (e180b2af2725c, 083f1f71a9291). The fix applies
cleanly, has appropriate review/test signoffs, and implements a behavior
that was explicitly identified as needed during the original color
pipeline review. Affected stable trees are limited to v6.19.y and v7.0.y
where the colorop infrastructure exists.
## Verification
- [Phase 1] Parsed tags: Reviewed-by (AMD maintainer), Tested-by (AMD),
Signed-off-by chain through AMD maintainers; NO Fixes:, NO Cc: stable,
NO Reported-by, NO Link
- [Phase 1] Commit body: describes "incorrect cursor transformation" -
confirms this is a fix despite enhancement-style subject
- [Phase 2] Diff inventory verified: 1 file, ~49 insertions, ~4
deletions, adds `dm_plane_color_pipeline_active()` helper and modifies
`dm_crtc_get_cursor_mode()`
- [Phase 2] Bug mechanism: native cursor subject to underlying plane's
color pipeline transformation -> wrong cursor colors; fix forces
overlay mode (separate DCN pipe)
- [Phase 3] git log on file: confirmed `dm_crtc_get_cursor_mode()`
exists in v6.18 and forward
- [Phase 3] git log search: identified colorop infra introduced by
`cfc27680ee208` (Nov 26, 2025), tagged v6.19+
- [Phase 4] b4 dig -c d3a549f4df786: did not find lore submission
directly
- [Phase 4] Web search: found prior discussion on dri-devel April 2025
confirming this functionality was deferred during original color
pipeline review
- [Phase 5] Modified function `dm_crtc_get_cursor_mode()` callable via
amdgpu_dm_atomic_check from DRM atomic commit syscall
- [Phase 6] git cat-file -e v6.19.14:include/drm/drm_colorop.h: EXISTS
(bug applicable)
- [Phase 6] git cat-file -e v7.0.1:include/drm/drm_colorop.h: EXISTS
(bug applicable)
- [Phase 6] git cat-file -e v6.18.24:include/drm/drm_colorop.h: NOT
EXISTS (bug N/A for older stables)
- [Phase 6] Verified `for_each_oldnew_colorop_in_state` macro and
`drm_colorop_state.{colorop,bypass}`, `drm_colorop.plane` all present
in v6.19.14
- [Phase 6] git log v6.19..v6.19.14: found e180b2af2725c, 083f1f71a9291,
0b26c7e819c40 already backported (precedent for color pipeline fixes)
- [Phase 8] Failure mode: visual cursor color rendering incorrect; no
crash/corruption/security
- UNVERIFIED: Could not find direct lore submission via b4 dig (commit
has not yet appeared in lore search index, possibly because it is a
recent merge)
The fix targets a real correctness issue limited to v6.19+ stable trees,
applies cleanly, follows the same pattern as previously backported color
pipeline fixes, and has appropriate review/test signoffs from AMD
maintainers.
**YES**
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 53 +++++++++++++++++--
1 file changed, 49 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2328c1aa0ead1..853e62c67375e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -94,6 +94,7 @@
#include <drm/drm_utils.h>
#include <drm/drm_vblank.h>
#include <drm/drm_audio_component.h>
+#include <drm/drm_colorop.h>
#include <drm/drm_gem_atomic_helper.h>
#include <media/cec-notifier.h>
@@ -12278,6 +12279,38 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm
* available.
*/
+/**
+ * dm_plane_color_pipeline_active() - Check if a plane's color pipeline active.
+ * @state: DRM atomic state
+ * @plane: DRM plane to check
+ * @use_old: if true, inspect the old colorop states; otherwise the new ones
+ *
+ * A color pipeline may be selected (color_pipeline != NULL) but still is
+ * inactive if every colorop in the chain is bypassed. Only return
+ * true when at least one colorop has bypass == false, meaning the cursor
+ * would be subjected to the transformation in native mode.
+ *
+ * Return: true if the pipeline modifies pixels, false otherwise.
+ */
+static bool dm_plane_color_pipeline_active(struct drm_atomic_state *state,
+ struct drm_plane *plane,
+ bool use_old)
+{
+ struct drm_colorop *colorop;
+ struct drm_colorop_state *old_colorop_state, *new_colorop_state;
+ int i;
+
+ for_each_oldnew_colorop_in_state(state, colorop, old_colorop_state, new_colorop_state, i) {
+ struct drm_colorop_state *cstate = use_old ? old_colorop_state : new_colorop_state;
+
+ if (cstate->colorop->plane != plane)
+ continue;
+ if (!cstate->bypass)
+ return true;
+ }
+ return false;
+}
+
/**
* dm_crtc_get_cursor_mode() - Determine the required cursor mode on crtc
* @adev: amdgpu device
@@ -12289,8 +12322,8 @@ static int add_affected_mst_dsc_crtcs(struct drm_atomic_state *state, struct drm
* the dm_crtc_state.
*
* The cursor should be enabled in overlay mode if there exists an underlying
- * plane - on which the cursor may be blended - that is either YUV formatted, or
- * scaled differently from the cursor.
+ * plane - on which the cursor may be blended - that is either YUV formatted,
+ * scaled differently from the cursor, or has a color pipeline active.
*
* Since zpos info is required, drm_atomic_normalize_zpos must be called before
* calling this function.
@@ -12327,7 +12360,7 @@ static int dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
/*
* Cursor mode can change if a plane's format changes, scale changes, is
- * enabled/disabled, or z-order changes.
+ * enabled/disabled, z-order changes, or color management properties change.
*/
for_each_oldnew_plane_in_state(state, plane, old_plane_state, plane_state, i) {
int new_scale_w, new_scale_h, old_scale_w, old_scale_h;
@@ -12352,6 +12385,12 @@ static int dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
consider_mode_change = true;
break;
}
+
+ if (dm_plane_color_pipeline_active(state, plane, true) !=
+ dm_plane_color_pipeline_active(state, plane, false)) {
+ consider_mode_change = true;
+ break;
+ }
}
if (!consider_mode_change && !crtc_state->zpos_changed)
@@ -12392,6 +12431,12 @@ static int dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
return 0;
}
+ /* Underlying plane has an active color pipeline - cursor would be transformed */
+ if (dm_plane_color_pipeline_active(state, plane, false)) {
+ *cursor_mode = DM_CURSOR_OVERLAY_MODE;
+ return 0;
+ }
+
dm_get_plane_scale(plane_state,
&underlying_scale_w, &underlying_scale_h);
dm_get_plane_scale(cursor_state,
@@ -12770,7 +12815,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
goto fail;
} else if (required_cursor_mode == DM_CURSOR_OVERLAY_MODE) {
drm_dbg_driver(crtc->dev,
- "[CRTC:%d:%s] Cannot enable native cursor due to scaling or YUV restrictions\n",
+ "[CRTC:%d:%s] Cannot enable native cursor due to scaling, YUV, or color pipeline restrictions\n",
crtc->base.id, crtc->name);
ret = -EINVAL;
goto fail;
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH AUTOSEL 7.0-5.10] staging: fbtft: fix unchecked write return value in fb_agm1264k-fl
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amd/display: Use overlay cursor when color pipeline is active Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
7 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Artem Lytkin, Andy Shevchenko, Greg Kroah-Hartman, Sasha Levin,
dri-devel, linux-fbdev, linux-staging, linux-kernel
From: Artem Lytkin <iprintercanon@gmail.com>
[ Upstream commit f80760f5fc02c1ab384a974097964aa8e6720331 ]
The second call to par->fbtftops.write() does not capture the return
value, so the subsequent error check tests a stale value from the
first write call. Add the missing assignment so the error check
applies to the correct write operation.
Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Link: https://patch.msgid.link/20260207220523.3816-1-iprintercanon@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information to complete the analysis.
## Analysis
### PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1** - Subject parsing:
Record: Subsystem `staging: fbtft:`, action verb `fix`, summary:
"unchecked write return value in fb_agm1264k-fl"
**Step 1.2** - Tags:
Record:
- `Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>` (author)
- `Acked-by: Andy Shevchenko <andriy.shevchenko@intel.com>` (senior
reviewer, knowledgeable in staging/fbtft)
- `Link: https://patch.msgid.link/20260207220523.3816-1-
iprintercanon@gmail.com`
- `Signed-off-by: Greg Kroah-Hartman` (staging maintainer merged it)
- NO `Fixes:` tag, NO `Reported-by:`, NO `Cc: stable`, NO `Tested-by:`
**Step 1.3** - Commit body analysis:
Record: Author explains that the second `par->fbtftops.write()` call
does not assign its return to `ret`. The subsequent `if (ret < 0)` check
therefore tests a stale value from the first write. Fix adds the missing
assignment. Clear mechanism explanation, no stack trace, no version
info.
**Step 1.4** - Hidden bug detection:
Record: Not hidden; the commit is explicitly a "fix" and the title says
so. The mechanism description is honest and clear.
### PHASE 2: DIFF ANALYSIS
**Step 2.1** - Inventory:
Record: Single file `drivers/staging/fbtft/fb_agm1264k-fl.c`, +1/-1
line, single function `write_vmem()`, scope: surgical one-line fix.
**Step 2.2** - Code flow change:
Record: BEFORE: `par->fbtftops.write(par, buf, len);` — return value
discarded. The following `if (ret < 0)` tests stale `ret` from the prior
left-half write performed earlier in the same loop iteration.
AFTER: `ret = par->fbtftops.write(par, buf, len);` — return captured,
error check now applies to the correct call.
**Step 2.3** - Bug mechanism:
Record: Category (g) Logic/correctness fix — wrong variable (stale)
used; silent swallowing of errors returned by the write hook. If left
half succeeds (ret ≥ 0) and right half fails, the error is silently
ignored and `write_vmem()` returns 0 (success), causing the caller
`fbtft_update_display()` to also not log its error. Silent write failure
for the right half of the 128×64 LCD.
**Step 2.4** - Fix quality:
Record: Fix is obviously correct; zero chance of regression. It is
purely an error-reporting/propagation correction — no new branches, no
new locking, no ABI change.
### PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1** - git blame on the buggy line:
Record: The buggy line (379) was introduced by commit `b2ebd4be6fa1d2`
("staging: fbtft: add fb_agm1264k-fl driver") by Thomas Petazzoni,
2014-12-31 — first appearing in `v4.0-rc1`. Andy Shevchenko confirmed on
list: "it was like that from the day 1." The bug has been present for
~11 years in all stable trees that include this driver.
**Step 3.2** - Follow Fixes: tag:
Record: No Fixes: tag. Manually identified introducing commit
`b2ebd4be6fa1d2` via git blame. That commit is present in all stable
kernels since 4.0.
**Step 3.3** - File history:
Record: Recent changes to the file are almost all cleanups (BIT macro,
gpio descriptors, style). The line in question has been untouched since
2014. No series dependencies.
**Step 3.4** - Author context:
Record: Artem Lytkin has one other commit (`sm750fb: add missing
pci_release_region`) — also a staging bug fix. Not a maintainer; a
newcomer fixing real bugs. The Acked-by comes from Andy Shevchenko who
is the de facto staging/fbtft reviewer.
**Step 3.5** - Dependencies:
Record: None. `par->fbtftops.write` and `ret` exist unchanged in all
stable trees. Completely standalone, applies cleanly.
### PHASE 4: MAILING LIST RESEARCH
**Step 4.1** - b4 dig -c f80760f5fc02c:
Record: Matched by patch-id. Lore URL: https://lore.kernel.org/all/20260
207220523.3816-1-iprintercanon@gmail.com/. Only v1 of the patch was
submitted; no revisions.
**Step 4.2** - b4 dig -w (recipients):
Record: Artem Lytkin, Andy Shevchenko, Greg Kroah-Hartman, dri-devel,
linux-fbdev, linux-staging, linux-kernel — appropriate maintainer/list
coverage.
**Step 4.3** - Bug report search:
Record: No bug report link; no Reported-by; no syzbot. Bug was found by
code inspection.
**Step 4.4** - Series context:
Record: Single standalone patch. No series.
**Step 4.5** - Stable list:
Record: No stable mailing list discussion found. No reviewer explicitly
suggested Cc:stable; no one objected either. Andy's comment "it was like
that from the day 1" is an observation of longevity, not a NAK or
objection to stable.
### PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1** - Modified function:
Record: `write_vmem()` in `drivers/staging/fbtft/fb_agm1264k-fl.c`.
**Step 5.2** - Callers:
Record: `write_vmem` is the driver's `fbtftops.write_vmem` callback
(registered at line 432), called from `fbtft-core.c:272` in
`fbtft_update_display()` which in turn is called from the deferred-IO
workqueue when the framebuffer is dirtied by userspace writes.
**Step 5.3** - Callees:
Record: `par->fbtftops.write` → `write()` local function → bit-bangs
data onto GPIO lines. Failure path returns negative errno to
`write_vmem()`.
**Step 5.4** - Call chain / reachability:
Record: Userspace mmap/write to /dev/fb* → deferred IO →
`fbtft_update_display()` → `write_vmem()` → `par->fbtftops.write()`. The
buggy path is reached for every display refresh whenever `addr_win.xe >=
xres/2`, i.e. almost every update of any non-empty region.
**Step 5.5** - Similar patterns:
Record: Inspected sibling fbtft drivers (fb_uc1611, fb_ssd1306,
fb_pcd8544, etc.) — they call the central `fbtft_write_vmem16_bus8/9/16`
helpers and don't have this specific split-half bug. The bug is unique
to `fb_agm1264k-fl` because the AGM1264K-FL has two physically separate
64-column halves that must be written independently.
### PHASE 6: CROSS-REFERENCING STABLE TREES
**Step 6.1** - Code in stable:
Record: The driver was added in v4.0 (commit b2ebd4be6fa1d2, Dec 2014)
with the bug present. The buggy line has been textually unchanged since
then. Every stable tree that contains this driver (5.4, 5.10, 5.15, 6.1,
6.6, 6.12) has the bug.
**Step 6.2** - Backport complications:
Record: The file has had only cosmetic/stylistic changes since 2014. The
1-line change applies cleanly to all stable trees with no adjustments.
Expected: clean apply.
**Step 6.3** - Related fixes already in stable:
Record: No prior fix for this specific bug exists in stable.
### PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1** - Subsystem & criticality:
Record: `drivers/staging/fbtft/` — a staging framebuffer driver for
obscure small LCDs. Criticality: PERIPHERAL (used mainly by hobbyists
with the specific AGM1264K-FL 128×64 LCD).
**Step 7.2** - Activity:
Record: Moderately active — mostly cleanups, occasional real bug fixes
(e.g. `47d3949a9b04c` memory-leak fix in probe, `be26a07c61af5` build
failure fix). Staging/fbtft sees a steady trickle of commits.
### PHASE 8: IMPACT & RISK ASSESSMENT
**Step 8.1** - Affected users:
Record: Only users of the `fb_agm1264k-fl` driver
(CONFIG_FB_TFT_AGM1264K_FL), i.e., those with the AGM1264K-FL monochrome
LCD connected via GPIO. Niche hardware, likely a small number of users.
**Step 8.2** - Trigger conditions:
Record: Triggered whenever the underlying `par->fbtftops.write()` fails
on the right half of the display (I/O error on GPIO/SPI bus, allocation
failure in bit-bang helper, etc.). Failures of the write hook are rare
but real — they happen on transient hardware issues. No privilege
required (userspace framebuffer write eventually drives this).
**Step 8.3** - Failure mode severity:
Record: When a right-half write fails: (a) no dev_err logged, (b)
`write_vmem()` returns 0 falsely indicating success, (c)
`fbtft_update_display()` also suppresses the error. Net effect is silent
display corruption with no diagnostic trail. No crash, no memory
corruption, no security impact, no hang. Severity: LOW — pure error-
reporting/propagation bug; user-visible only as incorrect display output
without explanation.
**Step 8.4** - Risk-benefit:
Record: BENEFIT — low-moderate. Real users of this specific hardware
gain proper error diagnostics when writes fail. RISK — essentially zero.
The change is a one-line variable assignment in an error path; it cannot
introduce new behavior when writes succeed (ret still starts 0), and it
can only improve diagnostics when writes fail. No locking, no memory, no
ABI changes. Ratio strongly favors backporting.
### PHASE 9: FINAL SYNTHESIS
**Step 9.1** - Evidence:
FOR:
- Obviously correct 1-line fix
- Fixes a real bug (silent write failure, false success return)
- Acked by knowledgeable reviewer (Andy Shevchenko)
- Zero regression risk
- Applies cleanly to all stable trees back to v4.0
- Clear "fix" language in subject; mechanism explained in commit message
- Accepted by staging maintainer (Greg KH)
AGAINST:
- Staging driver (lower priority by convention)
- Obscure hardware (small user base)
- Bug is low severity (no crash/corruption/security)
- No Fixes: tag, no Reported-by:, no Cc: stable
- Bug has been dormant for 11 years with no user reports, indicating
hardware failures that reach this path are extremely rare
UNRESOLVED: None.
**Step 9.2** - Stable rules checklist:
1. Obviously correct and tested? YES — trivially correct; tested
implicitly by Acked-by and staging tree merge
2. Fixes a real bug? YES — silent error swallowing in driver error path
3. Important issue? MARGINAL — not a crash/corruption/security bug, just
bad diagnostics and a false success return
4. Small and contained? YES — 1 line, 1 file, 1 function
5. No new features/APIs? YES
6. Applies cleanly? YES — verified 11-year-unchanged context
**Step 9.3** - Exception category:
Not strictly an exception category (not a device ID, quirk, DT update,
build, or docs fix), so must stand on merits of being a legitimate bug
fix.
**Step 9.4** - Decision:
The fix is obviously correct, minimal (1 line), has zero regression
risk, and fixes a real if low-severity bug. The absence of
crash/security/corruption severity pushes this into borderline territory
for a staging driver; however, the "do no harm" principle of stable
(conservative about regressions) strongly favors inclusion because there
is literally no way this change can regress anything — it only changes
behavior when the write hook actually returns an error, and in that case
the new behavior is unambiguously better. This matches the profile of
fixes that AUTOSEL routinely accepts for staging subsystems.
## Verification
- [Phase 1] Read full commit message; confirmed tags: Artem Lytkin SOB,
Andy Shevchenko Acked-by, GKH SOB, Link to patch.msgid.link. No
Fixes/Reported-by/Cc:stable.
- [Phase 2] Read `drivers/staging/fbtft/fb_agm1264k-fl.c` lines 280-392;
confirmed `ret` initialized to 0 at line 283; left-half assigns `ret
=` (line 356); right-half lacks assignment (line 379); check on line
380 uses stale `ret`; function returns `ret` on line 391.
- [Phase 3.1] `git blame` on lines 370-390: line 379 (buggy code)
introduced in `b2ebd4be6fa1d2` on 2014-12-31.
- [Phase 3.1] `git describe --contains b2ebd4be6fa1d2` →
`v4.0-rc1~82^2~274`, confirming bug present since v4.0.
- [Phase 3.2] `git show b2ebd4be6fa1d2 --stat`: confirmed it is the
original driver add of 471 lines.
- [Phase 3.3] `git log --oneline --
drivers/staging/fbtft/fb_agm1264k-fl.c` showed only cosmetic changes
since 2014; line 379 untouched by any intermediate fix.
- [Phase 3.4] `git log --author="Artem Lytkin" --oneline` returned one
other commit (sm750fb pci_release_region fix) — author is a bug-hunter
in staging.
- [Phase 4.1] `b4 dig -c f80760f5fc02c` matched patch-id `a8ded4803c...`
→ lore thread https://lore.kernel.org/all/20260207220523.3816-1-
iprintercanon@gmail.com/.
- [Phase 4.1] `b4 dig -c f80760f5fc02c -a`: confirmed only v1 submitted;
no revisions.
- [Phase 4.1] Read the saved mbox at `/tmp/fbtft_thread.mbox`: confirmed
Andy Shevchenko's reply "Sounds about right, but it was like that from
the day 1. Acked-by: Andy Shevchenko". No NAKs, no stable nomination
request, no objections.
- [Phase 4.2] `b4 dig -c f80760f5fc02c -w`: confirmed CC list includes
Andy, GKH, dri-devel, linux-fbdev, linux-staging, LKML.
- [Phase 5] Grep for `write_vmem` across staging/fbtft: confirmed caller
is `fbtft-core.c:272` (`fbtft_update_display`), confirmed the callback
is registered as `.write_vmem = write_vmem` at line 432 of the driver.
- [Phase 5] Read `fbtft-core.c:270-276`: confirmed return value is only
used for dev_err logging; no propagation to userspace.
- [Phase 6] Confirmed via blame that the buggy line has been unchanged
since 2014; file is present and structurally similar across all stable
trees (5.4+).
- [Phase 7] Read Kconfig help: driver is "FB driver for the AGM1264K-FL
LCD display (two Samsung KS0108 compatible chips)" — confirmed two-
halves architecture that is the root cause of the split-write bug.
- [Phase 8] Failure mode verified by code inspection: silent error
swallowing + false success return; no crash/corruption/security
consequence.
- UNVERIFIED: The exact number of users running this driver in
production (unknowable); did not test runtime behavior on actual
hardware.
The commit is a trivial, obviously-correct, zero-risk one-line fix to a
real (if low-severity) bug present in every stable tree. It meets stable
kernel rules modulo the "important issue" criterion which is only weakly
satisfied, but the risk-benefit ratio overwhelmingly favors inclusion
because the fix cannot regress any working system.
**YES**
drivers/staging/fbtft/fb_agm1264k-fl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c b/drivers/staging/fbtft/fb_agm1264k-fl.c
index af2dbebefc72b..6fc8f4e9c814d 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -376,7 +376,7 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
/* write bitmap */
gpiod_set_value(par->RS, 1); /* RS->1 (data mode) */
- par->fbtftops.write(par, buf, len);
+ ret = par->fbtftops.write(par, buf, len);
if (ret < 0)
dev_err(par->info->device,
"write failed and returned: %d\n",
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-04-28 10:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260428104133.2858589-1-sashal@kernel.org>
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: fix CPER ring header parsing Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Pass min page size from SOC BB to dml2_1 plane config Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.18] drm/amdgpu: drop userq fence driver refs out of fence process() Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] fbdev: savage: fix probe-path EDID cleanup leaks Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-6.18] drm/amd/display: Fix HostVMMinPageSize unit mismatch in DML2.1 Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amdgpu: OR init_pte_flags into invalid leaf PTE updates Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0] drm/amd/display: Use overlay cursor when color pipeline is active Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] staging: fbtft: fix unchecked write return value in fb_agm1264k-fl Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox