Re: [PATCH 2/7] gpu: nova-core: gsp: add mechanism to wait for space on command queue

[Alexandre Courbot]

On Thu Feb 12, 2026 at 3:28 PM JST, Eliot Courtney wrote:
> Add `allocate_command_with_timeout` which waits for space on the GSP
> command queue. It uses a similar timeout to nouveau.
>
> Let `send_command` wait for space to free up in the command queue by
> calling `allocate_command_with_timeout`. This is required to

I'd name it just `allocate_command_timeout`, to follow the pattern of
the existing `read_poll_timeout` - but actually we might not even need
a new method (see below).

> support continuation records which can fill up the queue.
>
> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 42 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 46819a82a51a..baae06de0e09 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -243,6 +243,16 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
>          }
>      }
>  
> +    fn driver_bytes_available_to_write(&self) -> usize {

For consistency with `driver_write_area`, shall we name this
`driver_write_area_size`? And add a doccomment mentioning the returned
value is in bytes.

> +        let tx = self.cpu_write_ptr();
> +        let rx = self.gsp_read_ptr();
> +        // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_read_ptr` and

Nit: missing empty line.

> +        // `cpu_write_ptr`. The minimum value case is where `rx == 0` and `tx == MSGQ_NUM_PAGES -
> +        // 1`, which gives `0 + MSGQ_NUM_PAGES - (MSGQ_NUM_PAGES - 1) - 1 == 0`.
> +        let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
> +        num::u32_as_usize(slots) * GSP_PAGE_SIZE
> +    }
> +
>      /// Returns the region of the GSP message queue that the driver is currently allowed to read
>      /// from.
>      ///
> @@ -311,6 +321,25 @@ fn allocate_command(&mut self, size: usize) -> Result<GspCommand<'_>> {
>          })
>      }
>  
> +    /// Allocates a region on the command queue that is large enough to send a command of `size`
> +    /// bytes, waiting for space to become available.
> +    ///
> +    /// This returns a [`GspCommand`] ready to be written to by the caller.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `ETIMEDOUT` if space does not become available within the timeout.
> +    /// - `EIO` if the command header is not properly aligned.
> +    fn allocate_command_with_timeout(&mut self, size: usize) -> Result<GspCommand<'_>> {

Should the timeout be an argument? That way we can simply add it to
`allocate_command`, and invoke it with `Delta::ZERO` whenever we don't
want to wait. This is more explicit at the call site, removes the
need to have two methods, and removes the redundant size check from
`allocate_command` which is now done by this `read_poll_timeout`.

> +        read_poll_timeout(
> +            || Ok(self.driver_bytes_available_to_write()),
> +            |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
> +            Delta::ZERO,
> +            Delta::from_secs(1),
> +        )?;
> +        self.allocate_command(size)
> +    }
> +
>      // Returns the index of the memory page the GSP will write the next message to.
>      //
>      // # Invariants
> @@ -480,11 +509,18 @@ fn notify_gsp(bar: &Bar0) {
>              .write(bar);
>      }
>  
> +    fn command_size<M>(command: &M) -> usize

Shouldn't this be a member function of `CommandToGsp`? Please add some
basic documentation for it as well. As a general rule, all methods, even
basic ones, should have at least one line of doccomment.

> +    where
> +        M: CommandToGsp,
> +    {
> +        size_of::<M::Command>() + command.variable_payload_len()
> +    }
> +
>      /// Sends `command` to the GSP.
>      ///
>      /// # Errors
>      ///
> -    /// - `EAGAIN` if there was not enough space in the command queue to send the command.
> +    /// - `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.
>      ///
> @@ -495,8 +531,8 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>          // This allows all error types, including `Infallible`, to be used for `M::InitError`.
>          Error: From<M::InitError>,
>      {
> -        let command_size = size_of::<M::Command>() + command.variable_payload_len();
> -        let dst = self.gsp_mem.allocate_command(command_size)?;
> +        let command_size = Self::command_size(&command);

The addition of `command_size` looks like an unrelated change - it is
not really leveraged until patch 6 (although it is still valuable on its
own). Can you move it to its own patch for clarity?