rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Alistair Popple" <apopple@nvidia.com>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
	rust-for-linux@vger.kernel.org, dri-devel@lists.freedesktop.org,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	linux-kernel@vger.kernel.org, nouveau@lists.freedesktop.org
Subject: Re: [PATCH v2 05/10] gpu: nova-core: gsp: Add GSP command queue handling
Date: Tue, 30 Sep 2025 12:33:12 +0200	[thread overview]
Message-ID: <DD62F2EK4XHL.3QT9NHS42Q55R@kernel.org> (raw)
In-Reply-To: <qwqmek35wzu3chimtfsoftqvt4lw4g7btbvqzoc6qb4hoce3zq@pdu57fdxfb23>

On Tue Sep 30, 2025 at 2:36 AM CEST, Alistair Popple wrote:
> I feel like this is largely where the confusion/disagreement lies - what the
> `gsp` module provides versus what the `fw` module provides. My thinking is that
> the types don't leak outside the `gsp` module, the `fw` is really just providing
> raw binding information that the `gsp` module turns into something nice/abstract
> for the rest of nova-core in the form of functions (or types) to do whatever
> is needed.

The goal is that firmware specific structures (and the majority of their
implementation details) don't leak into the business logic of the driver code.

Of course, due to its nature the `gsp` module can't be entirely independent from
firmware specifics. However, think of it in different layers:

If we encapsulate the layout of firmware specific structures in new types (i.e.
wrap firmware structures into new types in the `fw` module), all changes to the
layout of a structure, new features, etc. are local to and can be handled
transparently by this abstraction layer.

Once we actually introduce a second firmware version, we will also introduce the
corresponding proc macros to annotate those structures accordingly with
`#[versions(GSP)]` and let the proc macro generate the version specific
structures, which subsequently allows us to treat the new types in the `fw`
module as if they'd be existing only once (and not per firmware version).

Hence, all the other code in the GSP module can be left untouched unless a new
firmware version introduces semantical changes (which should be rare).

This is not only a major advantage when it comes to maintainability (which is
one of the major motivations for the Nova project to begin with), but also leads
to a much cleaner structure and naturally separates the layout of data from its
semantics.

Yes, it does require a bit more code to set it up to begin with, but it avoids a
horrible mess leaking trivial firmware changes into the semantic layers of the
GSP implementation.

But even if that would never happen and the firmware interface would never
change (which I think will not and should not happen), the separation of how to
lay out data and the implementation of the semantics by itself adds clarity and
helps with maintainability.

The feedback Alex gave in this context looks pretty in line with the above.

I hope this helps.

- Danilo

  reply	other threads:[~2025-09-30 10:33 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22 11:30 [PATCH v2 00/11] gpu: nova-core: Boot GSP to RISC-V active Alistair Popple
2025-09-22 11:30 ` [PATCH v2 01/10] gpu: nova-core: Set correct DMA mask Alistair Popple
2025-09-22 15:25   ` Boqun Feng
2025-09-22 16:08   ` Danilo Krummrich
2025-09-23  2:16     ` John Hubbard
2025-09-23  4:29       ` Alistair Popple
2025-09-26 12:00         ` Danilo Krummrich
2025-09-29  0:19           ` Alistair Popple
2025-09-29  7:06             ` Danilo Krummrich
2025-09-29  7:39               ` Alistair Popple
2025-09-29 12:49                 ` Danilo Krummrich
2025-10-01  1:31                   ` [PATCH v2 01/10] gpu: nova-core: Set correct DMA mask^[ Alistair Popple
2025-09-24 19:43   ` [PATCH v2 01/10] gpu: nova-core: Set correct DMA mask Lyude Paul
2025-09-22 11:30 ` [PATCH v2 02/10] gpu: nova-core: Create initial Gsp Alistair Popple
2025-09-23 14:20   ` Alexandre Courbot
2025-09-24 20:13   ` Lyude Paul
2025-09-24 20:50     ` John Hubbard
2025-09-24 21:07       ` Lyude Paul
2025-09-24 21:15         ` John Hubbard
2025-09-22 11:30 ` [PATCH v2 03/10] gpu: nova-core: gsp: Create wpr metadata Alistair Popple
2025-09-24 20:24   ` Lyude Paul
2025-09-26  1:24     ` Alexandre Courbot
2025-09-29  0:29       ` Alistair Popple
2025-09-29 14:54         ` Alexandre Courbot
2025-09-26  1:24   ` Alexandre Courbot
2025-09-29  0:38     ` Alistair Popple
2025-09-22 11:30 ` [PATCH v2 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure Alistair Popple
2025-09-24 20:36   ` Lyude Paul
2025-09-29  0:44     ` Alistair Popple
2025-09-22 11:30 ` [PATCH v2 05/10] gpu: nova-core: gsp: Add GSP command queue handling Alistair Popple
2025-09-22 19:16   ` Timur Tabi
2025-09-24 22:03   ` Lyude Paul
2025-09-25  6:32     ` Alistair Popple
2025-09-26  2:20       ` Alexandre Courbot
2025-09-29  1:06         ` Alistair Popple
2025-09-29  7:24           ` Danilo Krummrich
2025-09-26  4:37   ` Alexandre Courbot
2025-09-29  6:19     ` Alistair Popple
2025-09-29 14:34       ` Alexandre Courbot
2025-09-29 14:38         ` Miguel Ojeda
2025-09-29 14:45           ` Alexandre Courbot
2025-09-30 11:41             ` Alistair Popple
2025-09-30 11:58               ` Miguel Ojeda
2025-10-01  0:42                 ` Alistair Popple
2025-09-30  0:36         ` Alistair Popple
2025-09-30 10:33           ` Danilo Krummrich [this message]
2025-09-30 13:36           ` Alexandre Courbot
2025-09-22 11:30 ` [PATCH v2 06/10] gpu: nova-core: gsp: Create rmargs Alistair Popple
2025-09-24 22:05   ` Lyude Paul
2025-09-26  7:27   ` Alexandre Courbot
2025-09-29  6:36     ` Alistair Popple
2025-09-29  7:18       ` Danilo Krummrich
2025-09-29  7:49         ` Alistair Popple
2025-09-22 11:30 ` [PATCH v2 07/10] gpu: nova-core: gsp: Create RM registry and sysinfo commands Alistair Popple
2025-09-22 19:10   ` Timur Tabi
2025-09-23  4:40     ` Alistair Popple
2025-09-23  4:46       ` Timur Tabi
2025-09-24 22:11   ` Lyude Paul
2025-09-22 11:30 ` [PATCH v2 08/10] nova-core: falcon: Add support to check if RISC-V is active Alistair Popple
2025-09-22 19:12   ` Timur Tabi
2025-09-23  1:07     ` John Hubbard
2025-09-23  4:23       ` Timur Tabi
2025-09-23  4:42       ` Alistair Popple
2025-09-24 22:12   ` Lyude Paul
2025-09-22 11:30 ` [PATCH v2 09/10] nova-core: falcon: Add support to write firmware version Alistair Popple
2025-09-24 22:14   ` Lyude Paul
2025-09-22 11:30 ` [PATCH v2 10/10] nova-core: gsp: Boot GSP Alistair Popple
2025-09-24 22:15   ` Lyude Paul
2025-09-24 22:24   ` John Hubbard
2025-09-25  3:09   ` Timur Tabi

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=DD62F2EK4XHL.3QT9NHS42Q55R@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=tzimmermann@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).