Re: [PATCH v2 2/4] gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`

["Gary Guo" <gary@garyguo.net>]

On Mon Mar 2, 2026 at 3:03 AM GMT, Alexandre Courbot wrote:
> On Mon Mar 2, 2026 at 11:22 AM JST, Eliot Courtney wrote:
>> On Sat Feb 28, 2026 at 3:11 PM JST, John Hubbard wrote:
>>> On 2/26/26 7:50 AM, Eliot Courtney wrote:
>>>> Add sync and async command queue API and the type infrastructure to know
>>>> what reply is expected from each `CommandToGsp`.
>>>> 
>>>> Use a marker type `NoReply` which does not implement `MessageFromGsp` to
>>>> mark async commands which don't expect a response.
>>>> 
>>> ...
>>>> +    /// Type of the reply expected from the GSP, or [`NoReply`] for async commands.
>>>
>>> Hi Eliot,
>>>
>>> The following does not need to hold up your patchset, but I want
>>> to bring it up somewhere just to work through it.
>>>
>>> The sync/async naming that GSP RM uses is a little bit "off". I
>>> spent some time discussing it with them, and the problem is that
>>> sync/async is a concept that is somewhat independent of whether
>>> a reply is expected. Usually, sync means a blocking wait for a
>>> response, which is not necessarily required in all case with
>>> GSP RM calls.
>>>
>>> The naming would be better here if it reflected simply that
>>> a response is expected, or not. I don't have great names for
>>> that, but "fire and forget" works well for what we have so
>>> far called "async". So we could do create a convention in which
>>> no annotation means that the API has a response that will come
>>> back, and some abbreviated for of "fire and forget" or "one way"
>>> added to the function name would mean that no response is
>>> expected.
>>>
>>> Again, I don't think this has to happen here, because we can
>>> go through and rename later, no problem there. But when I saw
>>> the sync/asynch and remembered the very recent discussion, I
>>> figured I'd better post something about it.
>>>
>>> And yes, I started us off in the wrong direction with the
>>> IS_ASYNCH thing! haha
>>>
>>> thanks,
>>
>> Hi John,
>>
>> I totally agree and was hoping that someone would have a good suggestion
>> for this. I discussed this exact thing with Alex before posting this
>> too. So if you have any naming suggestions would love to hear them.
>>
>> As you say, sync and async are orthogonal to reply vs no reply. I think
>> we have several ideas here actually:
>> - blocking vs non-blocking
>> - reply vs no-reply
>> - wait for reply vs don't wait for reply (practically equivalent to
>>   blocking vs non-blocking here, but conceptually the send could also be
>>   blocking vs non-blocking)
>>
>> We should also be careful with conflating waiting for the reply vs not
>> having a reply. So `send_without_waiting_for_reply` is definitely
>> confusing to me, because there may be a reply that we just don't wait
>> for.
>>
>> Some ideas:
>> - send_command_with_reply + send_command_without_reply
>>   - Maybe non-obvious that this blocks for send_command_with_reply.
>> - send_and_wait_for_reply + send_no_reply
>>   - More obvious that it blocks and gets the reply.
>>   - Should be obvious from context that you are sending a command
>>     anyway.
>>
>> Personally I think it's nice to keep a convention of keeping it
>> mostly obvious which functions block/wait. (e.g. we already have
>> wait_for_msg in cmdq.rs).
>>
>> For lack of a better idea  i suggest send_and_wait_for_reply +
>> send_no_reply for now.
>
> One important detail IMHO is that the API cannot be misused, i.e. you
> cannot call the fire-and-forget send method on a command that expects a
> reply. So the risk is mostly when adding support for a new command - but
> if that step is done properly, users will be directed to the right
> method by the compiler.
>
> This, I think, allows us to tolerate more ambiguity in the method names,
> as long as their documentation makes up for it. We all agree that
> `async` and `sync` are not a good fit, but `send`/`send_noreply` should
> be tolerable (I'd like to keep the names short if possible)
>
> Or maybe we can use a variant of the trick mentioned by Gary in [1] and
> have a single `send_command` method?

I think for this particular use case there isn't even a need to use any tricks?
You can simply just skip the receiving part when there's no reply expected.

Something like

    if TypeId::of::<M::Reply>() == TypeId::of::<NoReply>() {
        // SAFETY: `M::Reply` is `NoReply` and it's `Copy`.
        return Ok(unsafe { core::mem::transmute_copy(&NoReply) });
    }

Best,
Gary

>
> [1] https://lore.kernel.org/all/DGRJJA3068FV.3CE7J7SKLTN8O@garyguo.net/