Message-Id: <DD62F2EK4XHL.3QT9NHS42Q55R@kernel.org>
Date: Tue, 30 Sep 2025 12:33:12 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Alistair Popple" <apopple@...dia.com>
Cc: "Alexandre Courbot" <acourbot@...dia.com>,
 <rust-for-linux@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
 "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>,
 "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "David
 Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Maarten
 Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard"
 <mripard@...nel.org>, "Thomas Zimmermann" <tzimmermann@...e.de>, "John
 Hubbard" <jhubbard@...dia.com>, "Joel Fernandes" <joelagnelf@...dia.com>,
 "Timur Tabi" <ttabi@...dia.com>, <linux-kernel@...r.kernel.org>,
 <nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH v2 05/10] gpu: nova-core: gsp: Add GSP command queue
 handling

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

Powered by blists - more mailing lists