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