Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-08 16:58 UTC (permalink / raw)
  To: Eliot Courtney, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
	Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <DHNT32C2Q5HN.LLME0RV17Z8V@nvidia.com>

Hi Eliot,

On 4/8/2026 9:26 AM, Eliot Courtney wrote:
> On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
>> Hi Eliot,
>>
>> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
>>>>>> +    /// Compute upper bound on page table pages needed for `num_virt_pages`.
>>>>>> +    ///
>>>>>> +    /// Walks from PTE level up through PDE levels, accumulating the tree.
>>>>>> +    pub(crate) fn pt_pages_upper_bound(&self, num_virt_pages: usize) -> usize {
>>>>>> +        let mut total = 0;
>>>>>> +
>>>>>> +        // PTE pages at the leaf level.
>>>>>> +        let pte_epp = self.entries_per_page(self.pte_level());
>>>>>> +        let mut pages_at_level = num_virt_pages.div_ceil(pte_epp);
>>>>>> +        total += pages_at_level;
>>>>>> +
>>>>>> +        // Walk PDE levels bottom-up (reverse of pde_levels()).
>>>>>> +        for &level in self.pde_levels().iter().rev() {
>>>>>> +            let epp = self.entries_per_page(level);
>>>>>> +
>>>>>> +            // How many pages at this level do we need to point to
>>>>>> +            // the previous pages_at_level?
>>>>>> +            pages_at_level = pages_at_level.div_ceil(epp);
>>>>>> +            total += pages_at_level;
>>>>>> +        }
>>>>>> +
>>>>>> +        total
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>
>>>>> We have a lot of matches on the MMU version here (and below in Pte, Pde,
>>>>> DualPde). What about making MmuVersion into a trait (e.g. Mmu) with
>>>>> associated types for Pte, Pde, DualPde which can implement traits
>>>>> defining their common operations too?
>>>>
>>>> I coded this up and it did not look pretty, there's not much LOC savings and the
>>>> code becomes harder to read because of parametrization of several functions. Also:
>>>
>>> Thanks for looking into it. Sorry to be a bother, but would you have a
>>> branch around with the code? I'm curious what didn't look good about it.
>>
>> Sorry but I already mentioned that above, the parameterizing of dozens of
>> function call sites, 3-4 new traits (because each struct like
>> Pte/Pde/DualPde etc each need their own trait which different MMU versions
>> implement) etc. The code because hard to read and readability is the top
>> critical criteria for me - I am personally strictly against "Lets use shiny
>> features in language at the cost of making code unreadable". Because that
>> translates into bugs and nightmare for maintainability.
>>
>> I don't have the code at the moment, but if you still want to spend on time
>> on this direction, feel free to share a tree. I am happy to take a look.
> 
> I had a go at this, you can see the branch here [1] - it might not be
> perfect, but I think the shape is directionally good. It's structured so
> the HEAD commit has the diff from the current approach to the
> parametrised approach. The main decision is where to do the type
> erasure, I chose in `Vmm` since it looks like the main top level API for
> this code, but could do `BarUser` instead. I think it's overall better.
> I also think Alex's point about associated types making it easier to use
> the appropriate Bounded type is a good one.
> 
> [1]: https://github.com/Edgeworth/linux/commits/review/nova-mm-v10/
First, thanks for the effort. I looked through this, its pretty much what I
had before when I used traits. I don't think it is better to be honest. In
fact your version is worse, it adds many new types and things like the
following which I did not need before.

To put it mildly, the following suggestion should not be anywhere near my code:

/// Type-erased MMU-specific [`Vmm`] implementations.
enum VmmInner {
    /// `Vmm` implementation for MMU v2.
    V2(VmmImpl<MmuV2>),
    /// `Vmm` implementation for MMU v3.
    V3(VmmImpl<MmuV3>),
}

/// MMU-specific [`Vmm`] implementation.
struct VmmImpl<M: Mmu> {

Seriously, I have to pass on this. :-)

And, you unfortunately seem to have ignored my point about requiring 4 NEW
traits (Mmu, PteOps, PdeOps, DualPdeOps etc), which I did not need before.
So you're making the code much much worse than before actually. We don't
new traits and types pointlessly.

The only positive thing I could take away from your diff is the following
(I thought I had already done that, but I'll double check).

-    fn level_index(&self, level: u64) -> u64 {
+    fn level_index(&self, level: PageTableLevel) -> u64 {

Also you're parametrizing VirtualAddress as well which I did not have before:

-     let va = VirtualAddress::from(vfn);
+     let va = M::va(VirtualAddress::from(vfn));

This is another step back.

> I also think Alex's point about associated types making it easier to use
> the appropriate Bounded type is a good one.

I will reply to Alex thread, separately. I have some good data that should
hopefully convince you and Alex that my approach in this patch is better
(Version struct based dispatch than monomorphization). I would emphasize,
as we all know, that we should make optimizations and changes based on real
data and proper technical arguments so in the spirit of that, I have
collected data with both approaches and I will reply to Alex's email with
all that in there.

Also, the bounded types usage is orthogonal to version-parameterization.
That can be done regardless, we already use bitfield macro in this code and
can use bounded types within that if needed to restrict type creation. So I
don't think we should mix the 2 concepts "bounded types" and
"parameterization".

thanks,

--
Joel Fernandes



^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Danilo Krummrich @ 2026-04-08 18:01 UTC (permalink / raw)
  To: Joel Fernandes, Eliot Courtney
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
	Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <da8d03f8-0294-417b-b684-2c20d577f94a@nvidia.com>

On Wed Apr 8, 2026 at 6:58 PM CEST, Joel Fernandes wrote:
> So you're making the code much much worse than before actually. We don't
> new traits and types pointlessly.

I had a look at both approaches and yes, the traits can be considered
boilerplate. But, they are not complex and they just list method signatures that
each version's types already implement functionality wise and they get us rid of
a lot of dispatch sites. The implementation turns out cleaner as there is less
parameter threading throughout call chains, etc. Overall, it seems more
scalable.

On the other hand, there are indeed more abstractions and type indirections to
understand in Eliot's code. I.e. there are advantages and disadvantages to both
approaches.

That said, please engage with Eliot's proposal, it is not as off as your reply
implies and dismissing it right away is not what I'd like to see in this
situation.

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-08 19:04 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Eliot Courtney, linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Vivi Rodrigo, Tvrtko Ursulin, Rui Huang, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Alexandre Courbot, Andrea Righi,
	Andy Ritger, Zhi Wang, Balbir Singh, Philipp Stanner,
	Elle Rhumsaa, alexeyi, joel, linux-doc, amd-gfx, intel-gfx,
	intel-xe, linux-fbdev
In-Reply-To: <DHNYXGAVLNVM.3TV1VDZ9YXA9U@kernel.org>

On Wed, Apr 08, 2026 at 08:01:01PM +0200, Danilo Krummrich <dakr@kernel.org> wrote:
> On Wed Apr 8, 2026 at 6:58 PM CEST, Joel Fernandes wrote:
>> So you're making the code much much worse than before actually. We don't
>> new traits and types pointlessly.
>
> I had a look at both approaches and yes, the traits can be considered
> boilerplate. But, they are not complex and they just list method signatures that
> each version's types already implement functionality wise and they get us rid of
> a lot of dispatch sites. The implementation turns out cleaner as there is less
> parameter threading throughout call chains, etc. Overall, it seems more
> scalable.
>
> On the other hand, there are indeed more abstractions and type indirections to
> understand in Eliot's code. I.e. there are advantages and disadvantages to both
> approaches.
>
> That said, please engage with Eliot's proposal, it is not as off as your reply
> implies and dismissing it right away is not what I'd like to see in this
> situation.

No one is “dismissing it right away”. If you read all the threads, you will see
that I already came up with the same approach and have spent time coding
it up myself to learn its potential advantages and drawbacks. :-) I would never
dismiss anyone’s suggestion just for the sake of it, without evaluating. Hence
most of my morning went in collecting data with both approaches (more on that in
another upcoming reply).

Also yes of course we have to look at all the pros and cons and carefully make a
choice collectively, there are tradeoffs in both approaches.

thanks,

--
Joel Fernandes

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-08 20:19 UTC (permalink / raw)
  To: Alexandre Courbot, Eliot Courtney, Danilo Krummrich
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <DHNKYBM159T9.2UUQ7CU0RN0BU@nvidia.com>

Hi Alex, Eliot, Danilo,

Thanks for taking a look. Let me respond to the specific points below.

On Wed, 08 Apr 2026, Alexandre Courbot wrote:
> After a quick look I'd say that having a trait here would actually be
> *good* for correctness and maintainability.
>
> The current design implies that every operation on a page table (most
> likely using the walker) goes through a branching point. Just looking at
> `PtWalk::read_pte_at_level`, there are already at least 6
> `if version == 2 { } else { }` branches that all resolve to the same
> result. Include walking down the PDEs and you have at least a dozen of
> these just to resolve a virtual address. I know CPUs are fast, but this
> is still wasted cycles for no good reason.

I did some measurements and there is no notieceable difference in both
approaches. I ran perf and loaded nova with self-tests running. The extra
potential branching is lost in the noise. In both cases, loading nova and
running the self-tests has ~119.7M branch instructions on my Ampere. The total
instruction count is also identical (~615M).

I measured like this:
perf stat -e
branches,branch-misses,cache-references,cache-misses,instructions,cycles --
modprobe nova_core

So I think the branching argument is not a strong one. I also did more
measurements and the dominant time taken is MMIO. During the map prep and
execute, page table walks are done. A TLB flush alone costs ~1.4 microseconds.
And PRAMIN BAR0 writes to write the PTE is also about 1 microsecond. Considering
this, I don't think the extra branching argument holds (even without branch
prediction and speculation).

Also some branches cannot be eliminated even with parameterization:

    if level == self.mmu_version.dual_pde_level() {
        // 128-bit dual PDE read
    } else {
        // Regular 64-bit PDE read
    }

This isn't really a version branch -- it's a structural branch that
distinguishes between 64-bit PDE and 128-bit dual PDE entries. Any MMU
version with a dual PDE level would need this same distinction.

I also did code-generation size analysis (see diff of code used below):

Code generation analysis:

  Module .ko size:   Before: 511,792 bytes   After: 524,464 bytes  (+2.5%)
  .text section:     Before: 112,620 bytes   After: 116,628 bytes  (+4,008 bytes)

  The +4K .text growth is the monomorphization cost: every generic function
  is compiled twice (once for MmuV2, once for MmuV3).

> If you use a trait here, and make `PtWalk` generic against it, you can
> optimize this away. We had a similar situation when we introduced Turing
> support and the v2 ucode header, and tried both approaches: the
> trait-based one was slightly shorter, and arguably more readable.

Actually I was the one who suggested traits for Falcon ucode descriptor if you
see this thread [1]. So basically you and Eliot are telling me to do what I
suggested in [1]. :-) However, I disagree that it is the right choice for this code.

[1] https://lore.kernel.org/all/20251117231028.GA1095236@joelbox2/

I think the two cases are quite different in complexity:

The falcon ucode descriptor is essentially a set of flat field accessors
and a few params (imem_sec_load_params, dmem_load_params).
The trait has ~10 simple getter methods. There's no multi-level hierarchy,
no walker, and no generic propagation.

The MMU page table case is structurally different. Making PtWalk generic
over an Mmu trait would require:

  - PtWalk<M: Mmu> (the walker)
  - Plus all the associated types: M::Pte, M::Pde, M::DualPde each
    needing their own trait bounds

And we would also need:
  - Vmm<M: Mmu> (which creates PtWalk)
  - BarUser<M: Mmu> (which creates Vmm)

I am also against making Vmm an enum as Eliot suggested:
       enum Vmm {
           V2(VmmInner<MmuV2>),
           V3(VmmInner<MmuV3>),
       }

That moves the version complexity up to the reader. Code complexity IMO should
decrease as we go up abstractions, making it easier for users (Vmm/Bar).

If you look at the the changes in vmm.rs to handle version dispatch there [2]:
Added: +109
Removed: -28

[2]
https://github.com/Edgeworth/linux/commit/3627af550b61256184d589e7ec666c1108971f0e

The main benefit of my approach is version-specific dispatch complexity is
completely isolated inside MmuVersion thus making the code outside of
pagetable.rs much more readable, without having to parametrize anything, and
without code size increase. I think that is worth considering.

> But the main argument to use a trait here IMO is that it enables
> associated types and constants. That's particularly critical since some
> equivalent fields have different lengths between v2 and v3. An
> associated `Bounded` type for these would force the caller to validate
> the length of these fields before calling a non-fallible operation,
> which is exactly the level of caution that we want when dealing with
> page tables.

I think Bounded validation is orthogonal to the dispatch model.
We can add Bounded to the current design without restructuring
into traits. For example:

    // In ver2::Pte
    pub fn new_vram(pfn: Bounded<Pfn, 25>, writable: bool) -> Self { ... }

    // In ver3::Pte
    pub fn new_vram(pfn: Bounded<Pfn, 40>, writable: bool) -> Self { ... }

The unified Pte enum wrapper already dispatches to the correct
version-specific constructor, which would enforce the correct Bounded
constraint for that version.

> In order to fully benefit from it, we will need the bitfield macro from
> the `kernel` crate so the PDE/PTE fields can be `Bounded`, I will try to
> make it available quickly in a patch that you can depend on.

That would be great, and I'd be happy to integrate Bounded validation once
the macro is available. I just don't think we need to restructure the
dispatch model in order to benefit from it.

> But long story short, and although I need to dive deeper into the code,
> this looks like a good candidate for using a trait and associated types.

The walker code (walk.rs) is already version-agnostic and reads cleanly.
The version dispatch is encapsulated behind method calls, not exposed as
inline if/else blocks.

Generic propagation (or version-specific dispatch at higher levels) adds more
complexity at higher layers.

Enclosed below [3] is the diff I used for my testing with the data, I don't
really see a net readability win there (IMO, it is a net-loss in readability).

[3]
https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=trait-pt-dispatch&id=5eb0e98af11ba608ff4d0f7a06065ee863f5066a

thanks,

--
Joel Fernandes


^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: John Hubbard @ 2026-04-08 23:13 UTC (permalink / raw)
  To: Joel Fernandes, Eliot Courtney, linux-kernel
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <da8d03f8-0294-417b-b684-2c20d577f94a@nvidia.com>

On 4/8/26 9:58 AM, Joel Fernandes wrote:
> On 4/8/2026 9:26 AM, Eliot Courtney wrote:
>> On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
>>> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>>>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
...>> [1]: https://github.com/Edgeworth/linux/commits/review/nova-mm-v10/
> First, thanks for the effort. I looked through this, its pretty much what I
> had before when I used traits. I don't think it is better to be honest. In
> fact your version is worse, it adds many new types and things like the
> following which I did not need before.

Hi Joel and all,

I also looked through Eliot's above attempt carefully, and actually
liked it a lot (sorry! haha):

* It cleans up the code. The initial working version was readable, but
  also had lots of noise on the screen: match statements and pairs of
  v2/v3 statements.

  And interestingly, the mmu_version was, in effect, sporadically
  implementing a Trait-based approach. But because it is custom,
  readers don't benefit as much as they would with Traits, which
  tell you immediately how things are structured.

Joel, I am passionately in agreement with your principles: code must
be readable on the screen.

In this case, though, Traits make considerably more readable,
especially if one makes the very reasonable assumption that readers are
thoroughly accustomed to dealing with Rust traits.

> 
> To put it mildly, the following suggestion should not be anywhere near my code:
> 

lol I understand, believe me. But this is short and not too bad, really.

> /// Type-erased MMU-specific [`Vmm`] implementations.

Type erasure remains a semi-exotic thing, IMHO. As such, another
sentence to elaborate on this would be a nice touch.

> enum VmmInner {
>     /// `Vmm` implementation for MMU v2.
>     V2(VmmImpl<MmuV2>),
>     /// `Vmm` implementation for MMU v3.
>     V3(VmmImpl<MmuV3>),
> }
> 
> /// MMU-specific [`Vmm`] implementation.
> struct VmmImpl<M: Mmu> {
> 
> Seriously, I have to pass on this. :-)
> 
> And, you unfortunately seem to have ignored my point about requiring 4 NEW
> traits (Mmu, PteOps, PdeOps, DualPdeOps etc), which I did not need before.
> So you're making the code much much worse than before actually. We don't
> new traits and types pointlessly.

They are not pointless.

However! What I think would be nice is: do a new v11 with approximately
this approach, and then we can beat it into being as readable as 
possible.
 

thanks,
-- 
John Hubbard


^ permalink raw reply

* [PATCH v2] MAINTAINERS: Add dedicated entry for fbcon
From: Thomas Zimmermann @ 2026-04-09  7:26 UTC (permalink / raw)
  To: deller, simona; +Cc: dri-devel, linux-fbdev, linux-kernel, Thomas Zimmermann

Add an own entry for the framebuffer console code, which is jointly
used by the fbdev and DRM drivers.

v2:
- add Helge as maintainer
- list font and include files
- update commit message (Helge)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 MAINTAINERS | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e80296449ba..66769dcf5faa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10091,6 +10091,29 @@ S:	Maintained
 W:	https://floatingpoint.billm.au/
 F:	arch/x86/math-emu/
 
+FRAMEBUFFER CONSOLE
+M:	Helge Deller <deller@gmx.de>
+M:	Thomas Zimmermann <tzimmermann@suse.de>
+L:	dri-devel@lists.freedesktop.org
+L:	linux-fbdev@vger.kernel.org
+S:	Maintained
+T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
+F:	Documentation/fb/fbcon.rst
+F:	drivers/video/fbdev/core/bitblit.c
+F:	drivers/video/fbdev/core/fb_logo.c
+F:	drivers/video/fbdev/core/fbcon.c
+F:	drivers/video/fbdev/core/fbcon.h
+F:	drivers/video/fbdev/core/fbcon_ccw.c
+F:	drivers/video/fbdev/core/fbcon_cw.c
+F:	drivers/video/fbdev/core/fbcon_rotate.c
+F:	drivers/video/fbdev/core/fbcon_rotate.h
+F:	drivers/video/fbdev/core/fbcon_ud.c
+F:	drivers/video/fbdev/core/softcursor.c
+F:	drivers/video/fbdev/core/tileblit.c
+F:	include/linux/fbcon.h
+F:	include/linux/font.h
+F:	lib/fonts/
+
 FRAMEBUFFER CORE
 M:	Simona Vetter <simona@ffwll.ch>
 S:	Odd Fixes
-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v2] MAINTAINERS: Add dedicated entry for fbcon
From: Helge Deller @ 2026-04-09  8:13 UTC (permalink / raw)
  To: Thomas Zimmermann, simona; +Cc: dri-devel, linux-fbdev, linux-kernel
