On 12/17/25 7:29 PM, Timur Tabi wrote: > The with_falcon_mem() method initializes the 'imem' and 'sec' fields of > the NV_PFALCON_FALCON_DMATRFCMD register based on the value of > the FalconMem type. > > Signed-off-by: Timur Tabi <[email protected]> > --- > drivers/gpu/nova-core/falcon.rs | 16 ++++------------ > drivers/gpu/nova-core/regs.rs | 9 +++++++++ > 2 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs > index b92152277d00..44a1a531a361 100644 > --- a/drivers/gpu/nova-core/falcon.rs > +++ b/drivers/gpu/nova-core/falcon.rs > @@ -454,7 +454,6 @@ fn dma_wr<F: FalconFirmware<Target = E>>( > fw: &F, > target_mem: FalconMem, > load_offsets: FalconLoadTarget, > - sec: bool, > ) -> Result { > const DMA_LEN: u32 = 256; > > @@ -516,8 +515,7 @@ fn dma_wr<F: FalconFirmware<Target = E>>( > > let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::default() > .set_size(DmaTrfCmdSize::Size256B) > - .set_imem(target_mem != FalconMem::Dmem) > - .set_sec(if sec { 1 } else { 0 }); > + .with_falcon_mem(target_mem); > > for pos in (0..num_transfers).map(|i| i * DMA_LEN) { > // Perform a transfer of size `DMA_LEN`. > @@ -551,21 +549,15 @@ pub(crate) fn dma_load<F: FalconFirmware<Target = > E>>(&self, bar: &Bar0, fw: &F) > .set_mem_type(FalconFbifMemType::Physical) > }); > > - self.dma_wr( > - bar, > - fw, > - FalconMem::ImemSecure, > - fw.imem_sec_load_params(), > - true, > - )?; > - self.dma_wr(bar, fw, FalconMem::Dmem, fw.dmem_load_params(), true)?; > + self.dma_wr(bar, fw, FalconMem::ImemSecure, > fw.imem_sec_load_params())?; > + self.dma_wr(bar, fw, FalconMem::Dmem, fw.dmem_load_params())?; > > if let Some(nmem) = fw.imem_ns_load_params() { > // This code should never actual get executed, because the > Non-Secure > // section only exists on firmware used by Turing and GA100, and > // those platforms do not use DMA. But we include this code for > // consistency. > - self.dma_wr(bar, fw, FalconMem::ImemNonSecure, nmem, false)?; > + self.dma_wr(bar, fw, FalconMem::ImemNonSecure, nmem)?; > } > > self.hal.program_brom(self, bar, &fw.brom_params())?; > diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs > index 82cc6c0790e5..c049f5aaf2f2 100644 > --- a/drivers/gpu/nova-core/regs.rs > +++ b/drivers/gpu/nova-core/regs.rs > @@ -16,6 +16,7 @@ > FalconCoreRevSubversion, > FalconFbifMemType, > FalconFbifTarget, > + FalconMem, > FalconModSelAlgo, > FalconSecurityModel, > PFalcon2Base, > @@ -325,6 +326,14 @@ pub(crate) fn mem_scrubbing_done(self) -> bool { > 16:16 set_dmtag as u8; > }); > > +impl NV_PFALCON_FALCON_DMATRFCMD { > + /// Programs the 'imem' and 'sec' fields for the given FalconMem > + pub(crate) fn with_falcon_mem(self, mem: FalconMem) -> Self { > + self.set_imem(mem != FalconMem::Dmem) > + .set_sec(if mem == FalconMem::ImemSecure { 1 } else { 0 })
I went down a deep rabbit hole trying to understand why these methods are chained, even though setting is infallible. And I found a major API design choice that I passionately disagree with, in bitfield!(), so I'm posting a separate patch to "fix" it. ha. :) Anyway, this is all fine here, so: Reviewed-by: John Hubbard <[email protected]> thanks, -- John Hubbard > + } > +} > + > register!(NV_PFALCON_FALCON_DMATRFFBOFFS @ PFalconBase[0x0000011c] { > 31:0 offs as u32; > });
