[PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Alexandre Courbot]

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>

Re: [PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Eliot Courtney]

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>

Re: [PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Danilo Krummrich]

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/

Re: [PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Alexandre Courbot]

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?

Re: [PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Danilo Krummrich]

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.

Re: [PATCH] gpu: nova-core: gsp: move Cmdq's DMA handle to a struct member

[Alexandre Courbot]

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!