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

[Gary Guo]

On Mon Mar 2, 2026 at 11:42 AM GMT, Eliot Courtney wrote:
> Add a timeout to `allocate_command` which waits for space on the GSP
> command queue. It uses a similar timeout to nouveau.
>
> This lets `send_command` wait for space to free up in the command queue.
> This is required to support continuation records which can fill up the
> queue.

Any reason that this isn't implemented as a `poll_allocate_command` which just
returns a `EAGAIN`, and then a wrapper function that just waits for space to be
ready when it got one? This way the logic is cleaner.

(I write this with Rust async in mind)

>
> Tested-by: Zhi Wang <zhiw@xxxxxxxxxx>
> Signed-off-by: Eliot Courtney <ecourtney@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 39 +++++++++++++++++++++++++++------------
>  1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 87dbbd6d1be9..efbbc89f4d8a 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -250,6 +250,19 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
>          }
>      }
>  
> +    /// Returns the size of the region of the CPU message queue that the driver is currently allowed
> +    /// to write to, in bytes.
> +    fn driver_write_area_size(&self) -> usize {
> +        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
> +        // `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.
>      ///
> @@ -281,15 +294,22 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
>      }
>  
>      /// Allocates a region on the command queue that is large enough to send a command of `size`
> -    /// bytes.
> +    /// bytes, waiting for space to become available based on the provided timeout.
>      ///
>      /// This returns a [`GspCommand`] ready to be written to by the caller.
>      ///
>      /// # Errors
>      ///
> -    /// - `EAGAIN` if the driver area is too small to hold the requested command.
> +    /// - `ETIMEDOUT` if space does not become available within the timeout.
>      /// - `EIO` if the command header is not properly aligned.
> -    fn allocate_command(&mut self, size: usize) -> Result<GspCommand<'_>> {
> +    fn allocate_command(&mut self, size: usize, timeout: Delta) -> Result<GspCommand<'_>> {
> +        read_poll_timeout(
> +            || Ok(self.driver_write_area_size()),
> +            |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
> +            Delta::ZERO,
> +            timeout,
> +        )?;
> +
>          // Get the current writable area as an array of bytes.
>          let (slice_1, slice_2) = {
>              let (slice_1, slice_2) = self.driver_write_area();
> @@ -298,13 +318,6 @@ fn allocate_command(&mut self, size: usize) -> Result<GspCommand<'_>> {
>              (slice_1.as_flattened_mut(), slice_2.as_flattened_mut())
>          };
>  
> -        // If the GSP is still processing previous messages the shared region
> -        // may be full in which case we will have to retry once the GSP has
> -        // processed the existing commands.
> -        if size_of::<GspMsgElement>() + size > slice_1.len() + slice_2.len() {
> -            return Err(EAGAIN);
> -        }
> -
>          // Extract area for the `GspMsgElement`.
>          let (header, slice_1) = GspMsgElement::from_bytes_mut_prefix(slice_1).ok_or(EIO)?;
>  
> @@ -497,7 +510,7 @@ fn notify_gsp(bar: &Bar0) {
>      ///
>      /// # 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.
>      ///
> @@ -509,7 +522,9 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>          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 dst = self
> +            .gsp_mem
> +            .allocate_command(command_size, Delta::from_secs(1))?;

Let's make these consts, not magic numbers.

Best,
Gary

>  
>          // Extract area for the command itself.
>          let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;