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

[Eliot Courtney]

On Sat Mar 14, 2026 at 12:40 AM JST, Danilo Krummrich wrote:
> On Fri Feb 27, 2026 at 1:32 PM CET, Eliot Courtney wrote:
>> +/// Command for sending an RM control message to the GSP.
>> +struct RmControl<'a> {
>> +    h_client: u32,
>> +    h_object: u32,
>> +    cmd: RmControlMsgFunction,
>> +    params: &'a [u8],
>> +}
>
> Please expand the documentation, especially the fields.

Will do. Will add an explanation of client/device/subdevice/object
plus fix all the names.

>> +/// 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.flush_into_kvvec(GFP_KERNEL)?,
>> +        })
>> +    }
>> +}
>> +
>> +/// Sends an RM control command, checks the reply status, and returns the raw parameter bytes.
>> +#[expect(dead_code)]
>> +fn send_rm_control(
>
> Why isn't this a method of Cmdq?

Because this can be fully implemented in terms of existing primitives
available on Cmdq - it doesn't need to know anything about Cmdq
internals (it's a protocol on top of the Cmdq, not at the same level
of abstraction), and it fits the existing pattern of adding helpers for
sending messages to/from GSP (e.g. `get_gsp_info`), so IMO it is nicer
to keep it out of Cmdq.

>
>> +    cmdq: &Cmdq,
>> +    bar: &Bar0,
>> +    h_client: u32,
>> +    h_object: u32,
>> +    cmd: RmControlMsgFunction,
>> +    params: &[u8],
>> +) -> Result<KVVec<u8>> {
>> +    let reply = cmdq.send_sync_command(bar, RmControl::new(h_client, h_object, cmd, params))?;
>
> Why not let the caller construct RmControl?

Yeah, that seems fine to me, as long as we don't make `RmControl`
public outside of this module (so each helper would construct it and
call `send_rm_control`). If `RmControl` is public then it could be
misused and sent directly via `Cmdq`, which makes it easier to forget
to check the reply status.