From: "Eliot Courtney" <ecourtney@nvidia.com>
To: "Alexandre Courbot" <acourbot@nvidia.com>,
"Eliot Courtney" <ecourtney@nvidia.com>
Cc: Danilo Krummrich <dakr@kernel.org>,
Alice Ryhl <aliceryhl@google.com>,
Simona Vetter <simona@ffwll.ch>, Benno Lossin <lossin@kernel.org>,
Gary Guo <gary@garyguo.net>, Alistair Popple <apopple@nvidia.com>,
Joel Fernandes <joelagnelf@nvidia.com>,
nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
Zhi Wang <zhiw@nvidia.com>,
dri-devel <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`
Date: Thu, 05 Mar 2026 10:34:10 +0900 [thread overview]
Message-ID: <DGUGNCEXKDD2.ETWE4H89AL00@nvidia.com> (raw)
In-Reply-To: <DGTZ9NLS9TPM.2NRF5C9VN2B7C@nvidia.com>
On Wed Mar 4, 2026 at 8:56 PM JST, Alexandre Courbot wrote:
>> + /// Sends `command` to the GSP and waits for the reply.
>> + ///
>> + /// # Errors
>> + ///
>> + /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
>> + /// not received within the timeout.
>> + /// - `EIO` if the variable payload requested by the command has not been entirely
>> + /// written to by its [`CommandToGsp::init_variable_payload`] method.
>> + ///
>> + /// Error codes returned by the command and reply initializers are propagated as-is.
>> + pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
>> + where
>> + M: CommandToGsp,
>> + M::Reply: MessageFromGsp,
>> + Error: From<M::InitError>,
>> + Error: From<<M::Reply as MessageFromGsp>::InitError>,
>> + {
>> + self.send_command_internal(bar, command)?;
>> +
>> + loop {
>> + match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
>> + Ok(reply) => break Ok(reply),
>> + Err(ERANGE) => continue,
>> + Err(e) => break Err(e),
>> + }
>> + }
>
> There is an opportunity for factorize some more code here.
>
> Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
> `GspSequencer::run`) both use the same kind of loop, down to the same
> error handling. We could move that loop logic here and do it in a single
> place.
>
> In the future, we will probably want to add handlers for
> unexpected messages from the GSP and it will be easier if we receive all
> messages from a single place.
>
> This can be a separate patch from this one, but I think it makes sense
> to have that in this series.
>
> I expect the last patch to change a bit as a consequence of that - maybe
> we will need a `receive_msg_loop` or something in `CmdqInner`.
I agree we should migrate all callers and make Cmdq responsible for
draining / handling spontaneous messages from the GSP, but I was
planning on doing it in a separate patch series until now. I can put it
into this one though if you want though no worries.
WARNING: multiple messages have this Message-ID (diff)
From: "Eliot Courtney" <ecourtney@nvidia.com>
To: "Alexandre Courbot" <acourbot@nvidia.com>,
"Eliot Courtney" <ecourtney@nvidia.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Benno Lossin" <lossin@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Alistair Popple" <apopple@nvidia.com>,
"Joel Fernandes" <joelagnelf@nvidia.com>,
<nouveau@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
<rust-for-linux@vger.kernel.org>, "Zhi Wang" <zhiw@nvidia.com>,
"dri-devel" <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`
Date: Thu, 05 Mar 2026 10:34:10 +0900 [thread overview]
Message-ID: <DGUGNCEXKDD2.ETWE4H89AL00@nvidia.com> (raw)
In-Reply-To: <DGTZ9NLS9TPM.2NRF5C9VN2B7C@nvidia.com>
On Wed Mar 4, 2026 at 8:56 PM JST, Alexandre Courbot wrote:
>> + /// Sends `command` to the GSP and waits for the reply.
>> + ///
>> + /// # Errors
>> + ///
>> + /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
>> + /// not received within the timeout.
>> + /// - `EIO` if the variable payload requested by the command has not been entirely
>> + /// written to by its [`CommandToGsp::init_variable_payload`] method.
>> + ///
>> + /// Error codes returned by the command and reply initializers are propagated as-is.
>> + pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
>> + where
>> + M: CommandToGsp,
>> + M::Reply: MessageFromGsp,
>> + Error: From<M::InitError>,
>> + Error: From<<M::Reply as MessageFromGsp>::InitError>,
>> + {
>> + self.send_command_internal(bar, command)?;
>> +
>> + loop {
>> + match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
>> + Ok(reply) => break Ok(reply),
>> + Err(ERANGE) => continue,
>> + Err(e) => break Err(e),
>> + }
>> + }
>
> There is an opportunity for factorize some more code here.
>
> Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
> `GspSequencer::run`) both use the same kind of loop, down to the same
> error handling. We could move that loop logic here and do it in a single
> place.
>
> In the future, we will probably want to add handlers for
> unexpected messages from the GSP and it will be easier if we receive all
> messages from a single place.
>
> This can be a separate patch from this one, but I think it makes sense
> to have that in this series.
>
> I expect the last patch to change a bit as a consequence of that - maybe
> we will need a `receive_msg_loop` or something in `CmdqInner`.
I agree we should migrate all callers and make Cmdq responsible for
draining / handling spontaneous messages from the GSP, but I was
planning on doing it in a separate patch series until now. I can put it
into this one though if you want though no worries.
next prev parent reply other threads:[~2026-03-05 1:34 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-04 2:46 [PATCH v3 0/5] gpu: nova-core: gsp: add locking to Cmdq Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 2:46 ` [PATCH v3 1/5] gpu: nova-core: gsp: fix stale doc comments on command queue methods Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 11:25 ` Gary Guo
2026-03-04 11:25 ` Gary Guo
2026-03-04 11:55 ` Alexandre Courbot
2026-03-04 11:55 ` Alexandre Courbot
2026-03-04 2:46 ` [PATCH v3 2/5] gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 11:25 ` Gary Guo
2026-03-04 11:25 ` Gary Guo
2026-03-04 11:55 ` Alexandre Courbot
2026-03-04 11:55 ` Alexandre Courbot
2026-03-04 2:46 ` [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp` Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 11:27 ` Gary Guo
2026-03-04 11:27 ` Gary Guo
2026-03-04 11:56 ` Alexandre Courbot
2026-03-04 11:56 ` Alexandre Courbot
2026-03-05 1:34 ` Eliot Courtney [this message]
2026-03-05 1:34 ` Eliot Courtney
2026-03-05 2:10 ` Alexandre Courbot
2026-03-05 2:10 ` Alexandre Courbot
2026-03-05 7:44 ` Eliot Courtney
2026-03-05 7:44 ` Eliot Courtney
2026-03-05 10:40 ` Alexandre Courbot
2026-03-05 10:40 ` Alexandre Courbot
2026-03-04 14:17 ` Alexandre Courbot
2026-03-04 14:17 ` Alexandre Courbot
2026-03-05 1:29 ` Eliot Courtney
2026-03-05 1:29 ` Eliot Courtney
2026-03-05 1:37 ` Alexandre Courbot
2026-03-05 1:37 ` Alexandre Courbot
2026-03-04 2:46 ` [PATCH v3 4/5] gpu: nova-core: gsp: make `Cmdq` a pinned type Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 2:46 ` [PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq Eliot Courtney
2026-03-04 2:46 ` Eliot Courtney
2026-03-04 11:57 ` Alexandre Courbot
2026-03-04 11:57 ` Alexandre Courbot
2026-03-05 1:36 ` Eliot Courtney
2026-03-05 1:36 ` Eliot Courtney
2026-03-05 1:51 ` Alexandre Courbot
2026-03-05 1:51 ` Alexandre Courbot
2026-03-04 12:02 ` Alexandre Courbot
2026-03-04 12:02 ` Alexandre Courbot
2026-03-04 11:58 ` [PATCH v3 0/5] gpu: nova-core: gsp: add " Alexandre Courbot
2026-03-04 11:58 ` Alexandre Courbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DGUGNCEXKDD2.ETWE4H89AL00@nvidia.com \
--to=ecourtney@nvidia.com \
--cc=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=dakr@kernel.org \
--cc=dri-devel-bounces@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=gary@garyguo.net \
--cc=joelagnelf@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=zhiw@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.