[PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure

Eliot Courtney posted 9 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure
Posted by Eliot Courtney 2 weeks, 5 days ago
Add `RmControl` which implements CommandToGsp for sending RM control
RPCs.

Add `RmControlReply` which implements MessageFromGsp for getting the
reply back.

Add `send_rm_control` which sends an RM control RPC via the command
queue using the above structures.

This gives a generic way to send each RM control RPC. Each new RM
control RPC can be added by extending RmControlMsgFunction and adding
its bindings wrappers and writing a helper function to send it via
`send_rm_control`.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp.rs             |   1 +
 drivers/gpu/nova-core/gsp/fw/rm.rs       |   1 -
 drivers/gpu/nova-core/gsp/rm.rs          |   3 +
 drivers/gpu/nova-core/gsp/rm/commands.rs | 118 +++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 72f173726f87..14c734c53e7c 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -17,6 +17,7 @@
 pub(crate) mod cmdq;
 pub(crate) mod commands;
 mod fw;
+pub(crate) mod rm;
 mod sequencer;
 
 pub(crate) use fw::{
diff --git a/drivers/gpu/nova-core/gsp/fw/rm.rs b/drivers/gpu/nova-core/gsp/fw/rm.rs
index 4a4f97d88ecf..1c6e8b4c4865 100644
--- a/drivers/gpu/nova-core/gsp/fw/rm.rs
+++ b/drivers/gpu/nova-core/gsp/fw/rm.rs
@@ -53,7 +53,6 @@ pub(crate) struct GspRmControl {
     inner: bindings::rpc_gsp_rm_control_v03_00,
 }
 
-#[expect(dead_code)]
 impl GspRmControl {
     /// Creates a new RM control command.
     pub(crate) fn new<T>(
diff --git a/drivers/gpu/nova-core/gsp/rm.rs b/drivers/gpu/nova-core/gsp/rm.rs
new file mode 100644
index 000000000000..10e879a3e842
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/rm.rs
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0
+
+pub(crate) mod commands;
diff --git a/drivers/gpu/nova-core/gsp/rm/commands.rs b/drivers/gpu/nova-core/gsp/rm/commands.rs
new file mode 100644
index 000000000000..5a3ac7bd415a
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/rm/commands.rs
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use core::{
+    array,
+    convert::Infallible, //
+};
+
+use kernel::prelude::*;
+
+use crate::{
+    driver::Bar0,
+    gsp::{
+        cmdq::{
+            Cmdq,
+            CommandToGsp,
+            MessageFromGsp, //
+        },
+        commands::{
+            Client,
+            Handle, //
+        },
+        fw::{
+            rm::*,
+            MsgFunction,
+            NvStatus, //
+        },
+    },
+    sbuffer::SBufferIter,
+};
+
+/// Command for sending an RM control message to the GSP.
+///
+/// RM control messages are used to query or control RM objects (see [`Handle`] for more info on RM
+/// objects). It takes a client handle and an RM object handle identifying the target of the
+/// message, within the given client.
+struct RmControl<'a, T> {
+    /// The client handle under which `object` is allocated.
+    client: Handle<Client>,
+    /// The RM object handle to query or control.
+    object: Handle<T>,
+    /// The specific control message to send.
+    cmd: RmControlMsgFunction,
+    /// The raw parameter bytes to send with the control message. Interpretation of these bytes is
+    /// specific to the control message being sent.
+    params: &'a [u8],
+}
+
+impl<'a, T> RmControl<'a, T> {
+    /// Creates a new RM control command.
+    #[expect(dead_code)]
+    fn new(
+        client: Handle<Client>,
+        object: Handle<T>,
+        cmd: RmControlMsgFunction,
+        params: &'a [u8],
+    ) -> Self {
+        Self {
+            client,
+            object,
+            cmd,
+            params,
+        }
+    }
+}
+
+impl<T> CommandToGsp for RmControl<'_, T> {
+    const FUNCTION: MsgFunction = MsgFunction::GspRmControl;
+    type Command = GspRmControl;
+    type Reply = RmControlReply;
+    type InitError = Infallible;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        GspRmControl::new(self.client, self.object, self.cmd, self.params.len() as u32)
+    }
+
+    fn variable_payload_len(&self) -> usize {
+        self.params.len()
+    }
+
+    fn init_variable_payload(
+        &self,
+        dst: &mut SBufferIter<array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        dst.write_all(self.params)
+    }
+}
+
+/// Response from an RM control message.
+pub(crate) struct RmControlReply {
+    status: NvStatus,
+    params: KVVec<u8>,
+}
+
+impl MessageFromGsp for RmControlReply {
+    const FUNCTION: MsgFunction = MsgFunction::GspRmControl;
+    type Message = GspRmControl;
+    type InitError = Error;
+
+    fn read(
+        msg: &Self::Message,
+        sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError> {
+        Ok(RmControlReply {
+            status: msg.status(),
+            params: sbuffer.read_to_vec(GFP_KERNEL)?,
+        })
+    }
+}
+
+/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
+#[expect(dead_code)]
+fn send_rm_control<T>(cmdq: &Cmdq, bar: &Bar0, cmd: RmControl<'_, T>) -> Result<KVVec<u8>> {
+    let reply = cmdq.send_command(bar, cmd)?;
+
+    Result::from(reply.status)?;
+
+    Ok(reply.params)
+}

-- 
2.53.0
Re: [PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure
Posted by Danilo Krummrich 2 weeks, 5 days ago
On Wed Mar 18, 2026 at 8:14 AM CET, Eliot Courtney wrote:
> Add `RmControl` which implements CommandToGsp for sending RM control
> RPCs.
>
> Add `RmControlReply` which implements MessageFromGsp for getting the
> reply back.
>
> Add `send_rm_control` which sends an RM control RPC via the command
> queue using the above structures.
>
> This gives a generic way to send each RM control RPC. Each new RM
> control RPC can be added by extending RmControlMsgFunction and adding
> its bindings wrappers and writing a helper function to send it via
> `send_rm_control`.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp.rs             |   1 +
>  drivers/gpu/nova-core/gsp/fw/rm.rs       |   1 -
>  drivers/gpu/nova-core/gsp/rm.rs          |   3 +
>  drivers/gpu/nova-core/gsp/rm/commands.rs | 118 +++++++++++++++++++++++++++++++
>  4 files changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 72f173726f87..14c734c53e7c 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -17,6 +17,7 @@
>  pub(crate) mod cmdq;
>  pub(crate) mod commands;
>  mod fw;
> +pub(crate) mod rm;
>  mod sequencer;
>  
>  pub(crate) use fw::{
> diff --git a/drivers/gpu/nova-core/gsp/fw/rm.rs b/drivers/gpu/nova-core/gsp/fw/rm.rs
> index 4a4f97d88ecf..1c6e8b4c4865 100644
> --- a/drivers/gpu/nova-core/gsp/fw/rm.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/rm.rs
> @@ -53,7 +53,6 @@ pub(crate) struct GspRmControl {
>      inner: bindings::rpc_gsp_rm_control_v03_00,
>  }
>  
> -#[expect(dead_code)]
>  impl GspRmControl {
>      /// Creates a new RM control command.
>      pub(crate) fn new<T>(
> diff --git a/drivers/gpu/nova-core/gsp/rm.rs b/drivers/gpu/nova-core/gsp/rm.rs
> new file mode 100644
> index 000000000000..10e879a3e842
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/rm.rs
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +pub(crate) mod commands;
> diff --git a/drivers/gpu/nova-core/gsp/rm/commands.rs b/drivers/gpu/nova-core/gsp/rm/commands.rs
> new file mode 100644
> index 000000000000..5a3ac7bd415a
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/rm/commands.rs
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use core::{
> +    array,
> +    convert::Infallible, //
> +};
> +
> +use kernel::prelude::*;
> +
> +use crate::{
> +    driver::Bar0,
> +    gsp::{
> +        cmdq::{
> +            Cmdq,
> +            CommandToGsp,
> +            MessageFromGsp, //
> +        },
> +        commands::{
> +            Client,
> +            Handle, //
> +        },
> +        fw::{
> +            rm::*,
> +            MsgFunction,
> +            NvStatus, //
> +        },
> +    },
> +    sbuffer::SBufferIter,
> +};
> +
> +/// Command for sending an RM control message to the GSP.
> +///
> +/// RM control messages are used to query or control RM objects (see [`Handle`] for more info on RM
> +/// objects). It takes a client handle and an RM object handle identifying the target of the
> +/// message, within the given client.
> +struct RmControl<'a, T> {
> +    /// The client handle under which `object` is allocated.
> +    client: Handle<Client>,
> +    /// The RM object handle to query or control.
> +    object: Handle<T>,
> +    /// The specific control message to send.
> +    cmd: RmControlMsgFunction,
> +    /// The raw parameter bytes to send with the control message. Interpretation of these bytes is
> +    /// specific to the control message being sent.
> +    params: &'a [u8],
> +}
> +
> +impl<'a, T> RmControl<'a, T> {
> +    /// Creates a new RM control command.
> +    #[expect(dead_code)]
> +    fn new(
> +        client: Handle<Client>,
> +        object: Handle<T>,
> +        cmd: RmControlMsgFunction,
> +        params: &'a [u8],
> +    ) -> Self {
> +        Self {
> +            client,
> +            object,
> +            cmd,
> +            params,
> +        }
> +    }
> +}
> +
> +impl<T> CommandToGsp for RmControl<'_, T> {
> +    const FUNCTION: MsgFunction = MsgFunction::GspRmControl;
> +    type Command = GspRmControl;
> +    type Reply = RmControlReply;
> +    type InitError = Infallible;
> +
> +    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> +        GspRmControl::new(self.client, self.object, self.cmd, self.params.len() as u32)
> +    }
> +
> +    fn variable_payload_len(&self) -> usize {
> +        self.params.len()
> +    }
> +
> +    fn init_variable_payload(
> +        &self,
> +        dst: &mut SBufferIter<array::IntoIter<&mut [u8], 2>>,
> +    ) -> Result {
> +        dst.write_all(self.params)
> +    }
> +}
> +
> +/// Response from an RM control message.
> +pub(crate) struct RmControlReply {
> +    status: NvStatus,
> +    params: KVVec<u8>,
> +}
> +
> +impl MessageFromGsp for RmControlReply {
> +    const FUNCTION: MsgFunction = MsgFunction::GspRmControl;
> +    type Message = GspRmControl;
> +    type InitError = Error;
> +
> +    fn read(
> +        msg: &Self::Message,
> +        sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
> +    ) -> Result<Self, Self::InitError> {
> +        Ok(RmControlReply {
> +            status: msg.status(),
> +            params: sbuffer.read_to_vec(GFP_KERNEL)?,
> +        })
> +    }
> +}
> +
> +/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
> +#[expect(dead_code)]
> +fn send_rm_control<T>(cmdq: &Cmdq, bar: &Bar0, cmd: RmControl<'_, T>) -> Result<KVVec<u8>> {
> +    let reply = cmdq.send_command(bar, cmd)?;
> +
> +    Result::from(reply.status)?;
> +
> +    Ok(reply.params)
> +}

It still feels wrong to me for this to be a standalone function.

It should either be a method of Cmdq, or it should be a method of RmControl,
that takes self by value, i.e. either Cmdq::send_rm_ctrl() or RmControl::send().

Please choose one of those options.
Re: [PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure
Posted by Eliot Courtney 2 weeks, 4 days ago
On Wed Mar 18, 2026 at 9:35 PM JST, Danilo Krummrich wrote:
>> +/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
>> +#[expect(dead_code)]
>> +fn send_rm_control<T>(cmdq: &Cmdq, bar: &Bar0, cmd: RmControl<'_, T>) -> Result<KVVec<u8>> {
>> +    let reply = cmdq.send_command(bar, cmd)?;
>> +
>> +    Result::from(reply.status)?;
>> +
>> +    Ok(reply.params)
>> +}
>
> It still feels wrong to me for this to be a standalone function.
>
> It should either be a method of Cmdq, or it should be a method of RmControl,
> that takes self by value, i.e. either Cmdq::send_rm_ctrl() or RmControl::send().
>
> Please choose one of those options.

RmControl::send() seems good to me, will do that one.
Re: [PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure
Posted by Alexandre Courbot 2 weeks, 3 days ago
On Thu Mar 19, 2026 at 10:06 AM JST, Eliot Courtney wrote:
> On Wed Mar 18, 2026 at 9:35 PM JST, Danilo Krummrich wrote:
>>> +/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
>>> +#[expect(dead_code)]
>>> +fn send_rm_control<T>(cmdq: &Cmdq, bar: &Bar0, cmd: RmControl<'_, T>) -> Result<KVVec<u8>> {
>>> +    let reply = cmdq.send_command(bar, cmd)?;
>>> +
>>> +    Result::from(reply.status)?;
>>> +
>>> +    Ok(reply.params)
>>> +}
>>
>> It still feels wrong to me for this to be a standalone function.
>>
>> It should either be a method of Cmdq, or it should be a method of RmControl,
>> that takes self by value, i.e. either Cmdq::send_rm_ctrl() or RmControl::send().
>>
>> Please choose one of those options.
>
> RmControl::send() seems good to me, will do that one.

Honestly if we can just extend `Cmdq` with a RM-dedicated impl block in
`rm.rs` and make these regular `Cmdq` methods, why are we jumping
through hoops?

RM commands are one kind of command, I don't see why they need to be
sent through an associated function that takes a `Cmdq` as the first
argument anyway.
Re: [PATCH v2 7/9] gpu: nova-core: gsp: add RM control command infrastructure
Posted by Eliot Courtney 1 week, 5 days ago
On Fri Mar 20, 2026 at 11:42 PM JST, Alexandre Courbot wrote:
> On Thu Mar 19, 2026 at 10:06 AM JST, Eliot Courtney wrote:
>> On Wed Mar 18, 2026 at 9:35 PM JST, Danilo Krummrich wrote:
>>>> +/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
>>>> +#[expect(dead_code)]
>>>> +fn send_rm_control<T>(cmdq: &Cmdq, bar: &Bar0, cmd: RmControl<'_, T>) -> Result<KVVec<u8>> {
>>>> +    let reply = cmdq.send_command(bar, cmd)?;
>>>> +
>>>> +    Result::from(reply.status)?;
>>>> +
>>>> +    Ok(reply.params)
>>>> +}
>>>
>>> It still feels wrong to me for this to be a standalone function.
>>>
>>> It should either be a method of Cmdq, or it should be a method of RmControl,
>>> that takes self by value, i.e. either Cmdq::send_rm_ctrl() or RmControl::send().
>>>
>>> Please choose one of those options.
>>
>> RmControl::send() seems good to me, will do that one.
>
> Honestly if we can just extend `Cmdq` with a RM-dedicated impl block in
> `rm.rs` and make these regular `Cmdq` methods, why are we jumping
> through hoops?
>
> RM commands are one kind of command, I don't see why they need to be
> sent through an associated function that takes a `Cmdq` as the first
> argument anyway.

I posted my reason for not wanting to do this here[1]. But TLDR is that
these RM control commands are built on top of the Cmdq transport. They
don't need visibility into the Cmdq implementation; they're at a higher
level of abstraction. But let me think if there is a nicer way to do all
of this.

[1]: https://lore.kernel.org/all/DH46ZI0CRRKZ.1UZTS1TIWPRTQ@nvidia.com/