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.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 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))?;
 
         // Extract area for the command itself.
         let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;

-- 
2.53.0

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@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  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)?;

On Mon Mar 2, 2026 at 11:18 PM JST, Gary Guo wrote:
> 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.

This kind of structure is what I originally did (see 
https://lore.kernel.org/all/DGHRDFE9M6P7.L7JEOCLL3VS9@nvidia.com/)
although it wasn't quite as nice since `allocate_command` takes
&mut self and isn't callable from `read_poll_timeout`. Alex suggested
merging these into one as it simplifies a few things. But maybe I am
missing some better way to do it.

On Tue Mar 3, 2026 at 12:07 PM JST, Eliot Courtney wrote:
> On Mon Mar 2, 2026 at 11:18 PM JST, Gary Guo wrote:
>> 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.
>
> This kind of structure is what I originally did (see 
> https://lore.kernel.org/all/DGHRDFE9M6P7.L7JEOCLL3VS9@nvidia.com/)
> although it wasn't quite as nice since `allocate_command` takes
> &mut self and isn't callable from `read_poll_timeout`. Alex suggested
> merging these into one as it simplifies a few things. But maybe I am
> missing some better way to do it.

The rationale for this is that we don't have a case for polling for
command queue availability - the GSP is expected to consume all the data
sent to it rapidly, so we have a soft guarantee that the queue will be
ready soon. We also don't have a case for doing something else until the
queue is ready, AFAICT.

The current form is simpler code-wise, and if we ever need to break it
into two poll and wait methods, we can do it trivially when the need
arises.