Re: [PATCH v4 08/11] gpu: nova-core: add Falcon HAL method supports_dma()

["Alexandre Courbot" <acourbot@nvidia.com>]

On Thu Dec 18, 2025 at 4:48 PM JST, Alexandre Courbot wrote:
> On Thu Dec 18, 2025 at 12:29 PM JST, Timur Tabi wrote:
>> Some GPUs do not support using DMA to transfer code/data from system
>> memory to Falcon memory, and instead must use programmed I/O (PIO).
>> Add a function to the Falcon HAL to indicate whether a given GPU's
>> Falcons support DMA for this purpose.
>>
>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/falcon.rs           | 7 +++++++
>>  drivers/gpu/nova-core/falcon/hal.rs       | 3 +++
>>  drivers/gpu/nova-core/falcon/hal/ga102.rs | 4 ++++
>>  drivers/gpu/nova-core/falcon/hal/tu102.rs | 4 ++++
>>  4 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
>> index 6b54c0ca458a..50c87dadf2ea 100644
>> --- a/drivers/gpu/nova-core/falcon.rs
>> +++ b/drivers/gpu/nova-core/falcon.rs
>> @@ -630,6 +630,13 @@ pub(crate) fn is_riscv_active(&self, bar: &Bar0) -> bool {
>>          self.hal.is_riscv_active(bar)
>>      }
>>  
>> +    /// Check if this Falcon supports DMA for loading firmware.
>> +    ///
>> +    /// Returns `true` if DMA is supported, `false` if PIO must be used instead.
>> +    pub(crate) fn supports_dma(&self) -> bool {
>> +        self.hal.supports_dma()
>> +    }
>
> Rather than this, we should make the selection of the loading method
> transparent to the caller. Right now `boot.rs` needs to calls
> `supports_dma` itself and choose the right method, which leaves room for
> error.
>
> I'd suggest the following:
>
> - A `fn load(bar, fw) -> Result` method in the Falcon HAL that redirects
>   to either `load_dma` or `load_pio`.

My suggestion won't work because `load` takes a generic parameter, and a
virtual function cannot be generic.

I see two possible workarounds:

1. The HAL `load` method takes a `&dyn FalconFirmware`. I think
   `FalconFirmware` can be turned into a trait object, so that should
   work.

2. We do something that is a mix of your initial proposal and mine: The
   new `Falcon::load` method calls a HAL method that returns the kind of
   load operation supported (please use an enum instead of a boolean for
   clarity and future-proofing), and dispatches accordingly to either
   `pio_load` or `dma_load`.

In both cases, `dma_load` and `pio_load` still become private methods of
`Falcon`.

I would tend to favor 2. because the trait object looks a bit laborious,
and it will call back into `dma_load` or `pio_load` which are still
generic (this will work, but defeats the purpose of having them generic
in the first place). Also it is probably simpler to implement as it is
closer to your original idea.