In-Reply-To: <20260409072711.12500-1-tzimmermann@suse.de>

On 4/9/26 09:26, Thomas Zimmermann wrote:
> Add an own entry for the framebuffer console code, which is jointly
> used by the fbdev and DRM drivers.
> 
> v2:
> - add Helge as maintainer
> - list font and include files
> - update commit message (Helge)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   MAINTAINERS | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
applied.

Thanks!
Helge

^ permalink raw reply

* Re: [PATCH] fbdev: omap2: fix inconsistent lock returns in omapfb_mmap
From: Helge Deller @ 2026-04-09  8:29 UTC (permalink / raw)
  To: zenghongling, linux-omap, linux-fbdev, dri-devel
  Cc: linux-kernel, zhongling0719, kernel test robot
In-Reply-To: <20260402093403.12173-1-zenghongling@kylinos.cn>

On 4/2/26 11:34, zenghongling wrote:
> Fix the warning about inconsistent returns for '&rg->lock' in
> omapfb_mmap() function. The warning arises because the error path
> uses 'ofbi->region' while the normal path uses 'rg'.
> 
> smatch warnings:
> drivers/video/fbdev/omap2/omapfb/omapfb-main.c:1126 omapfb_mmap()
> warn: inconsistent returns '&rg->lock'.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: zenghongling <zenghongling@kylinos.cn>
> ---
>   drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

applied.
PS: I replaced author/email name by "Hongling Zeng" instead of "zenghongling".
I assume that's Ok, if not please let me know.

Thanks!
Helge

^ permalink raw reply

* 回复: Re: [PATCH] fbdev: omap2: fix inconsistent lock returns in omapfb_mmap
From: 曾红玲 @ 2026-04-09  9:33 UTC (permalink / raw)
  To: linux-omap, linux-fbdev, dri-devel, Helge Deller
  Cc: linux-kernel, zhongling0719, kernel test robot

[-- Attachment #1: Type: text/html, Size: 2312 bytes --]

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Joel Fernandes @ 2026-04-09 10:33 UTC (permalink / raw)
  To: John Hubbard
  Cc: Joel Fernandes, Eliot Courtney, linux-kernel, Miguel Ojeda,
	Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Vivi Rodrigo, Tvrtko Ursulin, Rui Huang, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, Alexey Ivanov,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <42dd707f-e23a-4725-8b6f-08ca346b0143@nvidia.com>

> On Apr 8, 2026, at 7:13 PM, John Hubbard <jhubbard@nvidia.com> wrote:
> 
> On 4/8/26 9:58 AM, Joel Fernandes wrote:
>>> On 4/8/2026 9:26 AM, Eliot Courtney wrote:
>>> On Tue Apr 7, 2026 at 10:59 PM JST, Joel Fernandes wrote:
>>>> On 4/7/2026 9:42 AM, Eliot Courtney wrote:
>>>>> On Tue Apr 7, 2026 at 6:55 AM JST, Joel Fernandes wrote:
> ...>> [1]: https://github.com/Edgeworth/linux/commits/review/nova-mm-v10/
>> First, thanks for the effort. I looked through this, its pretty much what I
>> had before when I used traits. I don't think it is better to be honest. In
>> fact your version is worse, it adds many new types and things like the
>> following which I did not need before.
> 
> Hi Joel and all,
> 
> I also looked through Eliot's above attempt carefully, and actually
> liked it a lot (sorry! haha):
> 
> * It cleans up the code. The initial working version was readable, but
>  also had lots of noise on the screen: match statements and pairs of
>  v2/v3 statements.
> 
>  And interestingly, the mmu_version was, in effect, sporadically
>  implementing a Trait-based approach. But because it is custom,
>  readers don't benefit as much as they would with Traits, which
>  tell you immediately how things are structured.
> 
> Joel, I am passionately in agreement with your principles: code must
> be readable on the screen.
> 
> In this case, though, Traits make considerably more readable,
> especially if one makes the very reasonable assumption that readers are
> thoroughly accustomed to dealing with Rust traits.
> 
>> 
>> To put it mildly, the following suggestion should not be anywhere near my code:
>> 
> 
> lol I understand, believe me. But this is short and not too bad, really.
> 
>> /// Type-erased MMU-specific [`Vmm`] implementations.
> 
> Type erasure remains a semi-exotic thing, IMHO. As such, another
> sentence to elaborate on this would be a nice touch.
> 
>> enum VmmInner {
>>    /// `Vmm` implementation for MMU v2.
>>    V2(VmmImpl<MmuV2>),
>>    /// `Vmm` implementation for MMU v3.
>>    V3(VmmImpl<MmuV3>),
>> }
>> 
>> /// MMU-specific [`Vmm`] implementation.
>> struct VmmImpl<M: Mmu> {
>> 
>> Seriously, I have to pass on this. :-)
>> 
>> And, you unfortunately seem to have ignored my point about requiring 4 NEW
>> traits (Mmu, PteOps, PdeOps, DualPdeOps etc), which I did not need before.
>> So you're making the code much much worse than before actually. We don't
>> new traits and types pointlessly.
> 
> They are not pointless.
> 
> However! What I think would be nice is: do a new v11 with approximately
> this approach, and then we can beat it into being as readable as
> possible.

Since it is 3 against 1 here, I rest my case :-). I am still in
disagreement since I do not see much benefit (that is why I said
pointless above). Actually it is not even about readability, that is
subjective (and I haven’t heard most people say parametrizing code for
the sake of it makes it more readable anyway). It is that the code gen
is worse, and the complexity is just moved to a higher level in the
code, not removed. So what are we getting out of this really, other than
more boiler plate in higher layers of the code that did not exist
before? Not performance, not better generated code. Really nothing. See
all the data points in my previous reply.

Note that if the mmu version threading bothers everyone so much, we can
also pass down chipset instead and let the walker deal with determining
versioning. Would that be better?

But otherwise and since you guys asked, here comes a parameterized v11... ;-).
  (Coming next week since this week I’m working on IRQ handling).

thanks,

-- 
Joel Fernandes

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Alexandre Courbot @ 2026-04-09 10:56 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Eliot Courtney, Danilo Krummrich, linux-kernel, Miguel Ojeda,
	Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Dave Airlie,
	Daniel Almeida, Koen Koning, dri-devel, rust-for-linux,
	Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <2f004511-61d1-4197-84b6-cddcdd275e55@nvidia.com>

On Thu Apr 9, 2026 at 5:19 AM JST, Joel Fernandes wrote:
> Hi Alex, Eliot, Danilo,
>
> Thanks for taking a look. Let me respond to the specific points below.
>
> On Wed, 08 Apr 2026, Alexandre Courbot wrote:
>> After a quick look I'd say that having a trait here would actually be
>> *good* for correctness and maintainability.
>>
>> The current design implies that every operation on a page table (most
>> likely using the walker) goes through a branching point. Just looking at
>> `PtWalk::read_pte_at_level`, there are already at least 6
>> `if version == 2 { } else { }` branches that all resolve to the same
>> result. Include walking down the PDEs and you have at least a dozen of
>> these just to resolve a virtual address. I know CPUs are fast, but this
>> is still wasted cycles for no good reason.
>
> I did some measurements and there is no notieceable difference in both
> approaches. I ran perf and loaded nova with self-tests running. The extra
> potential branching is lost in the noise. In both cases, loading nova and
> running the self-tests has ~119.7M branch instructions on my Ampere. The total
> instruction count is also identical (~615M).

That's expected - as I said, CPUs are fast - and that's also not my
point. My issue is that we are doing countless tests that all resolve to
the code path, a code path that is already known during probe time.
That's a huge code smell.

When we create the GPU, we know whether we will be using v2 or v3 page
tables. That we need to test that again 12 times per address resolution
is a design issue, irrespective of performance. There are 24 version
match sites in patch 12 alone.

And that's precisely a good justification for using monomorphization. v2
and v3 are technically two different page table implementations (they
even have their own distinct module in your series), we just use
generics to factorize the (source) code a bit.

>
> I measured like this:
> perf stat -e
> branches,branch-misses,cache-references,cache-misses,instructions,cycles --
> modprobe nova_core
>
> So I think the branching argument is not a strong one. I also did more
> measurements and the dominant time taken is MMIO. During the map prep and
> execute, page table walks are done. A TLB flush alone costs ~1.4 microseconds.
> And PRAMIN BAR0 writes to write the PTE is also about 1 microsecond. Considering
> this, I don't think the extra branching argument holds (even without branch
> prediction and speculation).
>
> Also some branches cannot be eliminated even with parameterization:
>
>     if level == self.mmu_version.dual_pde_level() {
>         // 128-bit dual PDE read
>     } else {
>         // Regular 64-bit PDE read
>     }
>
> This isn't really a version branch -- it's a structural branch that
> distinguishes between 64-bit PDE and 128-bit dual PDE entries. Any MMU
> version with a dual PDE level would need this same distinction.

The dual PDE level should be an associated constant - you still need to
do the test, but note that you would also do it if there was only a
single page table version. It's orthogonal to whether we use a trait or
not here.

>
> I also did code-generation size analysis (see diff of code used below):
>
> Code generation analysis:
>
>   Module .ko size:   Before: 511,792 bytes   After: 524,464 bytes  (+2.5%)
>   .text section:     Before: 112,620 bytes   After: 116,628 bytes  (+4,008 bytes)
>
>   The +4K .text growth is the monomorphization cost: every generic function
>   is compiled twice (once for MmuV2, once for MmuV3).

I would say this is working as intended then.

>
>> If you use a trait here, and make `PtWalk` generic against it, you can
>> optimize this away. We had a similar situation when we introduced Turing
>> support and the v2 ucode header, and tried both approaches: the
>> trait-based one was slightly shorter, and arguably more readable.
>
> Actually I was the one who suggested traits for Falcon ucode descriptor if you
> see this thread [1]. So basically you and Eliot are telling me to do what I
> suggested in [1]. :-) However, I disagree that it is the right choice for this code.
>
> [1] https://lore.kernel.org/all/20251117231028.GA1095236@joelbox2/
>
> I think the two cases are quite different in complexity:

Exactly. The complexity is different (this one involves multiple traits
and associated types) but the pattern is the same - and that's a pattern
traits are designed to address. If we were supposed to stop applying it
when things go beyond a certain level of complexity, the conceptors of
Rust would not have bothered addings things like associated types.

These traits are nothing new, they simply formalize a reality that
already exists in your code, which is that each version of the page
table needs to implement a given set of methods. It's already there with
the version doing dispatches, only it is not articulated clearly to the
reader. So in that respect, having traits make the code *more* readable
imho.

