Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
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


             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