Skip to content

dmaengine: qcom: gpi: order event ring pointer reads - #45

Closed
ooaklee wants to merge 1 commit into
sp11/ubuntu-qcom-x1e-7.2.y-betafrom
sp11/beta-gpi-event-ordering
Closed

ooaklee wants to merge 1 commit into
sp11/ubuntu-qcom-x1e-7.2.y-betafrom
sp11/beta-gpi-event-ordering

Conversation

@ooaklee

@ooaklee ooaklee commented Sep 3, 2026 •

Copy link
Copy Markdown
Owner

Summary

Use ordered MMIO reads for the GPI event-ring read pointer before consuming
events written by the device into coherent memory.

This is a one-variable replacement for the withdrawn early-EOT experiment in
#42. It does not change QSPI completion policy, descriptor ownership, SPI HID,
the MSHW0485 driver, recovery, runtime PM, or userspace.

Evidence boundary

Proven from the failed beta boots:

  • beta2 and v6 repeatedly accepted a QSPI RX completion with descriptor length
    3628 and reported residue 3628, so the event length was observed as zero;
  • v19 on the same Surface Pro 11 has not produced that pair failure;
  • v7's additional early-completion path turned the existing failure into an
    invalid-cookie kernel BUG, so that patch and its aggregate were withdrawn.

The missing read-side ordering is a hypothesis, not yet a hardware-qualified
root cause. The GPI driver reads the device producer pointer with
readl_relaxed() and immediately reads dma_alloc_coherent() event memory.
Linux's MMIO contract reserves ordered readl() for exactly this relationship.
The analogous Qualcomm IPA GSI event-ring path also uses ordered ioread32().

Scope and guardrail

  • only gpi_read_ev_rp() changes;
  • all unrelated register accesses remain relaxed;
  • the shared helper covers both gpi_process_events() and
    gpi_mark_stale_events();
  • both halves of Denali's retained 64-bit pointer are read with the same ordered
    accessor;
  • no module parameter, timing delay, logging dependency, zero-length-event
    suppression, or cookie workaround is introduced.

The change is generic because ordering a device publication pointer before its
coherent payload is a GPI core correctness requirement, not Denali hardware
policy. SP11 qualification can validate the exposed QSPI case; wider upstream
submission will still require review and regression coverage on other GPI
clients.

Validation

  • git diff --check: pass
  • strict patch checkpatch.pl: 0 errors, 0 warnings, 0 checks
  • ARM64 drivers/dma/qcom/gpi.o build using the v7 configuration: pass
  • two adversarial OpenCode passes: no critical, high, or medium findings after
    correcting the MMIO/DMA and split-pointer analysis

Current-head evidence

  • exact-current-head integration check on beta bf631f9: pass
  • exact-source v10 ARM64 Lexr build and four-package inspection at e71361d: pass; the packaged GPI module has exact target ABI vermagic

Hardware qualification

Keep this PR draft and unmerged until a touch-only beta build passes on the
Surface Pro 11 with tracing disabled:

  1. cold boot and confirm the expected DTB and ABI;
  2. sustain touchscreen and pen use well beyond the former three-second failure;
  3. repeat suspend/resume and HIDRAW recovery;
  4. verify no SP11 QSPI pair failed, zero-length completion, DMA cookie BUG,
    host-fault loop, or loss of the IPTS bridge;
  5. soak long enough to exercise repeated transfers without diagnostic timing
    overhead.

Failure with the same residue signature falsifies this hypothesis and this PR
must be closed rather than expanded in place.

Merge policy

Do not merge on compile evidence alone. Do not combine this test with the
speaker repair or any other unqualified topic.

Qualified-source transport note

The qualified source also contains an exported qcom_geni_spi_biosref_xfer() helper and public header, but its final tree has no caller; mshw0485_touch uses normal spi_sync() messages. Those dead helper artifacts are not part of the guarded relift. The active protocol-9 behavior lives in the beta branch scoped qcom,geni-qspi / QCOM_GPI_QSPI route; this PR changes only the ordering used when that GPI path consumes its event ring.

@ooaklee

