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

["Gary Guo" <gary@garyguo.net>]

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,
>          })
>      }
>  }