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 <[email protected]> > Signed-off-by: Eliot Courtney <[email protected]> > --- > 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)?;
