Re: [PATCH 5/7] gpu: nova-core: falcon: allow DMEM DMA transfers to request tagging

[Alexandre Courbot]

On Thu Nov 5, 2026 at 10:50 PM JST, Eliot Courtney wrote:
> On Tue Nov 3, 2026 at 7:22 PM JST, Alexandre Courbot wrote:
>> Add a tag flag to FalconMem::Dmem and set DMATRFCMD's DMTAG bit from it.
>>
>> This is needed for the r000 load-and-execute event which specifies
>> whether DMEM should be tagged or not.
>>
>> No functional change intended.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>
> It would be nice if this explained what the tagging was.
>
>> ---
>>  drivers/gpu/nova-core/falcon.rs | 20 +++++++++++++-------
>>  drivers/gpu/nova-core/regs.rs   |  2 +-
>>  2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 1bceb38c33c6..f48f2eabab0f 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -131,8 +131,9 @@ pub(crate) enum FalconMem {
>>      /// Non-Secure Instruction Memory.
>>      #[expect(unused)]
>>      ImemNonSecure,
>> -    /// Data Memory.
>> -    Dmem,
>> +    /// Data Memory. If `tag` is set, a transfer into DMEM also tags each block with its source
>> +    /// offset, which is then the DMEM virtual address of the image.
>> +    Dmem { tag: bool },
>>  }
>>  
>>  bounded_enum! {
>> @@ -529,9 +530,9 @@ fn dma_wr(
>>          // code addresses in the firmware (and the boot vector) are virtual.
>>          //
>>          // For DMEM, the start offset is folded into the DMA address.
>> -        let (src_start, dma_start) = match target_mem {
>> -            FalconMem::ImemSecure | FalconMem::ImemNonSecure => (load_offsets.src_start, 0),
>> -            FalconMem::Dmem => (0, usize::from_safe_cast(load_offsets.src_start)),
>> +        let (src_start, dma_start, tag_dmem) = match target_mem {
>> +            FalconMem::ImemSecure | FalconMem::ImemNonSecure => (load_offsets.src_start, 0, false),
>> +            FalconMem::Dmem { tag } => (0, usize::from_safe_cast(load_offsets.src_start), tag),
>
> I am not sure, but is it correct to have `src_start` as zero for
> tag==true here? Surely something should change based on the value of tag
> w.r.t. the addressing.

Yes, there is indeed a problem, tagged DMEM should behave like IMEM
(which is always tagged) here. I'll fix it.

The poor variable names are in great part responsible for the confusion,
so I'll also introduce a patch renaming them.

>
>>          };
>>  
>>          let dma_address = {
>> @@ -568,7 +569,8 @@ fn dma_wr(
>>  
>>          let cmd = regs::NV_PFALCON_FALCON_DMATRFCMD::zeroed()
>>              .with_size(DmaTrfCmdSize::Size256B)
>> -            .with_falcon_mem(target_mem);
>> +            .with_falcon_mem(target_mem)
>> +            .with_set_dmtag(tag_dmem);
>
> Instead of taking tag_dmem out of FalconMem and using
> .with_set_dmtag(tag_dmem), let `with_falcon_mem` set it? It looks like
> it's always just passed through.

Good idea!