Re: [PATCH v2 2/2] gpu: nova-core: add initial documentation

[Danilo Krummrich <dakr@kernel.org>]

On Fri, Feb 07, 2025 at 05:23:37PM +0900, Alexandre Courbot wrote:
> On Wed Feb 5, 2025 at 10:56 PM JST, Zhi Wang wrote:
> > On Tue,  4 Feb 2025 20:03:12 +0100
> > Danilo Krummrich <dakr@kernel.org> wrote:
> >> +  regressions with all 2nd level drivers.
> >> diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst
> >> new file mode 100644
> >> index 000000000000..5e66ec35c5e3
> >> --- /dev/null
> >> +++ b/Documentation/gpu/nova/core/todo.rst
> >> @@ -0,0 +1,445 @@
> >> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >> +
> >> +=========
> >> +Task List
> >> +=========
> >> +
> >
> > ...
> >
> >> +
> >> +Generic register abstraction
> >> +----------------------------
> >> +
> >> +Work out how register constants and structures can be automatically generated
> >> +through generalized macros.
> >> +
> >> +Example:
> >> +
> >> +.. code-block:: rust
> >> +
> >> +	register!(BOOT0, 0x0, u32, pci::Bar<SIZE>, Fields [
> >> +	   MINOR_REVISION(3:0, RO),
> >> +	   MAJOR_REVISION(7:4, RO),
> >> +	   REVISION(7:0, RO), // Virtual register combining major and minor rev.
> >> +	])
> >> +
> >
> > I think it is better not to tie this to pci::Bar and its operations. It
> > would be better to have a intermediate container as the macro param. The
> > container holds the register region vaddr pointer, size, read/write traits.
> > The macro expands it from there, thus, we can also use this on firmware
> > memory structures, e.g. GSP WPR2 info.
> 
> Another reason for not tying this to a particular bus is that Tegra
> doesn't use PCI to reach the registers of its integrated GPU. Maybe we
> can remove that parameter from the register!() macro and have read()
> take a generic argument for its `bar` parameter instead, so that method
> gets automatically specialized for every type of bus we need to use?

This is just an example, I do not mean to tie this to the PCI bus. But rather
make it generic, such that it can be tied to any I/O resource.

Being able to tie the generated code (but not the macro itself) to a specific
resource though would be nice to have.