>
> The falcon ucode descriptor is essentially a set of flat field accessors
> and a few params (imem_sec_load_params, dmem_load_params).
> The trait has ~10 simple getter methods. There's no multi-level hierarchy,
> no walker, and no generic propagation.
>
> The MMU page table case is structurally different. Making PtWalk generic
> over an Mmu trait would require:
>
>   - PtWalk<M: Mmu> (the walker)
>   - Plus all the associated types: M::Pte, M::Pde, M::DualPde each
>     needing their own trait bounds
>
> And we would also need:
>   - Vmm<M: Mmu> (which creates PtWalk)
>   - BarUser<M: Mmu> (which creates Vmm)
>
> I am also against making Vmm an enum as Eliot suggested:
>        enum Vmm {
>            V2(VmmInner<MmuV2>),
>            V3(VmmInner<MmuV3>),
>        }
>
> That moves the version complexity up to the reader. Code complexity IMO should
> decrease as we go up abstractions, making it easier for users (Vmm/Bar).
>
> If you look at the the changes in vmm.rs to handle version dispatch there [2]:
> Added: +109
> Removed: -28
>
> [2]
> https://github.com/Edgeworth/linux/commit/3627af550b61256184d589e7ec666c1108971f0e
>
> The main benefit of my approach is version-specific dispatch complexity is
> completely isolated inside MmuVersion thus making the code outside of
> pagetable.rs much more readable, without having to parametrize anything, and
> without code size increase. I think that is worth considering.
>
>> But the main argument to use a trait here IMO is that it enables
>> associated types and constants. That's particularly critical since some
>> equivalent fields have different lengths between v2 and v3. An
>> associated `Bounded` type for these would force the caller to validate
>> the length of these fields before calling a non-fallible operation,
>> which is exactly the level of caution that we want when dealing with
>> page tables.
>
> I think Bounded validation is orthogonal to the dispatch model.
> We can add Bounded to the current design without restructuring
> into traits. For example:
>
>     // In ver2::Pte
>     pub fn new_vram(pfn: Bounded<Pfn, 25>, writable: bool) -> Self { ... }
>
>     // In ver3::Pte
>     pub fn new_vram(pfn: Bounded<Pfn, 40>, writable: bool) -> Self { ... }
>
> The unified Pte enum wrapper already dispatches to the correct
> version-specific constructor, which would enforce the correct Bounded
> constraint for that version.

But then what type does the `new_vram` dispatch method take? Generic
code lets us expose the expected `Bounded` type to the caller, which can
do the proper validation. This is a small example, but I expect this
pattern to come up in other parts of the code as well.

>
>> In order to fully benefit from it, we will need the bitfield macro from
>> the `kernel` crate so the PDE/PTE fields can be `Bounded`, I will try to
>> make it available quickly in a patch that you can depend on.
>
> That would be great, and I'd be happy to integrate Bounded validation once
> the macro is available. I just don't think we need to restructure the
> dispatch model in order to benefit from it.

I'll finish the series and hopefully send it a bit later today. That's
another significant rework for the series (sorry about that) but it
should be worth the effort for the added correctness.

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Danilo Krummrich @ 2026-04-09 11:00 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: John Hubbard, Eliot Courtney, linux-kernel, Miguel Ojeda,
	Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Dave Airlie,
	Daniel Almeida, Koen Koning, dri-devel, rust-for-linux,
	Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Vivi Rodrigo, Tvrtko Ursulin, Rui Huang, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, Alistair Popple, Timur Tabi, Edwin Peer,
	Alexandre Courbot, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, Alexey Ivanov,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <1775730646.3752.4760@nvidia.com>

On Thu Apr 9, 2026 at 12:33 PM CEST, Joel Fernandes wrote:
> Since it is 3 against 1 here, I rest my case :-).

That's not how I'd view it. :)

Anyways, in case I'm included in "3", that's not my position. My point was to
ensure we keep discussing advantages and disadvantages on their merits, as I
think you both have good points.

> I am still in disagreement since I do not see much benefit (that is why I said
> pointless above).

That is fair -- in this case please explain why the advantages pointed out by
others are not worth it, propose something that picks up the best of both
worlds, etc.

You can also turn it around and ask people whether they can tweak their counter
proposal to get rid of specific parts you dislike for a reason.

IOW, keep the ball rolling, so we can come up with the best possible solution.

^ permalink raw reply

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: Gary Guo @ 2026-04-09 11:02 UTC (permalink / raw)
  To: Joel Fernandes, Alexandre Courbot, Eliot Courtney,
	Danilo Krummrich
  Cc: linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Dave Airlie, Daniel Almeida, Koen Koning, dri-devel,
	rust-for-linux, Nikola Djukic, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Jonathan Corbet,
	Alex Deucher, Christian Koenig, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, Tvrtko Ursulin, Huang Rui, Matthew Auld,
	Matthew Brost, Lucas De Marchi, Thomas Hellstrom, Helge Deller,
	Alex Gaynor, Boqun Feng, John Hubbard, Alistair Popple,
	Timur Tabi, Edwin Peer, Andrea Righi, Andy Ritger, Zhi Wang,
	Balbir Singh, Philipp Stanner, Elle Rhumsaa, alexeyi, joel,
	linux-doc, amd-gfx, intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <2f004511-61d1-4197-84b6-cddcdd275e55@nvidia.com>

