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

["Alexandre Courbot" <acourbot@nvidia.com>]

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?

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