From: Junhua Shen <Junhua.Shen@amd.com>
To: <igt-dev@lists.freedesktop.org>
Cc: Vitaly Prosyak <vitaly.prosyak@amd.com>,
Jesse Zhang <Jesse.Zhang@amd.com>,
Sunil Khatri <sunil.khatri@amd.com>,
Honglei Huang <honglei1.huang@amd.com>,
Huang Rui <ray.huang@amd.com>, Yiru Ma <yiru.ma@amd.com>,
Junhua Shen <Junhua.Shen@amd.com>
Subject: [PATCH i-g-t v2 0/6] lib/amdgpu: refactor cmd_context ownership and packet operands
Date: Wed, 9 Sep 2026 18:45:56 +0800 [thread overview]
Message-ID: <20260909104602.13807-1-Junhua.Shen@amd.com> (raw)
This series refactors the amdgpu command-submission helper library
(lib/amdgpu/amd_command_submission.*) around a single principle:
cmd_context should own only the PM4/IB command stream, while the buffers
those commands read and write, together with their GPU virtual addresses,
are supplied and kept resident by the caller.
Currently cmd_context both builds the PM4 stream and allocates and clears
an internal data BO, and the IB is sized from the data transfer size
(write_length) rather than from the command-stream capacity (pm4_size).
This couples unrelated concerns, hides residency management from callers,
and leaves the IB size ambiguous. The series separates these concerns
incrementally while keeping every existing caller compiling and
functional.
v2 also folds in two independent test-correctness fixes:
patch 5 derives the compute-dispatch version from
hw_ip_version_major (in amd_mem and amd_remote_mem), which surfaced while
reworking amd_mem's compute path; patch 6 fixes the KFD aperture
node-count query in amd_kfd_dmabuf_unload that the kernel rejects with
-EINVAL.
Scope: cmd_context is a convenience wrapper for SDMA data operations
(linear write/copy/const-fill/atomic). Only three tests currently use it
(amd_mem, amd_dmabuf_unload, amd_kfd_dmabuf_unload); the rest of the
amdgpu tests deliberately drive libdrm_amdgpu directly to validate the
kernel submission UAPI (cs_submit, bo_list, syncobj, gang, compute
dispatch), and are left untouched by the cmd_context refactor. The one
exception is the standalone compute-dispatch-version fix in patch 5, which
also updates amd_remote_mem; it is unrelated to the cmd_context contract.
The ring_context field usage in those low-level callers is already
consistent with the contract documented here, so no changes are required
on their side.
Overview of the changes
-----------------------
1. document amdgpu_ring_context field contract
Add IN/OUT/INTERNAL annotations to struct amdgpu_ring_context so the
ownership and direction of each field is unambiguous before the
behavioural changes are introduced. Documentation only, no functional
change.
2. drop external BO from cmd_context
cmd_context no longer allocates, tracks, or clears a caller-visible
data BO. Callers now register the residency of those buffers
explicitly via the new cmd_context_add_resource(). cmd_context_create()
loses the write_length parameter (the IB is sized from pm4_size alone)
and all call sites are updated (amd_dmabuf_unload.c,
amd_kfd_dmabuf_unload.c, amd_mem.c).
3. source packet operands from params and size the IB from pm4_size
The legacy IB byte size is now derived from pm4_size (the command
stream capacity), not write_length (the data transfer size).
WRITE_LINEAR, WRITE_ATOMIC and COPY_LINEAR take their source and
destination GPU virtual addresses from cmd_packet_params_t, so callers
pass those addresses directly rather than relying on context-internal
BOs.
4. add CONST_FILL packet type and pass caller fill value through
Add a CMD_PACKET_CONST_FILL packet type with a per-packet size guard,
and propagate the caller-supplied fill value
(write_data/write_data_valid) through write_linear and const_fill in
the IP block builders.
5. derive compute dispatch version from hw_ip_version_major
amd_mem's ptrace VRAM verification derived the GFX generation from
family_id; query amdgpu_query_hw_ip_info() and use
hw_ip_version_major instead, mirroring amd_dispatch.c, so no
wrong-generation PM4 is emitted. amd_remote_mem's build_poll_shader()
carried the same family_id-based version table and is converted to the
query in the same patch. The cache-invalidate tests are also gated to
Arcturus, where the RW MTYPE cache behaviour they exercise is
specific.
6. query KFD aperture node count before filling array
kfd_get_gpuvm_base() passed num_of_nodes = NUM_OF_SUPPORTED_GPUS
unconditionally, which the kernel rejects with -EINVAL. Follow the
documented two-step protocol: an initial call with num_of_nodes == 0
returns the actual node count, then fill up to that count.
Changes since v1
----------------
v1: https://lists.freedesktop.org/archives/igt-dev/ (cover: "[PATCH i-g-t
v1 0/4] lib/amdgpu: refactor cmd_context ownership and packet operands").
- Patch 1 (document ring_context): add the commit message body that was
missing in v1 (builder/submitter roles and the IN/OUT/INTERNAL
rationale). The annotations themselves are unchanged.
- Patch 2 (drop external BO): address Jesse Zhang's review of v1 2/4.
cmd_submit_packet() now propagates cmd_wait_completion() errors instead
of unconditionally clearing submit_pending and reusing the IB;
submit_pending is cleared inside cmd_wait_completion() on success to
avoid redundant waits; cmd_context_destroy() drains any pending
submission before freeing the IB. The two amd_mem submit paths the
reviewer flagged now register residency explicitly:
test_cache_invalidate_on_sdma_write_asm() calls
cmd_context_add_resource(dma_ctx, bo_vram) before the write, and
submit_dma_write_operation() registers ctx->dma_scratch_bo and writes
to its VA rather than the removed internal data BO. The internal helper
cmd_reset_packet_operands() is renamed cmd_clear_packet_inputs().
- Patch 3 (source packet operands / size IB from pm4_size): unchanged
from v1 3/4.
- Patch 4 (CONST_FILL): rebased onto the renamed helper; no functional
change from v1 4/4.
- New patch 5 (tests/amdgpu: derive compute dispatch version from
hw_ip_version_major): test-correctness fix uncovered while reworking
amd_mem's compute path. The same family_id-based version derivation in
amd_remote_mem's build_poll_shader() is switched to
amdgpu_query_hw_ip_info() in this patch as well.
- New patch 6 (tests/amdgpu: query KFD aperture node count before filling
array): test-correctness fix for the -EINVAL rejection from
kfd_ioctl_get_process_apertures_new().
Testing
-------
Ran on local hardware: AMD GFX1036 (GFX10_3). The three cmd_context
callers (amd_mem, amd_dmabuf_unload, amd_kfd_dmabuf_unload) and the
low-level tests that exercise the changed shared code (amd_basic SDMA and
compute submission, amd_userptr_invalidation) all pass. There is no
regression from this series.
Known limitations / follow-up work
----------------------------------
The following items are intentionally excluded from this series to limit
its scope; they are candidates for follow-up patches:
- pm4_size is currently a hard-coded 256 dwords in cmd_context_create().
It should become a caller-provided parameter (or be derived from the
expected packet mix) so large PM4 streams are not silently truncated.
- The residency set (amdgpu_ring_context.resources[]) is a fixed-size
array (4 entries). cmd_context_add_resource() returns -ENOSPC when it
overflows; a growable set would remove that limit for callers that
require more operands per submit.
- Some low-level callers open-code the same linear write/copy/fill
sequences that cmd_context now wraps. Where a test only needs the data
operation itself (and is not specifically exercising the submission
UAPI), it could be migrated onto the high-level cmd_context API to
remove boilerplate. Tests that deliberately drive libdrm_amdgpu to
validate cs_submit/bo_list/syncobj/gang/compute paths should stay as
they are.
Junhua Shen (6):
lib/amdgpu: document amdgpu_ring_context field contract
lib/amdgpu: drop external BO from cmd_context
lib/amdgpu: source packet operands from params and size the IB from
pm4_size
lib/amdgpu: add CONST_FILL packet type and pass caller fill value
through
tests/amdgpu: derive compute dispatch version from hw_ip_version_major
tests/amdgpu: query KFD aperture node count before filling array
lib/amdgpu/amd_command_submission.c | 424 +++++++++++++++++++----------------
lib/amdgpu/amd_command_submission.h | 102 ++++++++-
lib/amdgpu/amd_ip_blocks.c | 20 +-
lib/amdgpu/amd_ip_blocks.h | 59 ++++-
tests/amdgpu/amd_dmabuf_unload.c | 15 +-
tests/amdgpu/amd_kfd_dmabuf_unload.c | 23 +-
tests/amdgpu/amd_mem.c | 171 +++++++-------
tests/amdgpu/amd_remote_mem.c | 20 +-
8 files changed, 492 insertions(+), 342 deletions(-)
--
2.34.1
next reply other threads:[~2026-09-09 10:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-09 10:45 Junhua Shen [this message]
2026-09-09 10:45 ` [PATCH i-g-t v2 1/6] lib/amdgpu: document amdgpu_ring_context field contract Junhua Shen
2026-09-09 10:45 ` [PATCH i-g-t v2 2/6] lib/amdgpu: drop external BO from cmd_context Junhua Shen
2026-09-14 2:18 ` Zhang, Jesse(Jie)
2026-09-09 10:45 ` [PATCH i-g-t v2 3/6] lib/amdgpu: source packet operands from params and size the IB from pm4_size Junhua Shen
2026-09-09 10:46 ` [PATCH i-g-t v2 4/6] lib/amdgpu: add CONST_FILL packet type and pass caller fill value through Junhua Shen
2026-09-09 10:46 ` [PATCH i-g-t v2 5/6] tests/amdgpu: derive compute dispatch version from hw_ip_version_major Junhua Shen
2026-09-09 10:46 ` [PATCH i-g-t v2 6/6] tests/amdgpu: query KFD aperture node count before filling array Junhua Shen
2026-09-09 18:19 ` ✓ i915.CI.BAT: success for lib/amdgpu: refactor cmd_context ownership and packet operands (rev2) Patchwork
2026-09-09 18:20 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-10 3:38 ` ✓ Xe.CI.FULL: " Patchwork
2026-09-10 11:49 ` ✗ i915.CI.Full: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260909104602.13807-1-Junhua.Shen@amd.com \
--to=junhua.shen@amd.com \
--cc=Jesse.Zhang@amd.com \
--cc=honglei1.huang@amd.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=ray.huang@amd.com \
--cc=sunil.khatri@amd.com \
--cc=vitaly.prosyak@amd.com \
--cc=yiru.ma@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox