The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Danilo Krummrich" <dakr@kernel.org>, "Gary Guo" <gary@garyguo.net>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	driver-core@lists.linux.dev, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org, nova-gpu@lists.linux.dev,
	dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH 08/10] gpu: nova-core: use projection for PFALCON and PFALCON2 registers
Date: Wed, 29 Jul 2026 20:51:32 +0100	[thread overview]
Message-ID: <DKBBF354W8PU.OSV8SJACE415@garyguo.net> (raw)
In-Reply-To: <DKBAX2P3POG1.3CVTICXR9RYHS@kernel.org>

On Wed Jul 29, 2026 at 8:28 PM BST, Danilo Krummrich wrote:
>> The offset for `subregion` is no different to the offset for `read()`. In both
>> cases you are supposed to only supply build-time const values; and in both cases
>> you can argue they are needed there due to const generics being too limited to
>> what they do.
>
> I see where you are coming from, but I see it from a different angle:
>
> An offset given to read() can legitimately be a runtime value that happens to be
> constrained in a way that the compiler can proof that it is within bounds (with
> dead code elimination doing the rest). We have reasonable use-cases where the
> offset given to read() is not a "true" constant, and hence build_assert! is the
> correct tool.
>
> subregion() is different in this regard, as we do not have reasonable use-cases
> where the offset is not a "true" constant. (And I would guess that this also
> made you choose the implementation you picked for this patch.)

I think it's the same. Imagine instead of register arrays, you have subregion
arrays.

>
> It just happens that we hit a language limitation where const evaluation of a
> "true" constant does not work, but that does not make build time assertion the
> correct tool, it just makes it a workaround for the existing limitation.
>
> This is also in line with the documentation in rust/kernel/build_assert.rs.
>
>> We probably could even just add a method that gives you a subregion based on
>> `IoLoc` similar to other methods. This actually might be a good idea that I'd
>> pursue in next version.
>
> So, you are proposing that subregion() takes an argument of type
> L: impl IoLoc<_>?
>
> Where would this argument come from?
>
> I assume you would need to bring back
>
> 	impl RegisterBase<Foo> for Bar {
> 	    const BASE: usize = 0xff;
> 	}
>
> impl blocks? Which seems like just adding boilerplate? Or do you have something
> else in mind?

No, I was thinking of

    struct MySubRegion([u8; 4096]);

    impl Register for MySubRegion {
        type Storage = Self;
        const OFFSET: usize = ...;
    }

this can be generated with `register!` macro if you can attach a specific type
and not have it generate `bitfield!()`.

E.g.

    struct MySubRegion([u8; 4096]);

    register! {
        pub SUBREGION: MySubRegion @ 0x1000;
    }


Best,
Gary

  reply	other threads:[~2026-07-29 19:51 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-21 16:54 [PATCH 00/10] rust: io: make register bases typed and remove relative registers Gary Guo
2026-07-21 16:54 ` [PATCH 01/10] rust: io: register: allow explicit base type specification Gary Guo
2026-07-24  6:38   ` Alexandre Courbot
2026-07-24 13:22     ` Gary Guo
2026-07-27  5:27       ` Eliot Courtney
2026-07-27  8:14         ` Alexandre Courbot
2026-07-28 19:12           ` John Hubbard
2026-07-21 16:54 ` [PATCH 02/10] gpu: nova-core: specify base type for registers Gary Guo
2026-07-28  6:47   ` Alexandre Courbot
2026-07-21 16:54 ` [PATCH 03/10] drm/tyr: " Gary Guo
2026-07-21 16:54 ` [PATCH 04/10] samples: rust: pci: " Gary Guo
2026-07-21 16:54 ` [PATCH 05/10] rust: io: register: make register have a typed base Gary Guo
2026-07-21 16:54 ` [PATCH 06/10] rust: io: add static `cast()` method for views Gary Guo
2026-07-21 16:54 ` [PATCH 07/10] rust: io: add subregion method with compile-time check Gary Guo
2026-07-28  7:21   ` Alexandre Courbot
2026-07-21 16:54 ` [PATCH 08/10] gpu: nova-core: use projection for PFALCON and PFALCON2 registers Gary Guo
2026-07-28  6:45   ` Alexandre Courbot
2026-07-28 10:24     ` Gary Guo
2026-07-28 15:09       ` Alexandre Courbot
2026-07-28 17:01         ` Danilo Krummrich
2026-07-28 18:26           ` Gary Guo
2026-07-28 18:56             ` Danilo Krummrich
2026-07-28 19:03               ` Gary Guo
2026-07-28 19:30                 ` Miguel Ojeda
2026-07-28 20:07                 ` Danilo Krummrich
2026-07-29 10:24                   ` Gary Guo
2026-07-29 19:28                     ` Danilo Krummrich
2026-07-29 19:51                       ` Gary Guo [this message]
2026-07-21 16:54 ` [PATCH 09/10] gpu: nova-core: convert hshub0 from relative register to projection Gary Guo
2026-07-21 16:54 ` [PATCH 10/10] rust: io: register: remove relative registers Gary Guo

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=DKBBF354W8PU.OSV8SJACE415@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /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