ooaklee commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Qualification bundle is ready in draft PR #46.

  • exact source revision: 8fbab0760dca728ccedcd9e9eb9de2a314ea4851
  • source tree: 04d9ec207dcbd8a33aa6803249917495fcaf3e00
  • isolated Lexr --reset-source build: pass
  • ABI: 7.2.2-jg-0sp11v8-qcom-x1e
  • packages: four; all SHA256SUMS entries pass
  • Lexr kernel inspect --json: pass
  • modules package contains gpi.ko.zst and both declared Surface DTBs

The full Ubuntu configuration emitted five non-fatal warnings in unrelated,
unmodified UCSI, Mellanox, and WireGuard code. No warning involved GPI, SPI,
HID, IPTS, device tree, or packaging.

PR #46 records the controlled hardware protocol. Because software mute did not
contain the failed v7 audio static, the first v8 boot will transiently blacklist
the X1E machine/SoundWire modules through a one-time GRUB edit. A pass therefore
qualifies touch transport under audio isolation only and remains merely
consistent with the ordering hypothesis; it is not final combined-beta
qualification.

Keep this PR draft and unmerged until the physical gates in #46 pass.

@ooaklee

ooaklee commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Message-only submission cleanup

The commit message was wrapped to satisfy full-patch checkpatch.pl; source tree 151890649ef2faa531f087e76e06afadb7b23a2a, author identity, and author date are unchanged. Current head: f35a9f804bdf3666820737d1a4d0df7bd1b82249. Full patch and per-commit source checks now pass with zero findings.

The device writes event elements into coherent memory before publishing
its event ring read pointer. Both gpi_process_events() and
gpi_mark_stale_events() read that pointer through readl_relaxed() and then
consume the event memory without an ordering guarantee.

On architectures such as arm64, this can expose a published event before
all its fields are visible. Denali QSPI exposed this as an accepted
completion whose length was observed as zero.

Use ordered MMIO reads only for the event pointer. Keep all other register
accesses relaxed. This follows the ordered ioread32() pattern used by the
Qualcomm IPA GSI event-ring consumer.

Fixes: 5d0c353 ("dmaengine: qcom: Add GPI dma driver")
Signed-off-by: Leon Silcott <leon@boasi.io>
@ooaklee

ooaklee commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

September 29: isolated 7.2.5 review port published

The GPI ordering port is available on x/sp11-gpi-7.2.5-20260928 at f9c68b9c62f651652bfe6cbe7de0f1c18effd63e, based on the refresh branch at bdae86ee3b4e. This PR's existing head and diff are unchanged; the published ref is an experimental review candidate, not a replacement accepted by implication.

See the pinned-base mapping and current qualification checkpoint for replay/build evidence, dependencies and the actual v1 hardware result. No branch was force-updated, merged or deleted, and no duplicate PR was opened. Hardware and non-SP11 qualification remain outstanding.

@ooaklee

ooaklee commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

Closing this superseded review/qualification vehicle without merging. Current review work is tracked in ooaklee/linux_ms_dev_kit-sp11#61; ooaklee/linux_ms_dev_kit-sp11#68 is the single combined 7.2.5 qualification candidate. This does not discard the original feature intent or close any outstanding acceptance gate.

The current combined source is 8ba2b75713feb5c72bbc11dd8cce4048e23cdeea, ABI 7.2.5-jg-0sp11v8-qcom-x1e; see the experimental v8 release. Ordinary-use results on one X1E/OLED unit are recorded, but the release remains experimental and the detailed hardware, lifecycle, deviation, CI and provenance gates in ooaklee/linux_ms_dev_kit-sp11#68 remain open.

This PR's branch and exact head bd9f5e29b2a8, authored history, discussion and test evidence are retained unchanged. Closing is administrative supersession, not a merge, an acceptance decision, or a claim that the old and current trees are identical. Beta and the repository default are unchanged.

The two additional deferred GPI corrections are explicitly deferred, not removed from an accepted fix list. V8's existing working GPI/touch implementation remains intact; the known residual risks and separate width-cast disposition are not waived.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant