drivers/gpu/nova-core/gsp/cmdq.rs | 26 +++++++++++++++----------- drivers/gpu/nova-core/gsp/fw.rs | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-)
The command-queue structure has a `dma_handle` method that returns the
DMA handle to the memory segment shared with the GSP. This works, but is
not ideal for the following reasons:
- That method is effectively only ever called once, and is technically
an accessor method since the handle doesn't change over time,
- It feels a bit out-of-place with the other methods of `Cmdq` which
only deal with the sending or receiving of messages,
- The method has `pub(crate)` visibility, allowing other driver code to
access this highly-sensitive handle.
Address all these issues by turning `dma_handle` into a struct member
with `pub(super)` visibility. This keeps the method space focused, and
also ensures the member is not visible outside of the modules that need
it.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/cmdq.rs | 26 +++++++++++++++-----------
drivers/gpu/nova-core/gsp/fw.rs | 2 +-
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index d36a62ba1c60..03c7c2d0ea35 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -30,6 +30,8 @@
SplitState, //
};
+use pin_init::pin_init_scope;
+
use crate::{
driver::Bar0,
gsp::{
@@ -455,6 +457,8 @@ pub(crate) struct Cmdq {
/// Inner mutex-protected state.
#[pin]
inner: Mutex<CmdqInner>,
+ /// DMA handle of the command queue's shared memory region.
+ pub(super) dma_handle: DmaAddress,
}
impl Cmdq {
@@ -479,12 +483,17 @@ impl Cmdq {
/// Creates a new command queue for `dev`.
pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Error> + '_ {
- try_pin_init!(Self {
- inner <- new_mutex!(CmdqInner {
- dev: dev.into(),
- gsp_mem: DmaGspMem::new(dev)?,
- seq: 0,
- }),
+ pin_init_scope(move || {
+ let gsp_mem = DmaGspMem::new(dev)?;
+
+ Ok(try_pin_init!(Self {
+ dma_handle: gsp_mem.0.dma_handle(),
+ inner <- new_mutex!(CmdqInner {
+ dev: dev.into(),
+ gsp_mem,
+ seq: 0,
+ }),
+ }))
})
}
@@ -570,11 +579,6 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
{
self.inner.lock().receive_msg(timeout)
}
-
- /// Returns the DMA handle of the command queue's shared memory region.
- pub(crate) fn dma_handle(&self) -> DmaAddress {
- self.inner.lock().gsp_mem.0.dma_handle()
- }
}
/// Inner mutex protected state of [`Cmdq`].
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index a061131b5412..0506c2293e7c 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -892,7 +892,7 @@ impl MessageQueueInitArguments {
/// Creates a new init arguments structure for `cmdq`.
fn new(cmdq: &Cmdq) -> Self {
Self(bindings::MESSAGE_QUEUE_INIT_ARGUMENTS {
- sharedMemPhysAddr: cmdq.dma_handle(),
+ sharedMemPhysAddr: cmdq.dma_handle,
pageTableEntryCount: num::usize_into_u32::<{ Cmdq::NUM_PTES }>(),
cmdQueueOffset: num::usize_as_u64(Cmdq::CMDQ_OFFSET),
statQueueOffset: num::usize_as_u64(Cmdq::STATQ_OFFSET),
---
base-commit: a19457958c3018783881c4416f272cd594f13049
change-id: 20260319-b4-cmdq-dma-handle-c56ae9006104
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>On Thu Mar 19, 2026 at 3:00 PM JST, Alexandre Courbot wrote: > The command-queue structure has a `dma_handle` method that returns the > DMA handle to the memory segment shared with the GSP. This works, but is > not ideal for the following reasons: > > - That method is effectively only ever called once, and is technically > an accessor method since the handle doesn't change over time, > - It feels a bit out-of-place with the other methods of `Cmdq` which > only deal with the sending or receiving of messages, > - The method has `pub(crate)` visibility, allowing other driver code to > access this highly-sensitive handle. > > Address all these issues by turning `dma_handle` into a struct member > with `pub(super)` visibility. This keeps the method space focused, and > also ensures the member is not visible outside of the modules that need > it. > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> Pushed to drm-rust-next, thanks!
On Thu Mar 19, 2026 at 7:00 AM CET, Alexandre Courbot wrote: > The command-queue structure has a `dma_handle` method that returns the > DMA handle to the memory segment shared with the GSP. This works, but is > not ideal for the following reasons: > > - That method is effectively only ever called once, and is technically > an accessor method since the handle doesn't change over time, > - It feels a bit out-of-place with the other methods of `Cmdq` which > only deal with the sending or receiving of messages, > - The method has `pub(crate)` visibility, allowing other driver code to > access this highly-sensitive handle. > > Address all these issues by turning `dma_handle` into a struct member > with `pub(super)` visibility. This keeps the method space focused, and > also ensures the member is not visible outside of the modules that need > it. > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> Linking my note from the original patch series [1]. Reviewed-by: Danilo Krummrich <dakr@kernel.org> [1] https://lore.kernel.org/all/DH60S91O053Z.1DDUJSL0D6NQQ@kernel.org/
On Fri Mar 20, 2026 at 9:45 PM JST, Danilo Krummrich wrote: > On Thu Mar 19, 2026 at 7:00 AM CET, Alexandre Courbot wrote: >> The command-queue structure has a `dma_handle` method that returns the >> DMA handle to the memory segment shared with the GSP. This works, but is >> not ideal for the following reasons: >> >> - That method is effectively only ever called once, and is technically >> an accessor method since the handle doesn't change over time, >> - It feels a bit out-of-place with the other methods of `Cmdq` which >> only deal with the sending or receiving of messages, >> - The method has `pub(crate)` visibility, allowing other driver code to >> access this highly-sensitive handle. >> >> Address all these issues by turning `dma_handle` into a struct member >> with `pub(super)` visibility. This keeps the method space focused, and >> also ensures the member is not visible outside of the modules that need >> it. >> >> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > > Linking my note from the original patch series [1]. > > Reviewed-by: Danilo Krummrich <dakr@kernel.org> > > [1] https://lore.kernel.org/all/DH60S91O053Z.1DDUJSL0D6NQQ@kernel.org/ Do you want me to add a `Link:` tag to the commit?
On 3/21/26 2:41 PM, Alexandre Courbot wrote: > On Fri Mar 20, 2026 at 9:45 PM JST, Danilo Krummrich wrote: >> On Thu Mar 19, 2026 at 7:00 AM CET, Alexandre Courbot wrote: >>> The command-queue structure has a `dma_handle` method that returns the >>> DMA handle to the memory segment shared with the GSP. This works, but is >>> not ideal for the following reasons: >>> >>> - That method is effectively only ever called once, and is technically >>> an accessor method since the handle doesn't change over time, >>> - It feels a bit out-of-place with the other methods of `Cmdq` which >>> only deal with the sending or receiving of messages, >>> - The method has `pub(crate)` visibility, allowing other driver code to >>> access this highly-sensitive handle. >>> >>> Address all these issues by turning `dma_handle` into a struct member >>> with `pub(super)` visibility. This keeps the method space focused, and >>> also ensures the member is not visible outside of the modules that need >>> it. >>> >>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> >> >> Linking my note from the original patch series [1]. >> >> Reviewed-by: Danilo Krummrich <dakr@kernel.org> >> >> [1] https://lore.kernel.org/all/DH60S91O053Z.1DDUJSL0D6NQQ@kernel.org/ > > Do you want me to add a `Link:` tag to the commit? I don't think that's necessary.
On Thu Mar 19, 2026 at 3:00 PM JST, Alexandre Courbot wrote: > The command-queue structure has a `dma_handle` method that returns the > DMA handle to the memory segment shared with the GSP. This works, but is > not ideal for the following reasons: > > - That method is effectively only ever called once, and is technically > an accessor method since the handle doesn't change over time, > - It feels a bit out-of-place with the other methods of `Cmdq` which > only deal with the sending or receiving of messages, > - The method has `pub(crate)` visibility, allowing other driver code to > access this highly-sensitive handle. > > Address all these issues by turning `dma_handle` into a struct member > with `pub(super)` visibility. This keeps the method space focused, and > also ensures the member is not visible outside of the modules that need > it. > > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com> > --- Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
© 2016 - 2026 Red Hat, Inc.