On Wed Apr 8, 2026 at 9:19 PM BST, Joel Fernandes wrote:
> Hi Alex, Eliot, Danilo,
>
> Thanks for taking a look. Let me respond to the specific points below.
>
> On Wed, 08 Apr 2026, Alexandre Courbot wrote:
>> After a quick look I'd say that having a trait here would actually be
>> *good* for correctness and maintainability.
>>
>> The current design implies that every operation on a page table (most
>> likely using the walker) goes through a branching point. Just looking at
>> `PtWalk::read_pte_at_level`, there are already at least 6
>> `if version == 2 { } else { }` branches that all resolve to the same
>> result. Include walking down the PDEs and you have at least a dozen of
>> these just to resolve a virtual address. I know CPUs are fast, but this
>> is still wasted cycles for no good reason.
>
> I did some measurements and there is no notieceable difference in both
> approaches. I ran perf and loaded nova with self-tests running. The extra
> potential branching is lost in the noise. In both cases, loading nova and
> running the self-tests has ~119.7M branch instructions on my Ampere. The total
> instruction count is also identical (~615M).
>
> I measured like this:
> perf stat -e
> branches,branch-misses,cache-references,cache-misses,instructions,cycles --
> modprobe nova_core
>
> So I think the branching argument is not a strong one. I also did more
> measurements and the dominant time taken is MMIO. During the map prep and
> execute, page table walks are done. A TLB flush alone costs ~1.4 microseconds.
> And PRAMIN BAR0 writes to write the PTE is also about 1 microsecond. Considering
> this, I don't think the extra branching argument holds (even without branch
> prediction and speculation).
>
> Also some branches cannot be eliminated even with parameterization:
>
>     if level == self.mmu_version.dual_pde_level() {
>         // 128-bit dual PDE read
>     } else {
>         // Regular 64-bit PDE read
>     }
>
> This isn't really a version branch -- it's a structural branch that
> distinguishes between 64-bit PDE and 128-bit dual PDE entries. Any MMU
> version with a dual PDE level would need this same distinction.
>
> I also did code-generation size analysis (see diff of code used below):
>
> Code generation analysis:
>
>   Module .ko size:   Before: 511,792 bytes   After: 524,464 bytes  (+2.5%)
>   .text section:     Before: 112,620 bytes   After: 116,628 bytes  (+4,008 bytes)
>
>   The +4K .text growth is the monomorphization cost: every generic function
>   is compiled twice (once for MmuV2, once for MmuV3).
>
>> If you use a trait here, and make `PtWalk` generic against it, you can
>> optimize this away. We had a similar situation when we introduced Turing
>> support and the v2 ucode header, and tried both approaches: the
>> trait-based one was slightly shorter, and arguably more readable.
>
> Actually I was the one who suggested traits for Falcon ucode descriptor if you
> see this thread [1]. So basically you and Eliot are telling me to do what I
> suggested in [1]. :-) However, I disagree that it is the right choice for this code.
>
> [1] https://lore.kernel.org/all/20251117231028.GA1095236@joelbox2/
>
> I think the two cases are quite different in complexity:
>
> The falcon ucode descriptor is essentially a set of flat field accessors
> and a few params (imem_sec_load_params, dmem_load_params).
> The trait has ~10 simple getter methods. There's no multi-level hierarchy,
> no walker, and no generic propagation.
>
> The MMU page table case is structurally different. Making PtWalk generic
> over an Mmu trait would require:
>
>   - PtWalk<M: Mmu> (the walker)
>   - Plus all the associated types: M::Pte, M::Pde, M::DualPde each
>     needing their own trait bounds
>
> And we would also need:
>   - Vmm<M: Mmu> (which creates PtWalk)
>   - BarUser<M: Mmu> (which creates Vmm)
>
> I am also against making Vmm an enum as Eliot suggested:
>        enum Vmm {
>            V2(VmmInner<MmuV2>),
>            V3(VmmInner<MmuV3>),
>        }
>
> That moves the version complexity up to the reader. Code complexity IMO should
> decrease as we go up abstractions, making it easier for users (Vmm/Bar).
>
> If you look at the the changes in vmm.rs to handle version dispatch there [2]:
> Added: +109
> Removed: -28
>
> [2]
> https://github.com/Edgeworth/linux/commit/3627af550b61256184d589e7ec666c1108971f0e
>
> The main benefit of my approach is version-specific dispatch complexity is
> completely isolated inside MmuVersion thus making the code outside of
> pagetable.rs much more readable, without having to parametrize anything, and
> without code size increase. I think that is worth considering.
>
>> But the main argument to use a trait here IMO is that it enables
>> associated types and constants. That's particularly critical since some
>> equivalent fields have different lengths between v2 and v3. An
>> associated `Bounded` type for these would force the caller to validate
>> the length of these fields before calling a non-fallible operation,
>> which is exactly the level of caution that we want when dealing with
>> page tables.
>
> I think Bounded validation is orthogonal to the dispatch model.
> We can add Bounded to the current design without restructuring
> into traits. For example:
>
>     // In ver2::Pte
>     pub fn new_vram(pfn: Bounded<Pfn, 25>, writable: bool) -> Self { ... }
>
>     // In ver3::Pte
>     pub fn new_vram(pfn: Bounded<Pfn, 40>, writable: bool) -> Self { ... }
>
> The unified Pte enum wrapper already dispatches to the correct
> version-specific constructor, which would enforce the correct Bounded
> constraint for that version.
>
>> In order to fully benefit from it, we will need the bitfield macro from
>> the `kernel` crate so the PDE/PTE fields can be `Bounded`, I will try to
>> make it available quickly in a patch that you can depend on.
>
> That would be great, and I'd be happy to integrate Bounded validation once
> the macro is available. I just don't think we need to restructure the
> dispatch model in order to benefit from it.
>
>> But long story short, and although I need to dive deeper into the code,
>> this looks like a good candidate for using a trait and associated types.
>
> The walker code (walk.rs) is already version-agnostic and reads cleanly.
> The version dispatch is encapsulated behind method calls, not exposed as
> inline if/else blocks.
>
> Generic propagation (or version-specific dispatch at higher levels) adds more
> complexity at higher layers.
>
> Enclosed below [3] is the diff I used for my testing with the data, I don't
> really see a net readability win there (IMO, it is a net-loss in readability).
>
> [3]
> https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/commit/?h=trait-pt-dispatch&id=5eb0e98af11ba608ff4d0f7a06065ee863f5066a

IMO this diff is quite has got me quite in favour of trait approach.

I wanted about to purpose something similar (or maybe I had already?) trait
approach some versions ago but didn't due to the eventual need of `match` like
dispatch (like you had with `vmm_dispatch`), but your code made that looks not
as bad as I thought it would be.

Best,
Gary

^ permalink raw reply

* [PATCH] fbdev: tdfxfb: avoid divide-by-zero on FBIOPUT_VSCREENINFO
From: Greg Kroah-Hartman @ 2026-04-09 13:23 UTC (permalink / raw)
  To: linux-fbdev, dri-devel
  Cc: linux-kernel, Greg Kroah-Hartman, Helge Deller, stable

Much like commit 19f953e74356 ("fbdev: fb_pm2fb: Avoid potential divide
by zero error"), we also need to prevent that same crash from happening
in the udlfb driver as it uses pixclock directly when dividing, which
will crash.

Cc: Helge Deller <deller@gmx.de>
Assisted-by: gregkh_clanker_t1000
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/video/fbdev/tdfxfb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index 51ebe78359ec..531fb8478e20 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -496,6 +496,9 @@ static int tdfxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 		}
 	}
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	if (PICOS2KHZ(var->pixclock) > par->max_pixclock) {
 		DPRINTK("pixclock too high (%ldKHz)\n",
 			PICOS2KHZ(var->pixclock));
-- 
2.53.0


^ permalink raw reply related

* [PATCH] fbdev: udlfb: avoid divide-by-zero on FBIOPUT_VSCREENINFO
From: Greg Kroah-Hartman @ 2026-04-09 13:23 UTC (permalink / raw)
  To: linux-fbdev, dri-devel
  Cc: linux-kernel, Greg Kroah-Hartman, Bernie Thompson, Helge Deller

Much like commit 19f953e74356 ("fbdev: fb_pm2fb: Avoid potential divide
by zero error"), we also need to prevent that same crash from happening
in the udlfb driver as it uses pixclock directly when dividing, which
will crash.

Cc: Bernie Thompson <bernie@plugable.com>
Cc: Helge Deller <deller@gmx.de>
Fixes: 59277b679f8b ("Staging: udlfb: add dynamic modeset support")
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/video/fbdev/udlfb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index 3c6a9b5758d9..c341d76bc564 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -1018,6 +1018,9 @@ static int dlfb_ops_check_var(struct fb_var_screeninfo *var,
 	struct fb_videomode mode;
 	struct dlfb_data *dlfb = info->par;
 
+	if (!var->pixclock)
+		return -EINVAL;
+
 	/* set device-specific elements of var unrelated to mode */
 	dlfb_var_color_format(var);
 
-- 
2.53.0


^ permalink raw reply related

* [PATCH 0/2] staging: sm750fb: improve error handling in de_wait path
From: Hungyu Lin @ 2026-04-09 14:41 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin

This series improves error handling in the sm750fb driver.

The de_wait() helpers currently return -1 on timeout and callers
discard error codes by always returning -1. This series replaces
-1 with -ETIMEDOUT and propagates the error code to improve
error reporting.

Patch 1 replaces -1 with -ETIMEDOUT in de_wait() helpers.
Patch 2 propagates error codes in callers instead of returning -1.

Hungyu Lin (2):
  staging: sm750fb: return -ETIMEDOUT on timeout in de_wait functions
  staging: sm750fb: propagate error codes from de_wait()

 drivers/staging/sm750fb/sm750_accel.c | 22 ++++++++++++----------
 drivers/staging/sm750fb/sm750_hw.c    |  4 ++--
 2 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH 1/2] staging: sm750fb: return -ETIMEDOUT on timeout in de_wait functions
From: Hungyu Lin @ 2026-04-09 14:41 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin
In-Reply-To: <20260409144119.21500-1-dennylin0707@gmail.com>

The hw_sm750le_de_wait() and hw_sm750_de_wait() functions return -1
when a timeout occurs. Replace these with -ETIMEDOUT to use a proper
errno value and better describe the error condition.

All callers check the return value as non-zero, so this change does
not alter existing behavior.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/staging/sm750fb/sm750_hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index a2798d428663..3809401baa3a 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -502,7 +502,7 @@ int hw_sm750le_de_wait(void)
 			return 0;
 	}
 	/* timeout error */
-	return -1;
+	return -ETIMEDOUT;
 }
 
 int hw_sm750_de_wait(void)
