From: Sami Tolvanen <[email protected]> Tyr maps shmem-backed buffers write-combine, but the shmem layer zero-inits new pages through a cached mapping. On a non-coherent device those stale cache lines can land over later uncached writes, so the GPU or userspace reads a freshly written value back as zero.
Read the device's coherence once at probe and map buffers write-back when it's coherent, write-combine otherwise. For write-combine buffers, fetch the sgtable at create time so dma_map_sgtable() flushes the zeroed pages before anyone accesses them uncached. Signed-off-by: Sami Tolvanen <[email protected]> Co-developed-by: Alvin Sun <[email protected]> Signed-off-by: Alvin Sun <[email protected]> --- drivers/gpu/drm/tyr/driver.rs | 10 ++++++++++ drivers/gpu/drm/tyr/file.rs | 5 +++-- drivers/gpu/drm/tyr/fw.rs | 4 +++- drivers/gpu/drm/tyr/gem.rs | 46 ++++++++++++++++++++++++++++++++----------- drivers/gpu/drm/tyr/vm.rs | 9 ++++++--- 5 files changed, 57 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index 8baf88aee96ad..59a2a415a59c7 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -87,6 +87,12 @@ pub(crate) struct TyrDrmRegistrationData<'drm> { /// GPU information read from hardware during probe. pub(crate) gpu_info: GpuInfo, + + /// Whether the device is reported as DMA-coherent by firmware. + /// + /// Cached at probe via `device_get_dma_attr()`. Drives the BO + /// cacheability policy in `crate::gem::should_map_wc`. + pub(crate) coherent: bool, } fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result { @@ -150,6 +156,8 @@ fn probe<'bound>( // other threads of execution. unsafe { pdev.dma_set_mask_and_coherent(DmaMask::try_new(pa_bits)?)? }; + let coherent = pdev.as_ref().dma_coherent(); + let unreg_dev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, Ok(()))?; let mmu = Mmu::new(pdev.as_ref(), iomem.as_arc_borrow(), &gpu_info)?; @@ -160,6 +168,7 @@ fn probe<'bound>( &unreg_dev, mmu.as_arc_borrow(), &gpu_info, + coherent, )?; firmware.boot()?; @@ -179,6 +188,7 @@ fn probe<'bound>( }), iomem, gpu_info, + coherent, }); // SAFETY: `reg` is stored in `TyrPlatformDriverData` and dropped when the driver is diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs index e98eb0a46877f..526eb1d53de42 100644 --- a/drivers/gpu/drm/tyr/file.rs +++ b/drivers/gpu/drm/tyr/file.rs @@ -113,6 +113,7 @@ pub(crate) fn vm_create( pfile.reg.mmu.as_arc_borrow(), &pfile.reg.gpu_info, UserVaRequest::from_uapi(vmcreate.user_va_range), + pfile.reg.coherent, )?; let user_va_range = vm.layout.user.end; let id = pfile.vm_pool.add(vm)?; @@ -251,7 +252,7 @@ pub(crate) fn vm_get_state( pub(crate) fn bo_create( ddev: &TyrDrmDevice<Registered>, - _reg_data: &TyrDrmRegistrationData<'_>, + reg_data: &TyrDrmRegistrationData<'_>, bocreate: &mut uapi::drm_panthor_bo_create, file: &TyrDrmFile, ) -> Result<u32> { @@ -282,7 +283,7 @@ pub(crate) fn bo_create( ); EINVAL })?; - let bo = crate::gem::new_object(ddev, size, bocreate.flags)?; + let bo = crate::gem::new_object(ddev, size, bocreate.flags, reg_data.coherent)?; bocreate.handle = bo.create_handle(file)?; bocreate.size = bo.size() as u64; diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs index d790b54e373e6..fc9e9d10ea921 100644 --- a/drivers/gpu/drm/tyr/fw.rs +++ b/drivers/gpu/drm/tyr/fw.rs @@ -221,8 +221,9 @@ pub(crate) fn new( ddev: &TyrDrmDevice, mmu: ArcBorrow<'_, Mmu<'drm>>, gpu_info: &GpuInfo, + coherent: bool, ) -> Result<Firmware<'drm>> { - let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info)?; + let vm = Vm::new_for_fw(dev, ddev, mmu, gpu_info, coherent)?; vm.activate()?; let sections = (|| -> Result<KVec<Section<'drm>>> { @@ -239,6 +240,7 @@ pub(crate) fn new( size, KernelBoVaAlloc::Explicit(va), parsed.vm_map_flags, + coherent, )?; let section_start = parsed.data_range.start as usize; diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index 22a04c0b8a812..95e5537e7174b 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -63,23 +63,47 @@ fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Se /// Type alias for Tyr GEM buffer objects. pub(crate) type Bo = gem::shmem::Object<BoData>; +/// Returns whether a BO should be mapped write-combine given the device's +/// DMA coherence. +pub(crate) fn should_map_wc(coherent: bool) -> bool { + if coherent { + return false; + } + + true +} + /// Create a new GEM buffer object. -pub(crate) fn new_object(ddev: &TyrDrmDevice, size: usize, flags: u32) -> Result<ARef<Bo>> { +pub(crate) fn new_object( + ddev: &TyrDrmDevice, + size: usize, + flags: u32, + coherent: bool, +) -> Result<ARef<Bo>> { if size == 0 { return Err(EINVAL); } let aligned_size = size.checked_next_multiple_of(PAGE_SIZE).ok_or(EINVAL)?; - Bo::new( + let map_wc = should_map_wc(coherent); + let bo = Bo::new( ddev, aligned_size, shmem::ObjectConfig { - map_wc: true, + map_wc, parent_resv_obj: None, }, BoCreateArgs { flags }, - ) + )?; + + if map_wc { + // SAFETY: `ddev` is bound for the duration of this call. + let dev = unsafe { ddev.as_ref().as_ref().as_bound() }; + bo.sg_table(dev)?; + } + + Ok(bo) } /// Look up a GEM object by handle for a DRM file. @@ -88,18 +112,17 @@ pub(crate) fn lookup_handle(file: &TyrDrmFile, handle: u32) -> Result<ARef<Bo>> } /// Creates a dummy GEM object to serve as the root of a GPUVM. -pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice) -> Result<ARef<Bo>> { - let bo = Bo::new( +pub(crate) fn new_dummy_object(ddev: &TyrDrmDevice, coherent: bool) -> Result<ARef<Bo>> { + // FIXME: use a Rust resv-object abstraction once available, rather than a real BO. + Bo::new( ddev, 4096, shmem::ObjectConfig { - map_wc: true, + map_wc: should_map_wc(coherent), parent_resv_obj: None, }, BoCreateArgs { flags: 0 }, - )?; - - Ok(bo) + ) } /// Specifies how to choose a GPU virtual address for a [`KernelBo`]. @@ -137,6 +160,7 @@ pub(crate) fn new( size: u64, va_alloc: KernelBoVaAlloc, flags: VmMapFlags, + coherent: bool, ) -> Result<Self> { if size == 0 { dev_err!(vm.dev(), "Cannot create KernelBo with size 0"); @@ -152,7 +176,7 @@ pub(crate) fn new( ddev, bo_size, shmem::ObjectConfig { - map_wc: true, + map_wc: should_map_wc(coherent), parent_resv_obj: None, }, BoCreateArgs { flags: 0 }, diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs index 9f4d2e23a3ab4..c9ef7c3ff1b9e 100644 --- a/drivers/gpu/drm/tyr/vm.rs +++ b/drivers/gpu/drm/tyr/vm.rs @@ -474,13 +474,14 @@ pub(crate) fn new_for_fw( ddev: &TyrDrmDevice, mmu: ArcBorrow<'_, Mmu<'drm>>, gpu_info: &GpuInfo, + coherent: bool, ) -> Result<Arc<Vm<'drm>>> { // As in panthor: the CSF MCU is a Cortex-M7 and can only address 4G. let layout = VmLayout { full: 0..u64::SZ_4G, user: 0..0u64, }; - Self::new_internal(dev, ddev, mmu, gpu_info, layout) + Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent) } /// Creates a user VM, splitting the GPU VA range per `user_va`. @@ -490,6 +491,7 @@ pub(crate) fn new_for_user( mmu: ArcBorrow<'_, Mmu<'drm>>, gpu_info: &GpuInfo, user_va: UserVaRequest, + coherent: bool, ) -> Result<Arc<Vm<'drm>>> { let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features); let va_bits = mmu_features.va_bits().get(); @@ -503,7 +505,7 @@ pub(crate) fn new_for_user( range.end ); })?; - Self::new_internal(dev, ddev, mmu, gpu_info, layout) + Self::new_internal(dev, ddev, mmu, gpu_info, layout, coherent) } /// Initializes a VM with the given layout. @@ -513,6 +515,7 @@ fn new_internal( mmu: ArcBorrow<'_, Mmu<'drm>>, gpu_info: &GpuInfo, layout: VmLayout, + coherent: bool, ) -> Result<Arc<Vm<'drm>>> { let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features); let va_bits = mmu_features.va_bits().get(); @@ -521,7 +524,7 @@ fn new_internal( let reserve_range = 0..0u64; // dummy_obj is used to initialize the GPUVM tree. - let dummy_obj = gem::new_dummy_object(ddev).inspect_err(|e| { + let dummy_obj = gem::new_dummy_object(ddev, coherent).inspect_err(|e| { dev_err!(dev, "Failed to create dummy GEM object: {:?}", e); })?; -- 2.43.0
