Re: [PATCH v3 4/9] gpu: nova-core: gsp: cmdq: split the transport part of the send path

[Eliot Courtney]

On Wed Sep 30, 2026 at 11:55 PM JST, Alexandre Courbot wrote:
> Introduce the `CommandElement` trait and `RpcCommandElement` wrapper
> type to define how messages (independently of their type) are sent
> through the transport layer. `send_single_command` becomes
> `send_command_element`, which allocates the queue slots, writes the
> element header, and delegates the writing of the message itself to the
> implementation of `CommandElement` before computing the checksum,
> advancing the write pointer and ringing the doorbell.
>
> The RPC part of `send_single_command` (writing the RPC header and the
> command payload) is now part of the `CommandElement` implementation.
> This sets things up for moving the RPC code into its own sub-module,
> leaving the transport agnostic of the message type.
>
> No functional change intended.
>
> Suggested-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 133 +++++++++++++++++++++++---------------
>  1 file changed, 80 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index bda79f23626d..d8a7716fc500 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -35,10 +35,7 @@
>      },
>  };
>  
> -use continuation::{
> -    ContinuationRecord,
> -    SplitState, //
> -};
> +use continuation::SplitState;
>  
>  use pin_init::pin_init_scope;
>  
> @@ -67,6 +64,19 @@
>  /// reply type are sent using [`Cmdq::send_command_no_wait`].
>  pub(crate) struct NoReply;
>  
> +/// Trait implemented by types that can be sent as a single command queue element.
> +///
> +/// The command queue allocates `size()` bytes after the `GspMsgElement` header and calls `write()`
> +/// to fill them.
> +trait CommandElement {
> +    /// Size in bytes of the element, not including the `GspMsgElement` header.
> +    fn size(&self) -> usize;
> +
> +    /// Writes the contents of the command into `dst`. `dev` is the queue's device (to be used for
> +    /// logging), `seq` is the sequence number of the element.
> +    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result;
> +}
> +
>  /// Trait implemented by types representing a command to send to the GSP.
>  ///
>  /// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
> @@ -129,6 +139,62 @@ fn size(&self) -> usize {
>      }
>  }
>  
> +/// Wrapper type for sending a RPC command as a command queue element.
> +///
> +/// [`CommandElement`] cannot be directly implemented for all [`CommandToGsp`] with a blanket
> +/// implementation as it would conflict with other future command types.
> +struct RpcCommandElement<M>(M);
> +
> +impl<M> CommandElement for RpcCommandElement<M>
> +where
> +    M: CommandToGsp,
> +    Error: From<M::InitError>,
> +{
> +    fn size(&self) -> usize {
> +        self.0.size()
> +    }
> +
> +    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result {
> +        let command = &self.0;
> +        let size_in_bytes = command.size();
> +        // Extract area for the command itself. The GSP message header and the command header
> +        // together are guaranteed to fit entirely into a single page, so it's ok to only look
> +        // at `dst.contents.0` here.
> +        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
> +        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
> +        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
> +        // initializer fails.
> +        unsafe {
> +            pin_init::raw_try_init(
> +                core::ptr::from_mut(dst.header.rpc_header_mut()),
> +                rpc_header_init,
> +            )?;
> +        }
> +        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
> +        unsafe {
> +            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
> +        }
> +
> +        // Fill the variable-length payload, which may be empty.
> +        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
> +        command.init_variable_payload(&mut sbuffer)?;
> +
> +        if !sbuffer.is_empty() {
> +            return Err(EIO);
> +        }
> +
> +        dev_dbg!(
> +            dev,
> +            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
> +            seq,
> +            M::FUNCTION,
> +            size_in_bytes,
> +        );
> +
> +        Ok(())
> +    }
> +}
> +
>  /// Trait representing messages received from the GSP.
>  ///
>  /// This trait tells [`Cmdq::receive_msg`] how it can receive a given type of message.
> @@ -638,24 +704,18 @@ impl CmdqInner<'_> {
>      /// Timeout for waiting for space on the command queue.
>      const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1);
>  
> -    /// Sends `command` to the GSP, without splitting it.
> +    /// Allocate enough send slots to store `command`, initialize them using
> +    /// [`CommandElement::write`], and send the command to the GSP.

nit: not sure what send slots are / seems like a new term? - I think the
caller doesn't need to understand that it's per page anyway

Reviewed-by: Eliot Courtney <ecourtney@xxxxxxxxxx>