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

[Eliot Courtney]

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@xxxxxxxxxx/