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/[email protected]/ > > Signed-off-by: Alexandre Courbot <[email protected]> > --- > 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/[email protected]
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/[email protected] > > 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/[email protected] > > 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/[email protected] > > 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/[email protected] > > --- > 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 <[email protected]>
