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

[Alexandre Courbot]

On Thu Oct 1, 2026 at 11:21 AM JST, Eliot Courtney wrote:
> 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

Indeed, that detail is not necessary. Reworded, thanks!