drm/asahi: last_compute/last_render are assigned to the wrong commands in submit() — JobMeta.flush_stamps can never be set

[afrog33k]

DRAFT upstream bug report — drm/asahi flush_stamps dead code

Status: draft only, NOT filed. Verified against asahi branch tip
77cb8f24c2381a8abb7272d7bbdec548d6426a8a on 2026-09-20 (fetch of
https://raw.githubusercontent.com/AsahiLinux/linux/asahi/drivers/gpu/drm/asahi/queue/mod.rs).
Raw fetched sources (plus a line-numbered queue/mod.rs) are preserved at
zeus:/home/reckon/agx-inject-20260919/parallel-r1/UpstreamBugDraft/.
No GitHub search hits for flush_stamps in AsahiLinux/linux issues or commit messages, so it
appears unreported. File the text below (minus this header) as a GitHub issue when approved.

---

Title

drm/asahi: last_compute/last_render are assigned to the wrong commands in submit() — JobMeta.flush_stamps can never be set

Summary

In drivers/gpu/drm/asahi/queue/mod.rs, the first parsing pass over a submission's command
buffer records the index of the last command of each hardware subqueue — last_render under
DRM_ASAHI_CMD_RENDER and last_compute under DRM_ASAHI_CMD_COMPUTE — but the two
assignments are swapped. The second pass then compares command_index against
last_render in the render branch and against last_compute in the compute branch. Because
command indices are unique and strictly increasing, both comparisons are unsatisfiable: the
flush_stamps argument is false for every render and compute command of every submission,
including all-compute and all-render chains. As a result fw::job::raw::JobMeta.flush_stamps
is always submitted as 0 and the "flush stamps on the last command of a subqueue" behavior
the code comment describes never occurs. The bug has been present since the driver's initial
import (bdcc11ea1949 — "drm/asahi: Add the Asahi driver for Apple AGX GPUs"); queue/mod.rs
has no later commits.

Evidence

Pass 1 — queue/mod.rs:685-714 (comment :685-691, declarations :692-696, swapped
assignments at :705 and :709), asahi tip 77cb8f2:

        // First, parse the headers to determine the number of compute/render
        // commands. This will be used to determine when to flush stamps.
        //
        // We also use it to determine how many notifications the job will
        // generate. We could calculate that in the second pass since we don't
        // need until much later, but it's convenient to gather everything at
        // the same time.
        let mut nr_commands = 0;
        let mut last_compute = 0;
        let mut last_render = 0;
        let mut nr_render = 0;
        let mut nr_compute = 0;

        while !cmdbuf.is_empty() {
            let header: uapi::drm_asahi_cmd_header = cmdbuf.read()?;
            cmdbuf.skip(header.size as usize);
            nr_commands += 1;

            match header.cmd_type as u32 {
                uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_RENDER => {
                    last_compute = nr_commands;
                    nr_render += 1;
                }
                uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_COMPUTE => {
                    last_render = nr_commands;
                    nr_compute += 1;
                }
                _ => {}
            }
        }

Note: the RENDER arm updates last_compute (:705) and the COMPUTE arm updates
last_render (:709) — each variable records the other command type's last index.

Pass 2 — queue/mod.rs:826-862 (predicates at :837 and :861):

            match header.cmd_type as u32 {
                uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_RENDER => {
                    let render: uapi::drm_asahi_cmd_render = cmdbuf.read_up_to(header_size)?;

                    self.inner.submit_render(
                        &mut job,
                        &render,
                        &vertex_attachments,
                        &fragment_attachments,
                        objects,
                        id,
                        command_index == last_render,
                    )?;
                    ...
                }
                uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_COMPUTE => {
                    let compute: uapi::drm_asahi_cmd_compute = cmdbuf.read_up_to(header_size)?;

                    self.inner.submit_compute(
                        &mut job,
                        &compute,
                        &compute_attachments,
                        objects,
                        id,
                        command_index == last_compute,
                    )?;

(command_index is declared at :774 and pre-incremented per header at :784-785, so it runs
over the same 1..N values as nr_commands; the comment there reads
// Pre-increment command index to match last_compute/last_render.)

Why both predicates are always false. nr_commands and command_index take each value
1..N exactly once, so indices are unique. After pass 1, last_compute holds the index of a
RENDER command (or 0 if there is none) and last_render holds the index of a COMPUTE command
(or 0). The render branch tests command_index == last_render, which would require the
current RENDER command's index to equal some COMPUTE command's index (or 0, unreachable since
command_index >= 1) — impossible. The compute branch is symmetric. Therefore:

• submit_render is never called with flush_stamps == true, and
• submit_compute is never called with flush_stamps == true,

including the all-compute case (where last_compute stays 0 and the render branch is never
entered anyway) and the all-render case (where last_render stays 0).

Consumers. queue/mod.rs:830 and queue/mod.rs:855 are the only call sites of
submit_render/submit_compute in the tree. The flag lands in the firmware JobMeta:

• queue/render.rs:185-194 — pub(super) fn submit_render(..., flush_stamps: bool);
  written at :911 (fragment JobMeta) and :1352 (vertex JobMeta), each as
  flush_stamps: flush_stamps as u32,
• queue/compute.rs:38-46 — pub(super) fn submit_compute(..., flush_stamps: bool);
  written at :342, flush_stamps: flush_stamps as u32,
• fw/job.rs:19-31 — raw::JobMeta field pub(crate) flush_stamps: u32 (:28), a
  #[repr(C)] struct passed to the Apple GS firmware, with no other in-tree writer or reader.

Likely intent

Per the pass-1 comment ("This will be used to determine when to flush stamps"), the intended
semantics are that the last command of each hardware subqueue flushes its stamps: the
final RENDER command of a submission sets flush_stamps on its vertex and fragment
JobMetas, and the final COMPUTE command sets it on its compute JobMeta. The pass-2
comparisons (submit_render ← last_render, submit_compute ← last_compute) match that
intent; it is the pass-1 assignments that are crossed. The swapped pattern (RENDER→
last_compute, COMPUTE→last_render) suggests a copy-paste slip in the initial driver
import.

Impact

The flag is provably dead: no submission can ever set JobMeta.flush_stamps, so the firmware
always receives 0 in that field. Since the driver has shipped this way since its first import
without completion-related failures attributed to it, current firmware evidently flushes
completion stamps regardless (or treats the field as advisory), so there is no known
functional regression on supported firmware. The concrete consequences are that the intended
"flush only on subqueue-final commands" behavior never happens — which may matter for
per-command overhead (stamp flushes are per-job work the firmware could skip for
non-final commands) and would matter on any firmware that honors the flag strictly, where
stamps might never be flushed for a subqueue. Firmware-side semantics of the field are
reverse-engineered (the field has no documentation in-tree), so this assessment of runtime
effect is necessarily inferential; the dead-code status of the flag is not.

Suggested fix

Swap the two assignments in pass 1 so each variable records its own command type, matching
both the variable names and the pass-2 comparisons:

--- a/drivers/gpu/drm/asahi/queue/mod.rs
+++ b/drivers/gpu/drm/asahi/queue/mod.rs
@@ -702,13 +702,13 @@ impl Queue for Queue::ver {
             match header.cmd_type as u32 {
                 uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_RENDER => {
-                    last_compute = nr_commands;
+                    last_render = nr_commands;
                     nr_render += 1;
                 }
                 uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_COMPUTE => {
-                    last_render = nr_commands;
+                    last_compute = nr_commands;
                     nr_compute += 1;
                 }
                 _ => {}

(Equivalently, swapping the two pass-2 comparisons instead is behaviorally identical; fixing
pass 1 keeps the names meaningful.) With this change, flush_stamps becomes true exactly on
the last RENDER command (applied to both its vertex and fragment JobMetas at
render.rs:1352 and :911) and on the last COMPUTE command (compute.rs:342). Worth
confirming against the firmware behavior before merging: if the firmware requires a final
flush for completion events on some versions, enabling the intended path is a behavior
change; if it flushes unconditionally, this is a correctness/dead-code cleanup only.

Credit

Found during independent performance work on M2 Ultra decode (drm/asahi submission-path
analysis, Qwen3-4B Q4_0 decode benchmarking on an M2 Ultra running the Asahi driver stack).