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

["Eliot Courtney" <ecourtney@nvidia.com>]

On Sun Sep 27, 2026 at 10:46 PM JST, Alexandre Courbot wrote:
> Move the transport part of `send_single_command` into
> `send_command_element`, which allocates the queue slots, writes the
> element header, calls a closure to fill the remainder of the command,
> then computes the checksum, advances the write pointer and rings the
> doorbell.
>
> The RPC part of `send_single_command` (writing the RPC header and the
> command payload) is passed as a closure, unchanged apart from its
> indentation. 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.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 118 ++++++++++++++++++++++++--------------
>  1 file changed, 74 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 07e8e32c3d57..b6d50b0bd039 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -638,65 +638,35 @@ 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.
> +    /// Allocates enough send slots to store a command of `sizes_in_bytes` length, initialize them
> +    /// using `command_init`, and send the command to the GSP.
>      ///
>      /// # Errors
>      ///
>      /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
>      /// - `ETIMEDOUT` if space does not become available within the timeout.
> -    /// - `EIO` if the variable payload requested by the command has not been entirely
> -    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
>      ///
> -    /// Error codes returned by the command initializers are propagated as-is.
> -    fn send_single_command<M>(&mut self, command: M) -> Result
> -    where
> -        M: CommandToGsp,
> -        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
> -        Error: From<M::InitError>,
> -    {
> -        let size_in_bytes = command.size();
> -        let dst = self
> +    /// Error codes returned by `command_init` are returned as-is.
> +    fn send_command_element(
> +        &mut self,
> +        size_in_bytes: usize,
> +        command_init: impl FnOnce(&mut GspCommand<'_>) -> Result,
> +    ) -> Result {
> +        let mut dst = self
>              .gsp_mem
>              .allocate_command(size_in_bytes, Self::ALLOCATE_TIMEOUT)?;
>  
> +        let seq = self.seq;

nit: this local reads noisily to me

> +
>          // Fill the header.
> -        let msg_element_init = GspMsgElement::init(self.seq, size_in_bytes);
> +        let msg_element_init = GspMsgElement::init(seq, size_in_bytes);
>          // SAFETY: `msg_header` is a valid reference, and not touched if the initializer fails.
>          unsafe {
>              pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
>          }
>  
> -        // 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: `rpc_header_mut()` and `cmd` are valid references, and not touched if the
> -        // initializer fails.
> -        unsafe {
> -            pin_init::raw_try_init(
> -                core::ptr::from_mut(dst.header.rpc_header_mut()),
> -                rpc_header_init,
> -            )?;
> -            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);
> -        }
> -        drop(sbuffer);
> -
> -        dev_dbg!(
> -            &self.dev,
> -            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
> -            self.seq,
> -            M::FUNCTION,
> -            size_in_bytes,
> -        );
> +        // Initialize the message payload.
> +        command_init(&mut dst)?;
>  
>          // Compute checksum now that the whole message is ready.
>          dst.header
> @@ -715,6 +685,66 @@ fn send_single_command<M>(&mut self, command: M) -> Result
>          Ok(())
>      }
>  
> +    /// Sends `command` to the GSP, without splitting it.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
> +    /// - `ETIMEDOUT` if space does not become available within the timeout.
> +    /// - `EIO` if the variable payload requested by the command has not been entirely
> +    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
> +    ///
> +    /// Error codes returned by the command initializers are propagated as-is.
> +    fn send_single_command<M>(&mut self, command: M) -> Result
> +    where
> +        M: CommandToGsp,
> +        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
> +        Error: From<M::InitError>,
> +    {
> +        let dev = self.dev;
> +        let seq = self.seq;
> +        let size_in_bytes = command.size();

I suspect if we pass dev and seq into the closure below, we can
further split the RPC layer from the transport layer. That means the
patches after this won't have to impl on CmdqInner for e.g.
`send_single_command` and instead we can just define a trait that writes
in the message layer data (so send_single_command could e.g. instead
take an impl RpcCommandWriter or whatever, and we can impl
RpcCommandWriter for anything that impls CommandToGsp, inserting the
message layer protocol stuff in there).