@@ -520,7 +520,7 @@ int hw_sm750_de_wait(void)
 			return 0;
 	}
 	/* timeout error */
-	return -1;
+	return -ETIMEDOUT;
 }
 
 int hw_sm750_pan_display(struct lynxfb_crtc *crtc,
-- 
2.34.1


^ permalink raw reply related

* [PATCH 2/2] staging: sm750fb: propagate error codes from de_wait()
From: Hungyu Lin @ 2026-04-09 14:41 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin
In-Reply-To: <20260409144119.21500-1-dennylin0707@gmail.com>

The sm750 acceleration functions currently return -1 when
de_wait() fails, discarding the original error code.

Since de_wait() now returns proper errno values, propagate
the error code instead of returning -1.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/staging/sm750fb/sm750_accel.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 0f94d859e91c..891d12a5e2cc 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -90,14 +90,12 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 		      u32 color, u32 rop)
 {
 	u32 de_ctrl;
+	int ret;
 
-	if (accel->de_wait() != 0) {
-		/*
-		 * int time wait and always busy,seems hardware
-		 * got something error
-		 */
+	ret = accel->de_wait();
+	if (ret) {
 		pr_debug("De engine always busy\n");
-		return -1;
+		return ret;
 	}
 
 	write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base); /* dpr40 */
@@ -154,6 +152,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
 		      unsigned int rop2)
 {
 	unsigned int direction, de_ctrl;
+	int ret;
 
 	direction = LEFT_TO_RIGHT;
 	/* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left */
@@ -263,8 +262,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
 		   DE_WINDOW_WIDTH_DST_MASK) |
 		  (source_pitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
 
-	if (accel->de_wait() != 0)
-		return -1;
+	ret = accel->de_wait();
+	if (ret)
+		return ret;
 
 	write_dpr(accel, DE_SOURCE,
 		  ((sx << DE_SOURCE_X_K1_SHIFT) & DE_SOURCE_X_K1_MASK) |
@@ -326,14 +326,16 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *src_buf,
 	unsigned int de_ctrl = 0;
 	unsigned char remain[4];
 	int i, j;
+	int ret;
 
 	start_bit &= 7; /* Just make sure the start bit is within legal range */
 	bytes_per_scan = (width + start_bit + 7) / 8;
 	words_per_scan = bytes_per_scan & ~3;
 	bytes_remain = bytes_per_scan & 3;
 
-	if (accel->de_wait() != 0)
-		return -1;
+	ret = accel->de_wait();
+	if (ret)
+		return ret;
 
 	/*
 	 * 2D Source Base.
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH 2/2] staging: sm750fb: propagate error codes from de_wait()
From: Greg Kroah-Hartman @ 2026-04-09 15:05 UTC (permalink / raw)
  To: Hungyu Lin
  Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
	linux-kernel
In-Reply-To: <20260409144119.21500-3-dennylin0707@gmail.com>

On Thu, Apr 09, 2026 at 02:41:19PM +0000, Hungyu Lin wrote:
> The sm750 acceleration functions currently return -1 when
> de_wait() fails, discarding the original error code.
> 
> Since de_wait() now returns proper errno values, propagate
> the error code instead of returning -1.
> 
> Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> ---
>  drivers/staging/sm750fb/sm750_accel.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
> index 0f94d859e91c..891d12a5e2cc 100644
> --- a/drivers/staging/sm750fb/sm750_accel.c
> +++ b/drivers/staging/sm750fb/sm750_accel.c
> @@ -90,14 +90,12 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
>  		      u32 color, u32 rop)
>  {
>  	u32 de_ctrl;
> +	int ret;
>  
> -	if (accel->de_wait() != 0) {
> -		/*
> -		 * int time wait and always busy,seems hardware
> -		 * got something error
> -		 */

Why did you delete the comment?

And when did de_wait() start returning correct values?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2/2] staging: sm750fb: propagate error codes from de_wait()
From: Denny Lin @ 2026-04-09 15:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
	linux-kernel
In-Reply-To: <2026040931-robust-siamese-4e11@gregkh>

Hi Greg,

> Why did you delete the comment?

The comment was removed as part of simplifying the code while
changing the error handling to propagate the return value from
de_wait(). I can restore or update it if needed.

> And when did de_wait() start returning correct values?

de_wait() starts returning a proper errno value in patch 1 of this
series, where the timeout return value is changed from -1 to
-ETIMEDOUT. This patch then propagates that error value.

Thanks,
Hungyu

On Thu, Apr 9, 2026 at 8:05 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Apr 09, 2026 at 02:41:19PM +0000, Hungyu Lin wrote:
> > The sm750 acceleration functions currently return -1 when
> > de_wait() fails, discarding the original error code.
> >
> > Since de_wait() now returns proper errno values, propagate
> > the error code instead of returning -1.
> >
> > Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
> > ---
> >  drivers/staging/sm750fb/sm750_accel.c | 22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
> > index 0f94d859e91c..891d12a5e2cc 100644
> > --- a/drivers/staging/sm750fb/sm750_accel.c
> > +++ b/drivers/staging/sm750fb/sm750_accel.c
> > @@ -90,14 +90,12 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
> >                     u32 color, u32 rop)
> >  {
> >       u32 de_ctrl;
> > +     int ret;
> >
> > -     if (accel->de_wait() != 0) {
> > -             /*
> > -              * int time wait and always busy,seems hardware
> > -              * got something error
> > -              */
>
> Why did you delete the comment?
>
> And when did de_wait() start returning correct values?
>
> thanks,
>
> greg k-h

^ permalink raw reply

* Re: [PATCH 2/2] staging: sm750fb: propagate error codes from de_wait()
From: Greg Kroah-Hartman @ 2026-04-09 15:22 UTC (permalink / raw)
  To: Denny Lin
  Cc: Sudip Mukherjee, Teddy Wang, linux-fbdev, linux-staging,
	linux-kernel
In-Reply-To: <CAGEkeHd8p48H6U36VBFAAMffz_tW71jcb90POekDgG51JN4YUA@mail.gmail.com>

On Thu, Apr 09, 2026 at 08:15:36AM -0700, Denny Lin wrote:
> Hi Greg,
> 

Odd email style, try replying inline in the real email, not at the top
please :)

> > Why did you delete the comment?
> 
> The comment was removed as part of simplifying the code while
> changing the error handling to propagate the return value from
> de_wait(). I can restore or update it if needed.

Please keep it.

> > And when did de_wait() start returning correct values?
> 
> de_wait() starts returning a proper errno value in patch 1 of this
> series, where the timeout return value is changed from -1 to
> -ETIMEDOUT. This patch then propagates that error value.

Ah, that wasn't obvious, sorry.

thanks,

greg k-h

^ permalink raw reply

* [PATCH v2 0/2] staging: sm750fb: improve error handling in de_wait path
From: Hungyu Lin @ 2026-04-09 15:58 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin

This series improves error handling in the sm750fb driver.

The de_wait() helpers previously returned -1 on timeout, and
callers discarded the error code by always returning -1. This
series replaces -1 with -ETIMEDOUT and propagates the error code
to improve error reporting.

Patch 1 replaces -1 with -ETIMEDOUT in de_wait() helpers.
Patch 2 propagates error codes from de_wait().

v2:
- Restore the removed comment in sm750_accel.c as requested

Hungyu Lin (2):
  staging: sm750fb: return -ETIMEDOUT on timeout in de_wait functions
  staging: sm750fb: propagate error codes from de_wait()

 drivers/staging/sm750fb/sm750_accel.c | 20 +++++++++++++-------
 drivers/staging/sm750fb/sm750_hw.c    |  4 ++--
 2 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v2 1/2] staging: sm750fb: return -ETIMEDOUT on timeout in de_wait functions
