[PATCH 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta

Danilo Krummrich posted 8 patches 1 month, 1 week ago
[PATCH 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta
Posted by Danilo Krummrich 1 month, 1 week ago
Convert wpr_meta to use Coherent::init() and simplify the
initialization.  It also avoids a separate initialization of
GspFwWprMeta on the stack.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/gsp/boot.rs |  7 ++-----
 drivers/gpu/nova-core/gsp/fw.rs   | 20 +++++++++++++++-----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 7f46fa5e9b50..1a4d9ee4f256 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -2,8 +2,7 @@
 
 use kernel::{
     device,
-    dma::CoherentAllocation,
-    dma_write,
+    dma::Coherent,
     io::poll::read_poll_timeout,
     pci,
     prelude::*,
@@ -155,9 +154,7 @@ pub(crate) fn boot(
             bar,
         )?;
 
-        let wpr_meta =
-            CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
-        dma_write!(wpr_meta, [0]?, GspFwWprMeta::new(&gsp_fw, &fb_layout));
+        let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
 
         self.cmdq
             .send_command(bar, commands::SetSystemInfo::new(pdev))?;
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index f1797e1f0d9d..751d5447214d 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -131,7 +131,9 @@ pub(crate) fn wpr_heap_size(&self, chipset: Chipset, fb_size: u64) -> u64 {
 /// Structure passed to the GSP bootloader, containing the framebuffer layout as well as the DMA
 /// addresses of the GSP bootloader and firmware.
 #[repr(transparent)]
-pub(crate) struct GspFwWprMeta(bindings::GspFwWprMeta);
+pub(crate) struct GspFwWprMeta {
+    inner: bindings::GspFwWprMeta,
+}
 
 // SAFETY: Padding is explicit and does not contain uninitialized data.
 unsafe impl AsBytes for GspFwWprMeta {}
@@ -144,10 +146,14 @@ unsafe impl FromBytes for GspFwWprMeta {}
 type GspFwWprMetaBootInfo = bindings::GspFwWprMeta__bindgen_ty_1__bindgen_ty_1;
 
 impl GspFwWprMeta {
-    /// Fill in and return a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
+    /// Returns an initializer for a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
     /// `fb_layout` layout.
-    pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
-        Self(bindings::GspFwWprMeta {
+    pub(crate) fn new<'a>(
+        gsp_firmware: &'a GspFirmware,
+        fb_layout: &'a FbLayout,
+    ) -> impl Init<Self> + 'a {
+        #[allow(non_snake_case)]
+        let init_inner = init!(bindings::GspFwWprMeta {
             // CAST: we want to store the bits of `GSP_FW_WPR_META_MAGIC` unmodified.
             magic: bindings::GSP_FW_WPR_META_MAGIC as u64,
             revision: u64::from(bindings::GSP_FW_WPR_META_REVISION),
@@ -182,7 +188,11 @@ pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
             fbSize: fb_layout.fb.end - fb_layout.fb.start,
             vgaWorkspaceOffset: fb_layout.vga_workspace.start,
             vgaWorkspaceSize: fb_layout.vga_workspace.end - fb_layout.vga_workspace.start,
-            ..Default::default()
+            ..Zeroable::init_zeroed()
+        });
+
+        init!(GspFwWprMeta {
+            inner <- init_inner,
         })
     }
 }
-- 
2.53.0
Re: [PATCH 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta
Posted by Alexandre Courbot 2 weeks, 6 days ago
On Wed Mar 4, 2026 at 1:22 AM JST, Danilo Krummrich wrote:
> Convert wpr_meta to use Coherent::init() and simplify the
> initialization.  It also avoids a separate initialization of
> GspFwWprMeta on the stack.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Re: [PATCH 6/8] gpu: nova-core: use Coherent::init to initialize GspFwWprMeta
Posted by Gary Guo 1 month, 1 week ago
On Tue Mar 3, 2026 at 4:22 PM GMT, Danilo Krummrich wrote:
> Convert wpr_meta to use Coherent::init() and simplify the
> initialization.  It also avoids a separate initialization of
> GspFwWprMeta on the stack.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/gpu/nova-core/gsp/boot.rs |  7 ++-----
>  drivers/gpu/nova-core/gsp/fw.rs   | 20 +++++++++++++++-----
>  2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index 7f46fa5e9b50..1a4d9ee4f256 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -2,8 +2,7 @@
>  
>  use kernel::{
>      device,
> -    dma::CoherentAllocation,
> -    dma_write,
> +    dma::Coherent,
>      io::poll::read_poll_timeout,
>      pci,
>      prelude::*,
> @@ -155,9 +154,7 @@ pub(crate) fn boot(
>              bar,
>          )?;
>  
> -        let wpr_meta =
> -            CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
> -        dma_write!(wpr_meta, [0]?, GspFwWprMeta::new(&gsp_fw, &fb_layout));
> +        let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>  
>          self.cmdq
>              .send_command(bar, commands::SetSystemInfo::new(pdev))?;
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index f1797e1f0d9d..751d5447214d 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -131,7 +131,9 @@ pub(crate) fn wpr_heap_size(&self, chipset: Chipset, fb_size: u64) -> u64 {
>  /// Structure passed to the GSP bootloader, containing the framebuffer layout as well as the DMA
>  /// addresses of the GSP bootloader and firmware.
>  #[repr(transparent)]
> -pub(crate) struct GspFwWprMeta(bindings::GspFwWprMeta);
> +pub(crate) struct GspFwWprMeta {
> +    inner: bindings::GspFwWprMeta,
> +}
>  
>  // SAFETY: Padding is explicit and does not contain uninitialized data.
>  unsafe impl AsBytes for GspFwWprMeta {}
> @@ -144,10 +146,14 @@ unsafe impl FromBytes for GspFwWprMeta {}
>  type GspFwWprMetaBootInfo = bindings::GspFwWprMeta__bindgen_ty_1__bindgen_ty_1;
>  
>  impl GspFwWprMeta {
> -    /// Fill in and return a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
> +    /// Returns an initializer for a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
>      /// `fb_layout` layout.
> -    pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
> -        Self(bindings::GspFwWprMeta {
> +    pub(crate) fn new<'a>(
> +        gsp_firmware: &'a GspFirmware,
> +        fb_layout: &'a FbLayout,
> +    ) -> impl Init<Self> + 'a {
> +        #[allow(non_snake_case)]

I suppose this is from the field accessor generated?

@Benno we probably should add `#[allow(nonstandard_style)]` on the accessor
generated.

Best,
Gary

> +        let init_inner = init!(bindings::GspFwWprMeta {
>              // CAST: we want to store the bits of `GSP_FW_WPR_META_MAGIC` unmodified.
>              magic: bindings::GSP_FW_WPR_META_MAGIC as u64,
>              revision: u64::from(bindings::GSP_FW_WPR_META_REVISION),
> @@ -182,7 +188,11 @@ pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>              fbSize: fb_layout.fb.end - fb_layout.fb.start,
>              vgaWorkspaceOffset: fb_layout.vga_workspace.start,
>              vgaWorkspaceSize: fb_layout.vga_workspace.end - fb_layout.vga_workspace.start,
> -            ..Default::default()
> +            ..Zeroable::init_zeroed()
> +        });
> +
> +        init!(GspFwWprMeta {
> +            inner <- init_inner,
>          })
>      }
>  }