Skip to content

drm/asahi: last_render/last_compute are swapped in submit(), so JobMeta.flush_stamps is never set (LLM-assisted report) #633

Description

@afrog33k

Disclosure (per the Asahi Linux Generative AI Policy): this report was drafted with an LLM (Claude) during my own performance work on the driver. The LLM read the public source of queue/mod.rs on the asahi branch and wrote up the mismatch below; no traces, hypervisor output, or Apple material were involved. I have re-checked the line references by hand against tip 77cb8f24. If the policy means this should be closed, I understand — the two-line finding stands on its own for anyone who wants to re-derive it.

The bug

In drivers/gpu/drm/asahi/queue/mod.rs, submit() makes two passes over the command buffer. The first pass is meant to remember the index of the last RENDER command and the last COMPUTE command, so the second pass can set flush_stamps on those commands.

The first pass stores them in the wrong variables:

// queue/mod.rs:704-710 (asahi @ 77cb8f24)
uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_RENDER => {
    last_compute = nr_commands;   // <- a RENDER index goes into last_compute
    nr_render += 1;
}
uapi::drm_asahi_cmd_type_DRM_ASAHI_CMD_COMPUTE => {
    last_render = nr_commands;    // <- a COMPUTE index goes into last_render
    nr_compute += 1;
}

The second pass then compares the right way round:

// queue/mod.rs:837 (render) and :861 (compute)
command_index == last_render,    // passed as flush_stamps to submit_render
command_index == last_compute,   // passed as flush_stamps to submit_compute

Command indices are unique, so a RENDER command's index can never equal a COMPUTE command's index (and command_index starts at 1, so it never equals the 0 default either). Both comparisons are always false.

Effect

flush_stamps is false for every command of every submission, so JobMeta.flush_stamps is always sent to the firmware as 0 (queue/render.rs and queue/compute.rs write it straight into JobMeta; fw/job.rs has no other writer). The behaviour the comment above the first pass describes — flushing stamps on the last command of each subqueue — never happens.

The driver has shipped like this since the initial import (bdcc11ea1949), so current firmware evidently copes without the flag. I have no evidence of a functional problem; this is a dead-code / intent-mismatch report. Whether enabling the intended behaviour changes anything in the firmware is something I can't judge from the driver side.

Fix

Swap the two assignments in the first pass so each variable records its own command type:

                 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;
                 }

I'm not submitting this as a patch, given the policy; happy for a maintainer to take it from here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions