Re: [PATCH 2/7] gpu: nova-core: firmware: riscv: use dma::Coherent

[Alexandre Courbot]

On Sat Mar 21, 2026 at 11:58 PM JST, Gary Guo wrote:
> On Sat Mar 21, 2026 at 1:36 PM GMT, Alexandre Courbot wrote:
>> Replace the nova-core local `DmaObject` with a `Coherent` that can
>> fulfill the same role.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> ---
>>  drivers/gpu/nova-core/firmware/riscv.rs | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
>> index 14aad2f0ee8a..2afa7f36404e 100644
>> --- a/drivers/gpu/nova-core/firmware/riscv.rs
>> +++ b/drivers/gpu/nova-core/firmware/riscv.rs
>> @@ -5,13 +5,13 @@
>>  
>>  use kernel::{
>>      device,
>> +    dma::Coherent,
>>      firmware::Firmware,
>>      prelude::*,
>>      transmute::FromBytes, //
>>  };
>>  
>>  use crate::{
>> -    dma::DmaObject,
>>      firmware::BinFirmware,
>>      num::FromSafeCast, //
>>  };
>> @@ -66,7 +66,7 @@ pub(crate) struct RiscvFirmware {
>>      /// Application version.
>>      pub(crate) app_version: u32,
>>      /// Device-mapped firmware image.
>> -    pub(crate) ucode: DmaObject,
>> +    pub(crate) ucode: Coherent<[u8]>,
>>  }
>>  
>>  impl RiscvFirmware {
>> @@ -81,7 +81,7 @@ pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<
>>              let len = usize::from_safe_cast(bin_fw.hdr.data_size);
>>              let end = start.checked_add(len).ok_or(EINVAL)?;
>>  
>> -            DmaObject::from_data(dev, fw.data().get(start..end).ok_or(EINVAL)?)?
>> +            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
>
> `DmaObject` rounds the data up to be page-sized, while this new API doesn't.
>
> It has impact on alignment, as the allocator aligns things to the largest
> power-of-two exponent of the allocated size.

Doesn't `dma_alloc_coherent` always allocate from the page pool and thus
returns page-aligned memory though? I'm not sure why `DmaObject` was
doing that but it was redundant I think.