Re: [PATCH v5 4/6] gpu: nova-core: add FbHal::frts_size() for GA100 support

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Apr 21, 2026 at 6:15 AM JST, Timur Tabi wrote:
> On Mon, 2026-04-20 at 21:27 +0100, Gary Guo wrote:
>> 
>> Ah, I see, so this is what the "supports_display" refers to in the commit
>> message?
>
> Kinda.  It refers to this FbHal function:
>
>     /// Returns `true` is display is supported.
>     fn supports_display(&self, bar: &Bar0) -> bool;
>
> It reads an arch-specific register to determine whether display is "supported".  There is no
> register that tells us "needs FRTS", and so that's why frts_size() exists.
>
>> > So I guess what I'm really trying to say is that I've done a lot of research into how this code
>> > works, and it's written the way I wrote it for a reason.
>> > 
>> > As for using a Trait to define a default value that is just overridden by GA100, that would be
>> > okay,
>> > but it wouldn't be any clearer or more accurate.
>> > 
>> > I personally don't like `super::tu102::frts_size_tu102()` and I think both tu102::frts_size()
>> > and
>> > ga102::frts_size() should just return u64::SZ_1M, but I don't care enough to make a stink about
>> > it.
>> 
>> Thanks, that's a very good explanation how the reason the code is written as is.
>> With the information above, I'm happy with current approach.
>> 
>> Acked-by: Gary Guo <gary@garyguo.net>
>
> Thanks.
>
>> I think it's important to note that not all of us have knowledge (or even
>> access) to some of the context that you have, so something that may seem obvious
>> to you could require some additional background or reasons to be added in commit
>> message or comment.
>
> Unfortunately, the process to boot GSP-RM is so convoluted that it's not really possible to document
> everything publicly.  A lot of it just porting OpenRM (and to some degree Nouveau) to Rust and
> saying "Well, this is how it is".  In fact, it's much easier to mimic OpenRM than it is to actually
> read the internal documentation to figure out what to do.
>
>> Also, in the commit message you mentioned
>> 
>> > Introduce FbHal method frts_size() to return the size of the FRTS
>> > window.  GA100 is a special case in that there is no FRTS, and so
>> > the size must be set to 0.
>> 
>> and this probably should be corrected.
>
> Ugh, I thought I caught all of these.
>
> Alex, if you wouldn't mind, change that last sentence to:
>
> "GA100 is a special case in that although there is an FRTS, its size must arbitrarily be set to 0."

Sure, I've fixed it on my staging branch.

With this last point cleared, you can consider this series merged as I
will push it as soon as drm-rust-next reopens. It also makes the
Blackwell series a bit smoother, so I'm happy we can push it first!

>
>
> If you update to the latest git, you can use the new "git history" command:
> https://github.blog/open-source/git/highlights-from-git-2-54/

Thanks for the pointer, I do commit log edits *a lot* and this is
definitely going to help!