[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DDJJ5D7K8MC4.301XTWWSLD7YT@nvidia.com>
Date: Thu, 16 Oct 2025 15:24:30 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Alistair Popple" <apopple@...dia.com>,
<rust-for-linux@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<dakr@...nel.org>, <acourbot@...dia.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
<alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "David Airlie" <airlied@...il.com>,
"Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
<maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
"Thomas Zimmermann" <tzimmermann@...e.de>, "John Hubbard"
<jhubbard@...dia.com>, "Joel Fernandes" <joelagnelf@...dia.com>, "Timur
Tabi" <ttabi@...dia.com>, <linux-kernel@...r.kernel.org>,
<nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH v5 08/14] gpu: nova-core: gsp: Create rmargs
On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> Initialise the GSP resource manager arguments (rmargs) which provide
> initialisation parameters to the GSP firmware during boot. The rmargs
> structure contains arguments to configure the GSP message/command queue
> location.
>
> These are mapped for coherent DMA and added to the libos data structure
> for access when booting GSP.
>
> Signed-off-by: Alistair Popple <apopple@...dia.com>
>
> ---
>
> Changes for v5:
> - Derive Zeroable trait
>
> Changes for v2:
> - Rebased on Alex's latest series
> ---
> drivers/gpu/nova-core/gsp.rs | 16 +++
> drivers/gpu/nova-core/gsp/cmdq.rs | 24 +++-
> drivers/gpu/nova-core/gsp/fw.rs | 60 ++++++++
> .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 132 ------------------
Mmm, looks like we are removing bindings. Can we not add them in the
first place? :)
> 4 files changed, 97 insertions(+), 135 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 1d472c5fad7a..58b595b8badd 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -19,6 +19,7 @@
> mod fw;
>
> use fw::LibosMemoryRegionInitArgument;
> +use fw::GspArgumentsCached;
>
> pub(crate) mod cmdq;
>
> @@ -36,6 +37,7 @@ pub(crate) struct Gsp {
> logintr: LogBuffer,
> logrm: LogBuffer,
> pub(crate) cmdq: Cmdq,
> + rmargs: CoherentAllocation<GspArgumentsCached>,
> }
>
> #[repr(C)]
> @@ -117,12 +119,26 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
>
> // Creates its own PTE array.
> let cmdq = Cmdq::new(dev)?;
> + let rmargs = CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
Let's add a space between the declaration of `cmdq` and `rmargs`.
> + dev,
> + 1,
> + GFP_KERNEL | __GFP_ZERO,
> + )?;
> + dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs)?)?;
> +
> + dma_write!(
> + rmargs[0] = fw::GspArgumentsCached::new(
> + fw::MessageQueueInitArguments::new(&cmdq),
> + fw::GspSrInitArguments::new()
> + )
> + )?;
>
> Ok(try_pin_init!(Self {
> libos,
> loginit,
> logintr,
> logrm,
> + rmargs,
> cmdq,
> }))
> }
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 3f8cb7a35922..da074a2ed0d9 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -6,7 +6,7 @@
>
> use kernel::alloc::flags::GFP_KERNEL;
> use kernel::device;
> -use kernel::dma::CoherentAllocation;
> +use kernel::dma::{CoherentAllocation, DmaAddress};
> use kernel::dma_write;
> use kernel::io::poll::read_poll_timeout;
> use kernel::prelude::*;
> @@ -247,10 +247,25 @@ pub(crate) struct Cmdq {
> dev: ARef<device::Device>,
> seq: u32,
> gsp_mem: DmaGspMem,
> - pub _nr_ptes: u32,
We probably shouldn't have introduced this unused member in the first place.
> }
>
> impl Cmdq {
> + /// Offset of the data after the PTEs.
> + const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq);
> +
> + /// Offset of command queue ring buffer.
> + pub(crate) const CMDQ_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq)
> + + core::mem::offset_of!(Msgq, msgq)
> + - Self::POST_PTE_OFFSET;
> +
> + /// Offset of message queue ring buffer.
> + pub(crate) const STATQ_OFFSET: usize = core::mem::offset_of!(GspMem, gspq)
> + + core::mem::offset_of!(Msgq, msgq)
> + - Self::POST_PTE_OFFSET;
> +
> + /// Number of page table entries for the GSP shared region.
> + pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
> +
> pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
> let gsp_mem = DmaGspMem::new(dev)?;
> let nr_ptes = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
> @@ -260,7 +275,6 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
> dev: dev.into(),
> seq: 0,
> gsp_mem,
> - _nr_ptes: nr_ptes as u32,
> })
> }
>
> @@ -490,4 +504,8 @@ pub(crate) fn receive_msg_from_gsp<M: MessageFromGsp, R>(
> .advance_cpu_read_ptr(msg_header.length().div_ceil(GSP_PAGE_SIZE as u32));
> result
> }
> +
> + pub(crate) fn dma_handle(&self) -> DmaAddress {
> + self.gsp_mem.0.dma_handle()
> + }
> }
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index a2ce570ddfaf..70abda1c2af8 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -16,6 +16,7 @@
>
> use crate::firmware::gsp::GspFirmware;
> use crate::gpu::Chipset;
> +use crate::gsp::cmdq::Cmdq;
> use crate::gsp::FbLayout;
> use crate::gsp::GSP_PAGE_SIZE;
>
> @@ -483,3 +484,62 @@ unsafe impl AsBytes for GspMsgElement {}
> // SAFETY: This struct only contains integer types for which all bit patterns
> // are valid.
> unsafe impl FromBytes for GspMsgElement {}
> +
> +#[repr(transparent)]
> +pub(crate) struct GspArgumentsCached(bindings::GSP_ARGUMENTS_CACHED);
> +
> +impl GspArgumentsCached {
> + pub(crate) fn new(
> + queue_arguments: MessageQueueInitArguments,
> + sr_arguments: GspSrInitArguments,
> + ) -> Self {
> + Self(bindings::GSP_ARGUMENTS_CACHED {
> + messageQueueInitArguments: queue_arguments.0,
> + srInitArguments: sr_arguments.0,
> + bDmemStack: 1,
> + ..Default::default()
> + })
> + }
> +}
> +
> +impl From<GspArgumentsCached> for bindings::GSP_ARGUMENTS_CACHED {
> + fn from(value: GspArgumentsCached) -> Self {
> + value.0
> + }
> +}
This `From` impl seems unneeded?
Powered by blists - more mailing lists