Re: [PATCH v6 00/15] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types

[John Hubbard]

On 10/27/26 12:35 AM, Alexandre Courbot wrote:
> The upcoming r000 firmware support [1] introduces a new type of command
> named GMC alongside the existing and already supported RPC command type.
> 
> Since RPC was the only command type supported so far, it was embedded
> into the lowest command queue level: the message header actually
> includes the RPC header and the code considers them as one.
> 
> This makes the r000/GMC support difficult to land without a refactor,
> with the result of transport layer code being duplicated between the two
> message types, and no clear separation between the RPC and GMC code
> which are mixed together in the command queue. Merging the code that way
> would introduce quite some technical debt.
> 
> Thus, this series prepares a proper landing ground for a new message
> type by extracting all the RPC layer code and moving it into its own
> sub-modules, making the `cmdq` module completely agnostic of the type of
> message it transports.
> 
> With the transport and message layers properly separated, support for
> GMC can be added into sibling modules of `rpc`, and the transport layer
> can be converted once when doing the switch to r000.
> 
> This moves quite a bit of code around, but most of it is moved verbatim.
> No significant functional change is intended.
> 
> For the r000 series, this essentially means the following:
> 
> - Its patch 7 making `allocate_command` generic can be dropped as the
>   transport layer makes no assumption about the message type.
> - Patches adding GMC support will do it in dedicated `gmc` sub-modules,
>   following the model set by RPC. These `gmc` sub-modules will start as
>   `dead_code` as they are being built.
> - The "switch to r000" patch will essentially operate on the transport
>   layer level, i.e. `cmdq.rs`.
> - There should be no need for extra post-switch RPC fixing, as the
>   transport layer code will already be shared with GMC.
> 
> Overall, this should make the r000 series much easier to review and
> eventually land, while not creating new technical debt.
> 
> This revision fixes the latest nits while addressing a couple more
> issues found by Sashiko.
> 
> This series is based on `drm-rust-next`.
> 
> [1] https://lore.kernel.org/all/20260918010719.1176945-1-jhubbard@xxxxxxxxxx/
> 
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
> Changes in v6:
> - Add comment to `consume_element`'s documentation to disambiguate it.
>   (Eliot)
> - Add a patch to avoid sleeping in `drain` when reaching the end of the
>   queue. (Sashiko)
> - Fix badly-formatted import block. (Sashiko)
> - Link to v5: https://patch.msgid.link/20261020-cmdq-rpc-v5-0-dceeb8f360ec@xxxxxxxxxx

Hi Alex, looks like it is converging, are you planning to merge soon?
Or do you need more reviewers to weigh in?


thanks,
-- 
John Hubbard

> 
> Changes in v5:
> - Drop `MessageElement` and instead use a closure to process messages in
>   `consume_element`. This reduces the behavioral change of `await_msg`
>   and harmonizes it with the behavior of `drain` to the point where the
>   latter can also use `consume_element`. Eliot's R-b has
>   been dropped on that patch as the code has changed significantly.
> - Avoid unneeded temporary result variable. (Eliot)
> - Rename `await_msg` to `await_rpc_msg`. (Eliot)
> - Add patch splitting looping behavior of `await_msg` from its RPC
>   message handling.
> - Add patch factoring out `drain` code.
> - Add two doccomment fixes.
> - Link to v4: https://patch.msgid.link/20261009-cmdq-rpc-v4-0-c9ab8de1d3f2@xxxxxxxxxx
> 
> Changes in v4:
> - Rebase on top of latest drm-rust-next, including the interrupt series.
> - Move RPC parsing code to `RpcMessage`. (Eliot)
> - Do not mention the command queue slots in the public documentation. (Eliot)
> - Use different names for the length methods of the message and RPC
>   headers. (Eliot)
> - Fix bug where the RPC header length was used instead of the message
>   one to compute the number of pages to advance by. (Eliot)
> - Add a patch moving the message logging code to `RpcMessage`.
> - Make the `rpc` module only visible from `gsp`. (Eliot)
> - Link to v3: https://patch.msgid.link/20260930-cmdq-rpc-v3-0-91613f06520b@xxxxxxxxxx
> 
> Changes in v3:
> - Use dedicated `RpcMessage` type to distinguish parsed RPC message from
>   the transport-level `GspMessage`.
> - Introduce traits for sending and receiving elements on the command
>   queue (thanks Eliot!).
> - Split unsafe blocks, drop unused impls, and other fixes on patch 2.
>   (thanks Eliot!)
> - Squash the two move commits into a single one.
> - Link to v2: https://patch.msgid.link/20260927-cmdq-rpc-v2-0-c3f66ae73be4@xxxxxxxxxx
> 
> Changes in v2:
> - Fix checksum on receive incorrectly using the whole slots allocated
>   to the message instead of its actual length. (Sashiko)
> - Drop patch 1 to avoid having to validate the `elemCount` header
>   member against edge cases. (Sashiko)
> - Fix incorrect layer separation in receive path, where the CPU read
>   pointer was advanced in the message layer.
> - Move `log_event` into a method of `RpcMessage`.
> - Make the `rpc` module only visible to `gsp` and its submodules.
> - Fix doclinks pointing to old function names. (Sashiko)
> - Link to v1: https://patch.msgid.link/20260927-cmdq-rpc-v1-0-822db5af910e@xxxxxxxxxx
> 
> ---
> Alexandre Courbot (15):
>       gpu: nova-core: gsp: cmdq: remove stale comment
>       gpu: nova-core: gsp: cmdq: simplify framing error description
>       gpu: nova-core: gsp: cmdq: validate checksum earlier on receive
>       gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type
>       gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command
>       gpu: nova-core: gsp: cmdq: split the transport part of the send path
>       gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path
>       gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage
>       gpu: nova-core: gsp: cmdq: split the transport part of the receive path
>       gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg
>       gpu: nova-core: gsp: cmdq: do not sleep when waiting for a message with timeout 0
>       gpu: nova-core: gsp: cmdq: use consume_element to implement drain
>       gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module
>       gpu: nova-core: gsp: move the RPC commands into a sub-module
>       gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods
> 
>  Documentation/gpu/nova/core/interrupts.rst         |   7 +-
>  drivers/gpu/nova-core/api.rs                       |   2 +-
>  drivers/gpu/nova-core/gpu.rs                       |   2 +-
>  drivers/gpu/nova-core/gsp.rs                       |   4 +-
>  drivers/gpu/nova-core/gsp/boot.rs                  |  12 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs                  | 486 +++++----------------
>  drivers/gpu/nova-core/gsp/cmdq/rpc.rs              | 415 ++++++++++++++++++
>  .../nova-core/gsp/cmdq/{ => rpc}/continuation.rs   |   0
>  drivers/gpu/nova-core/gsp/commands.rs              | 329 +-------------
>  drivers/gpu/nova-core/gsp/commands/rpc.rs          | 333 ++++++++++++++
>  drivers/gpu/nova-core/gsp/fw.rs                    | 123 +++---
>  drivers/gpu/nova-core/gsp/sequencer.rs             |   6 +-
>  12 files changed, 947 insertions(+), 772 deletions(-)
> ---
> base-commit: 3497a403264b094ebd61b8b755165d56e7d9dc37
> change-id: 20260927-cmdq-rpc-ed5f850b207e
> 
> Best regards,
> --  
> Alexandre Courbot <acourbot@xxxxxxxxxx>