mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] gpu: nova-core: Hopper sysmem flush fix and FSP comment cleanup
@ 2026-06-03 23:50 John Hubbard
  2026-06-03 23:50 ` [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers John Hubbard
  2026-06-03 23:50 ` [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments John Hubbard
  0 siblings, 2 replies; 7+ messages in thread
From: John Hubbard @ 2026-06-03 23:50 UTC (permalink / raw)
  To: Danilo Krummrich, Alexandre Courbot
  Cc: Timur Tabi, Alistair Popple, Eliot Courtney, Shashank Sharma,
	Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML, John Hubbard

A follow-up to the recently merged Hopper/Blackwell FSP series. The
Blackwell sysmem flush fix deliberately left Hopper for later; patch 1
is that fix, and patch 2 cleans up two stale comments nearby.

Based on drm-rust-next, on top of Gary Guo's "move lifetime to `Bar0`".

John Hubbard (2):
  gpu: nova-core: Hopper: use correct sysmem flush registers
  gpu: nova-core: clean up FSP FRTS comments

 drivers/gpu/nova-core/fb/hal/gb202.rs | 12 +++++------
 drivers/gpu/nova-core/fb/hal/gh100.rs | 31 ++++++++++++++++++++++++---
 drivers/gpu/nova-core/fsp.rs          |  7 +++---
 drivers/gpu/nova-core/regs.rs         | 27 ++++++++++++++++++-----
 4 files changed, 60 insertions(+), 17 deletions(-)


base-commit: 99676aed1fec109d62822e21a06760eb098dc5f4
-- 
2.54.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers
  2026-06-03 23:50 [PATCH 0/2] gpu: nova-core: Hopper sysmem flush fix and FSP comment cleanup John Hubbard
@ 2026-06-03 23:50 ` John Hubbard
  2026-06-09 13:54   ` Alexandre Courbot
  2026-06-03 23:50 ` [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments John Hubbard
  1 sibling, 1 reply; 7+ messages in thread
From: John Hubbard @ 2026-06-03 23:50 UTC (permalink / raw)
  To: Danilo Krummrich, Alexandre Courbot
  Cc: Timur Tabi, Alistair Popple, Eliot Courtney, Shashank Sharma,
	Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML, John Hubbard

Hopper has its own sysmem flush page registers, but the Hopper
framebuffer HAL was still programming the Ampere registers, so the
flush address landed in the wrong location. Program Hopper's own
registers instead.

Open RM and Nouveau disagree on how these registers encode the
address. This commit follows Open RM's approach.

Note that this is still yet to be confirmed (tested) on real Hopper
hardware.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/fb/hal/gb202.rs | 12 +++++------
 drivers/gpu/nova-core/fb/hal/gh100.rs | 31 ++++++++++++++++++++++++---
 drivers/gpu/nova-core/regs.rs         | 27 ++++++++++++++++++-----
 3 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/nova-core/fb/hal/gb202.rs b/drivers/gpu/nova-core/fb/hal/gb202.rs
index 038d1278c634..00554e349bde 100644
--- a/drivers/gpu/nova-core/fb/hal/gb202.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb202.rs
@@ -30,11 +30,11 @@ impl RegisterBase<regs::Fbhub0Base> for Gb202 {
 
 fn read_sysmem_flush_page_gb202(bar: Bar0<'_>) -> u64 {
     let lo = u64::from(
-        bar.read(regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb202>())
+        bar.read(regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb202>())
             .adr(),
     );
     let hi = u64::from(
-        bar.read(regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb202>())
+        bar.read(regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb202>())
             .adr(),
     );
 
@@ -45,14 +45,14 @@ fn read_sysmem_flush_page_gb202(bar: Bar0<'_>) -> u64 {
 fn write_sysmem_flush_page_gb202(bar: Bar0<'_>, addr: Bounded<u64, 52>) {
     // Write HI first. The hardware will trigger the flush on the LO write.
     bar.write(
-        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb202>(),
-        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed()
+        regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI::of::<Gb202>(),
+        regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed()
             .with_adr(addr.shr::<32, 20>().cast::<u32>()),
     );
     bar.write(
-        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb202>(),
+        regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO::of::<Gb202>(),
         // CAST: lower 32 bits. Hardware ignores bits 7:0.
-        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(*addr as u32),
+        regs::NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(*addr as u32),
     );
 }
 
diff --git a/drivers/gpu/nova-core/fb/hal/gh100.rs b/drivers/gpu/nova-core/fb/hal/gh100.rs
index 5450c7254dad..6fd4f967cea5 100644
--- a/drivers/gpu/nova-core/fb/hal/gh100.rs
+++ b/drivers/gpu/nova-core/fb/hal/gh100.rs
@@ -2,24 +2,49 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
 use kernel::{
+    io::Io,
+    num::Bounded,
     prelude::*,
     sizes::SizeConstants, //
 };
 
 use crate::{
     driver::Bar0,
-    fb::hal::FbHal, //
+    fb::hal::FbHal,
+    regs, //
 };
 
 struct Gh100;
 
+fn read_sysmem_flush_page_gh100(bar: Bar0<'_>) -> u64 {
+    let lo = u64::from(bar.read(regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO).adr());
+    let hi = u64::from(bar.read(regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI).adr());
+
+    lo | (hi << 32)
+}
+
+/// Write the sysmem flush page address through the Hopper FBHUB registers.
+fn write_sysmem_flush_page_gh100(bar: Bar0<'_>, addr: Bounded<u64, 52>) {
+    // Write HI first. The hardware will trigger the flush on the LO write.
+    bar.write_reg(
+        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI::zeroed()
+            .with_adr(addr.shr::<32, 20>().cast::<u32>()),
+    );
+    bar.write_reg(
+        // CAST: lower 32 bits. Hardware ignores bits 7:0.
+        regs::NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO::zeroed().with_adr(*addr as u32),
+    );
+}
+
 impl FbHal for Gh100 {
     fn read_sysmem_flush_page(&self, bar: Bar0<'_>) -> u64 {
-        super::ga100::read_sysmem_flush_page_ga100(bar)
+        read_sysmem_flush_page_gh100(bar)
     }
 
     fn write_sysmem_flush_page(&self, bar: Bar0<'_>, addr: u64) -> Result {
-        super::ga100::write_sysmem_flush_page_ga100(bar, addr);
+        let addr = Bounded::<u64, 52>::try_new(addr).ok_or(EINVAL)?;
+
+        write_sysmem_flush_page_gh100(bar, addr);
 
         Ok(())
     }
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 0f49c1ab83ad..a57e95140ec0 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -131,6 +131,22 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
         23:0    adr_63_40;
     }
 
+    /// Low bits of the physical system memory address used by the GPU to perform sysmembar
+    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
+    ///
+    /// Unlike the Ampere `NV_PFB_NISO_FLUSH_SYSMEM_ADDR` registers, which encode the address with
+    /// an 8-bit right-shift, the Hopper FBHUB registers take the raw address split into lower and
+    /// upper halves. Hardware ignores bits 7:0 of the LO register.
+    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ 0x00100a34 {
+        31:0    adr => u32;
+    }
+
+    /// High bits of the physical system memory address used by the GPU to perform sysmembar
+    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
+    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x00100a38 {
+        19:0    adr;
+    }
+
     pub(crate) NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE(u32) @ 0x00100ce0 {
         30:30   ecc_mode_enabled => bool;
         9:4     lower_mag;
@@ -179,15 +195,16 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
         19:0    adr;
     }
 
-    // GB20x sysmem flush registers, relative to the FBHUB0 base. Unlike the older
-    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers which encode the address with an 8-bit
-    // right-shift, these take the raw address split into lower and upper halves. Hardware
+    // GB20x sysmem flush registers, relative to the FBHUB0 base. Like the Hopper
+    // NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR registers, and unlike the older
+    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers (which encode the address with an 8-bit
+    // right-shift), these take the raw address split into lower and upper halves. Hardware
     // ignores bits 7:0 of the LO register.
-    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
+    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
         31:0    adr => u32;
     }
 
-    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {
+    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {
         19:0    adr;
     }
 }
-- 
2.54.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments
  2026-06-03 23:50 [PATCH 0/2] gpu: nova-core: Hopper sysmem flush fix and FSP comment cleanup John Hubbard
  2026-06-03 23:50 ` [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers John Hubbard
@ 2026-06-03 23:50 ` John Hubbard
  2026-06-09 23:04   ` Eliot Courtney
  2026-06-09 23:30   ` Alexandre Courbot
  1 sibling, 2 replies; 7+ messages in thread
From: John Hubbard @ 2026-06-03 23:50 UTC (permalink / raw)
  To: Danilo Krummrich, Alexandre Courbot
  Cc: Timur Tabi, Alistair Popple, Eliot Courtney, Shashank Sharma,
	Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML, John Hubbard

Two comments in the FSP Chain of Trust message setup had drifted from
the code. One referred to a variable name that no longer exists, and
another described the unused sysmem FRTS fields as future work rather
than explaining why they are zero. Update both to describe the code as
it stands.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/fsp.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index 8fc243c66e35..0e2c596f695a 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -110,7 +110,8 @@ fn new<'a>(
         fsp_fw: &'a FspFirmware,
         args: &'a FmcBootArgs,
     ) -> Result<impl Init<Self> + 'a> {
-        // frts_offset is relative to FB end: FRTS_location = FB_END - frts_offset
+        // frts_vidmem_offset is measured from the end of FB, so FRTS sits at
+        // (end of FB) - frts_vidmem_offset.
         let frts_vidmem_offset = if !args.resume {
             let frts_reserved_size = fb_layout.heap.len() + u64::from(fb_layout.pmu_reserved_size);
 
@@ -143,8 +144,8 @@ fn new<'a>(
             msg.cot.gsp_fmc_sysmem_offset = fsp_fw.fmc_image.dma_handle();
             msg.cot.frts_vidmem_offset = frts_vidmem_offset;
             msg.cot.frts_vidmem_size = frts_size;
-            // frts_sysmem_* intentionally left at zero for now, but will be needed for e.g.
-            // systems without VRAM.
+            // frts_sysmem_* are left at zero because this path places FRTS in vidmem. The sysmem
+            // fields point to an FRTS buffer in sysmem instead, for systems without VRAM.
             msg.cot.gsp_boot_args_sysmem_offset = args.fmc_boot_params.dma_handle();
             msg.cot.sigs = *fsp_fw.fmc_sigs;
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers
  2026-06-03 23:50 ` [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers John Hubbard
@ 2026-06-09 13:54   ` Alexandre Courbot
  2026-06-10  2:47     ` John Hubbard
  0 siblings, 1 reply; 7+ messages in thread
From: Alexandre Courbot @ 2026-06-09 13:54 UTC (permalink / raw)
  To: John Hubbard
  Cc: Danilo Krummrich, Timur Tabi, Alistair Popple, Eliot Courtney,
	Shashank Sharma, Zhi Wang, David Airlie, Simona Vetter,
	Bjorn Helgaas, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML

On Thu Jun 4, 2026 at 8:50 AM JST, John Hubbard wrote:
<...>
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 0f49c1ab83ad..a57e95140ec0 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -131,6 +131,22 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>          23:0    adr_63_40;
>      }
>  
> +    /// Low bits of the physical system memory address used by the GPU to perform sysmembar
> +    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
> +    ///
> +    /// Unlike the Ampere `NV_PFB_NISO_FLUSH_SYSMEM_ADDR` registers, which encode the address with
> +    /// an 8-bit right-shift, the Hopper FBHUB registers take the raw address split into lower and
> +    /// upper halves. Hardware ignores bits 7:0 of the LO register.
> +    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ 0x00100a34 {
> +        31:0    adr => u32;
> +    }
> +
> +    /// High bits of the physical system memory address used by the GPU to perform sysmembar
> +    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
> +    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x00100a38 {
> +        19:0    adr;
> +    }
> +
>      pub(crate) NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE(u32) @ 0x00100ce0 {
>          30:30   ecc_mode_enabled => bool;
>          9:4     lower_mag;
> @@ -179,15 +195,16 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>          19:0    adr;
>      }
>  
> -    // GB20x sysmem flush registers, relative to the FBHUB0 base. Unlike the older
> -    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers which encode the address with an 8-bit
> -    // right-shift, these take the raw address split into lower and upper halves. Hardware
> +    // GB20x sysmem flush registers, relative to the FBHUB0 base. Like the Hopper
> +    // NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR registers, and unlike the older
> +    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers (which encode the address with an 8-bit
> +    // right-shift), these take the raw address split into lower and upper halves. Hardware
>      // ignores bits 7:0 of the LO register.
> -    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
> +    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
>          31:0    adr => u32;
>      }
>  
> -    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {
> +    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {

Since we are switching the name to `FBHUB0`, wouldn't it make sense to
make this an absolute register as well? Because now this reads like a
FBHUB0 register using Fbhub0 as its base, which is redundant.

I know I am the one who suggested making it relative, but after looking
at OpenRM again it doesn't really seem to make sense - there is really
only one base here, and the register is defined as an absolute one
anyway. So my bad for suggesting this in the first place.

If we do this, maybe split into two patches:

- Rename `NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI` into
  `NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI` and make it absolute to
  align with OpenRM,
- Introduce Hopper's `NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI` and use it.

This keeps the changes local and simple, as renaming makes things harder
to track.

As a side-note (not for this patch), we may want to move registers move
aggressively into sub-modules. The Hopper register seems to be only used
on Hopper, so it would make sense to me to move it into a `gh100`
submodule when we eventually move these definitions under `fb/regs.rs`
as is the plan.

Or maybe we can have an even more aggressive policy (which iirc has
already been mentioned on another patchset) of defining the registers
directly into the HALs themselves. Subsystems like `fb` only ever access
their registers in the HALs, so for them at least that could make a lot
of sense.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments
  2026-06-03 23:50 ` [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments John Hubbard
@ 2026-06-09 23:04   ` Eliot Courtney
  2026-06-09 23:30   ` Alexandre Courbot
  1 sibling, 0 replies; 7+ messages in thread
From: Eliot Courtney @ 2026-06-09 23:04 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alexandre Courbot
  Cc: Timur Tabi, Alistair Popple, Eliot Courtney, Shashank Sharma,
	Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML

On Thu Jun 4, 2026 at 8:50 AM JST, John Hubbard wrote:
> Two comments in the FSP Chain of Trust message setup had drifted from
> the code. One referred to a variable name that no longer exists, and
> another described the unused sysmem FRTS fields as future work rather
> than explaining why they are zero. Update both to describe the code as
> it stands.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments
  2026-06-03 23:50 ` [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments John Hubbard
  2026-06-09 23:04   ` Eliot Courtney
@ 2026-06-09 23:30   ` Alexandre Courbot
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Courbot @ 2026-06-09 23:30 UTC (permalink / raw)
  To: John Hubbard
  Cc: Danilo Krummrich, Timur Tabi, Alistair Popple, Eliot Courtney,
	Shashank Sharma, Zhi Wang, David Airlie, Simona Vetter,
	Bjorn Helgaas, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML

On Thu Jun 4, 2026 at 8:50 AM JST, John Hubbard wrote:
> Two comments in the FSP Chain of Trust message setup had drifted from
> the code. One referred to a variable name that no longer exists, and
> another described the unused sysmem FRTS fields as future work rather
> than explaining why they are zero. Update both to describe the code as
> it stands.
>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

Pushed this patch to `drm-rust-next`, thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers
  2026-06-09 13:54   ` Alexandre Courbot
@ 2026-06-10  2:47     ` John Hubbard
  0 siblings, 0 replies; 7+ messages in thread
From: John Hubbard @ 2026-06-10  2:47 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Danilo Krummrich, Timur Tabi, Alistair Popple, Eliot Courtney,
	Shashank Sharma, Zhi Wang, David Airlie, Simona Vetter,
	Bjorn Helgaas, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nova-gpu, LKML

On 6/9/26 6:54 AM, Alexandre Courbot wrote:
> On Thu Jun 4, 2026 at 8:50 AM JST, John Hubbard wrote:
> <...>
>> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
>> index 0f49c1ab83ad..a57e95140ec0 100644
>> --- a/drivers/gpu/nova-core/regs.rs
>> +++ b/drivers/gpu/nova-core/regs.rs
>> @@ -131,6 +131,22 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>>          23:0    adr_63_40;
>>      }
>>  
>> +    /// Low bits of the physical system memory address used by the GPU to perform sysmembar
>> +    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
>> +    ///
>> +    /// Unlike the Ampere `NV_PFB_NISO_FLUSH_SYSMEM_ADDR` registers, which encode the address with
>> +    /// an 8-bit right-shift, the Hopper FBHUB registers take the raw address split into lower and
>> +    /// upper halves. Hardware ignores bits 7:0 of the LO register.
>> +    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ 0x00100a34 {
>> +        31:0    adr => u32;
>> +    }
>> +
>> +    /// High bits of the physical system memory address used by the GPU to perform sysmembar
>> +    /// operations on Hopper (see [`crate::fb::SysmemFlush`]).
>> +    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ 0x00100a38 {
>> +        19:0    adr;
>> +    }
>> +
>>      pub(crate) NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE(u32) @ 0x00100ce0 {
>>          30:30   ecc_mode_enabled => bool;
>>          9:4     lower_mag;
>> @@ -179,15 +195,16 @@ fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>>          19:0    adr;
>>      }
>>  
>> -    // GB20x sysmem flush registers, relative to the FBHUB0 base. Unlike the older
>> -    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers which encode the address with an 8-bit
>> -    // right-shift, these take the raw address split into lower and upper halves. Hardware
>> +    // GB20x sysmem flush registers, relative to the FBHUB0 base. Like the Hopper
>> +    // NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR registers, and unlike the older
>> +    // NV_PFB_NISO_FLUSH_SYSMEM_ADDR registers (which encode the address with an 8-bit
>> +    // right-shift), these take the raw address split into lower and upper halves. Hardware
>>      // ignores bits 7:0 of the LO register.
>> -    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
>> +    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_LO(u32) @ Fbhub0Base + 0x00001d58 {
>>          31:0    adr => u32;
>>      }
>>  
>> -    pub(crate) NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {
>> +    pub(crate) NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI(u32) @ Fbhub0Base + 0x00001d5c {
> 
> Since we are switching the name to `FBHUB0`, wouldn't it make sense to
> make this an absolute register as well? Because now this reads like a
> FBHUB0 register using Fbhub0 as its base, which is redundant.
> 
> I know I am the one who suggested making it relative, but after looking
> at OpenRM again it doesn't really seem to make sense - there is really
> only one base here, and the register is defined as an absolute one
> anyway. So my bad for suggesting this in the first place.
> 
> If we do this, maybe split into two patches:
> 
> - Rename `NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI` into
>   `NV_PFB_FBHUB0_PCIE_FLUSH_SYSMEM_ADDR_HI` and make it absolute to
>   align with OpenRM,
> - Introduce Hopper's `NV_PFB_FBHUB_PCIE_FLUSH_SYSMEM_ADDR_HI` and use it.
> 
> This keeps the changes local and simple, as renaming makes things harder
> to track.

Ok yes, I'll post a v2 with the above approach.

> 
> As a side-note (not for this patch), we may want to move registers move
> aggressively into sub-modules. The Hopper register seems to be only used
> on Hopper, so it would make sense to me to move it into a `gh100`
> submodule when we eventually move these definitions under `fb/regs.rs`
> as is the plan.

Ack, some register organization is definitely called for, and that's
a solid first step.

> 
> Or maybe we can have an even more aggressive policy (which iirc has
> already been mentioned on another patchset) of defining the registers
> directly into the HALs themselves. Subsystems like `fb` only ever access
> their registers in the HALs, so for them at least that could make a lot
> of sense.

Perhaps, let's see.

thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-10  2:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-03 23:50 [PATCH 0/2] gpu: nova-core: Hopper sysmem flush fix and FSP comment cleanup John Hubbard
2026-06-03 23:50 ` [PATCH 1/2] gpu: nova-core: Hopper: use correct sysmem flush registers John Hubbard
2026-06-09 13:54   ` Alexandre Courbot
2026-06-10  2:47     ` John Hubbard
2026-06-03 23:50 ` [PATCH 2/2] gpu: nova-core: clean up FSP FRTS comments John Hubbard
2026-06-09 23:04   ` Eliot Courtney
2026-06-09 23:30   ` Alexandre Courbot

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

Powered by JetHome