From: Hungyu Lin @ 2026-04-09 15:58 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin
In-Reply-To: <20260409155821.23375-1-dennylin0707@gmail.com>

The hw_sm750le_de_wait() and hw_sm750_de_wait() functions return -1
when a timeout occurs. Replace these with -ETIMEDOUT to use a proper
errno value and better describe the error condition.

All callers check the return value as non-zero, so this change does
not alter existing behavior.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/staging/sm750fb/sm750_hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_hw.c b/drivers/staging/sm750fb/sm750_hw.c
index a2798d428663..3809401baa3a 100644
--- a/drivers/staging/sm750fb/sm750_hw.c
+++ b/drivers/staging/sm750fb/sm750_hw.c
@@ -502,7 +502,7 @@ int hw_sm750le_de_wait(void)
 			return 0;
 	}
 	/* timeout error */
-	return -1;
+	return -ETIMEDOUT;
 }
 
 int hw_sm750_de_wait(void)
@@ -520,7 +520,7 @@ int hw_sm750_de_wait(void)
 			return 0;
 	}
 	/* timeout error */
-	return -1;
+	return -ETIMEDOUT;
 }
 
 int hw_sm750_pan_display(struct lynxfb_crtc *crtc,
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 2/2] staging: sm750fb: propagate error codes from de_wait()
From: Hungyu Lin @ 2026-04-09 15:58 UTC (permalink / raw)
  To: Sudip Mukherjee, Teddy Wang, Greg Kroah-Hartman
  Cc: linux-fbdev, linux-staging, linux-kernel, Hungyu Lin
In-Reply-To: <20260409155821.23375-1-dennylin0707@gmail.com>

The sm750 acceleration functions currently return -1 when
de_wait() fails, discarding the original error code.

Since de_wait() now returns proper errno values, propagate
the error code instead of returning -1.

Signed-off-by: Hungyu Lin <dennylin0707@gmail.com>
---
 drivers/staging/sm750fb/sm750_accel.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index 0f94d859e91c..688ec262a8ed 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -90,14 +90,16 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 		      u32 color, u32 rop)
 {
 	u32 de_ctrl;
+	int ret;
 
-	if (accel->de_wait() != 0) {
+	ret = accel->de_wait();
+	if (ret) {
 		/*
-		 * int time wait and always busy,seems hardware
+		 * int time wait and always busy, seems hardware
 		 * got something error
 		 */
 		pr_debug("De engine always busy\n");
-		return -1;
+		return ret;
 	}
 
 	write_dpr(accel, DE_WINDOW_DESTINATION_BASE, base); /* dpr40 */
@@ -154,6 +156,7 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
 		      unsigned int rop2)
 {
 	unsigned int direction, de_ctrl;
+	int ret;
 
 	direction = LEFT_TO_RIGHT;
 	/* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left */
@@ -263,8 +266,9 @@ int sm750_hw_copyarea(struct lynx_accel *accel,
 		   DE_WINDOW_WIDTH_DST_MASK) |
 		  (source_pitch / Bpp & DE_WINDOW_WIDTH_SRC_MASK)); /* dpr3c */
 
-	if (accel->de_wait() != 0)
-		return -1;
+	ret = accel->de_wait();
+	if (ret)
+		return ret;
 
 	write_dpr(accel, DE_SOURCE,
 		  ((sx << DE_SOURCE_X_K1_SHIFT) & DE_SOURCE_X_K1_MASK) |
@@ -326,14 +330,16 @@ int sm750_hw_imageblit(struct lynx_accel *accel, const char *src_buf,
 	unsigned int de_ctrl = 0;
 	unsigned char remain[4];
 	int i, j;
+	int ret;
 
 	start_bit &= 7; /* Just make sure the start bit is within legal range */
 	bytes_per_scan = (width + start_bit + 7) / 8;
 	words_per_scan = bytes_per_scan & ~3;
 	bytes_remain = bytes_per_scan & 3;
 
-	if (accel->de_wait() != 0)
-		return -1;
+	ret = accel->de_wait();
+	if (ret)
+		return ret;
 
 	/*
 	 * 2D Source Base.
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v10 12/21] gpu: nova-core: mm: Add unified page table entry wrapper enums
From: John Hubbard @ 2026-04-09 18:02 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Eliot Courtney, linux-kernel, Miguel Ojeda, Boqun Feng, Gary Guo,
	Bjorn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Dave Airlie, Daniel Almeida,
	Koen Koning, dri-devel, rust-for-linux, Nikola Djukic,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, Jonathan Corbet, Alex Deucher, Christian Koenig,
	Jani Nikula, Joonas Lahtinen, Vivi Rodrigo, Tvrtko Ursulin,
	Rui Huang, Matthew Auld, Matthew Brost, Lucas De Marchi,
	Thomas Hellstrom, Helge Deller, Alex Gaynor, Boqun Feng,
	Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
	Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
	Philipp Stanner, Elle Rhumsaa, Alexey Ivanov, linux-doc, amd-gfx,
	intel-gfx, intel-xe, linux-fbdev
In-Reply-To: <1775730646.3752.4760@nvidia.com>

On 4/9/26 3:33 AM, Joel Fernandes wrote:
...
> Since it is 3 against 1 here, I rest my case :-). I am still in

As Danilo points out, we want to foster healthy debate and land
on whatever comes out of that. Not just by counting noses. :)

> disagreement since I do not see much benefit (that is why I said
> pointless above). Actually it is not even about readability, that is
> subjective (and I haven’t heard most people say parametrizing code for
> the sake of it makes it more readable anyway). It is that the code gen
> is worse, and the complexity is just moved to a higher level in the
> code, not removed. So what are we getting out of this really, other than
> more boiler plate in higher layers of the code that did not exist
> before? Not performance, not better generated code. Really nothing. See
> all the data points in my previous reply.

Alex's latest response[1] does a *much* better job than mine, in
explicitly highlighting what we get out of this. (It arrived a bit after
your response here.) Please take a very close look at that response and
see what you think.

The patterns in these page tables are not a new thing, and Rust has
language features to help express them. 

[1] https://lore.kernel.org/DHOKJ3MJNO5P.SXKOAYKX13JL@nvidia.com

thanks,
-- 
John Hubbard


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox