From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5F595CE7B1B for ; Fri, 14 Nov 2025 15:02:41 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D162B10EAA6; Fri, 14 Nov 2025 15:02:38 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id E55BB10EAA2; Fri, 14 Nov 2025 15:02:36 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AF7901063; Fri, 14 Nov 2025 07:02:28 -0800 (PST) Received: from [10.1.39.17] (e122027.cambridge.arm.com [10.1.39.17]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3C2113F66E; Fri, 14 Nov 2025 07:02:30 -0800 (PST) Message-ID: <5d511f84-5703-4992-b57d-9475dbce185d@arm.com> Date: Fri, 14 Nov 2025 15:02:28 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 03/16] drm/shmem: Add a drm_gem_shmem_sync() helper To: Boris Brezillon Cc: dri-devel@lists.freedesktop.org, Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Faith Ekstrand , Thierry Reding , Mikko Perttunen , Melissa Wen , =?UTF-8?Q?Ma=C3=ADra_Canal?= , Lucas De Marchi , =?UTF-8?Q?Thomas_Hellstr=C3=B6m?= , Rodrigo Vivi , Frank Binns , Matt Coster , Rob Clark , Dmitry Baryshkov , Abhinav Kumar , Jessica Zhang , Sean Paul , Marijn Suijten , Alex Deucher , =?UTF-8?Q?Christian_K=C3=B6nig?= , amd-gfx@lists.freedesktop.org, kernel@collabora.com References: <20251030140525.366636-1-boris.brezillon@collabora.com> <20251030140525.366636-4-boris.brezillon@collabora.com> From: Steven Price Content-Language: en-GB In-Reply-To: <20251030140525.366636-4-boris.brezillon@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" On 30/10/2025 14:05, Boris Brezillon wrote: > From: Faith Ekstrand > > This enables syncing mapped GEM objects between the CPU and GPU via calls > to dma_sync_*(). It's a bit annoying as it requires walking the sg_table > so it's best if every driver doesn't hand-roll it. > > v2: > - s/drm_gem_shmem_sync_mmap/drm_gem_shmem_sync/ > - Change the prototype to match drm_gem_object_funcs::sync() > - Add a wrapper for drm_gem_object_funcs::sync() > > v3: > - No changes > > v4: > - Add Steve's R-b > > v5: > - Change the semantics of the drm_gem_shmem_sync() helper to better > reflect the UMD cache flush/flush+invalidate semantics (discussed > with Faith) > - Drop R-bs > > Signed-off-by: Faith Ekstrand > Signed-off-by: Boris Brezillon Reviewed-by: Steven Price > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 93 ++++++++++++++++++++++++++ > include/drm/drm_gem_shmem_helper.h | 14 ++++ > 2 files changed, 107 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index e49c75739c20..d9266e22a0dc 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -690,6 +690,99 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap); > > +/** > + * drm_gem_shmem_sync - Sync CPU-mapped data > + * @shmem: shmem GEM object > + * @offset: Offset into the GEM object > + * @size: Size of the area to sync > + * @type: Type of synchronization > + * > + * Returns: > + * 0 on success or a negative error code on failure. > + */ > +int drm_gem_shmem_sync(struct drm_gem_shmem_object *shmem, size_t offset, > + size_t size, enum drm_gem_shmem_sync_type type) > +{ > + const struct drm_device *dev = shmem->base.dev; > + struct sg_table *sgt; > + struct scatterlist *sgl; > + unsigned int count; > + > + /* Make sure the range is in bounds. */ > + if (offset + size < offset || offset + size > shmem->base.size) > + return -EINVAL; > + > + /* Disallow CPU-cache maintenance on imported buffers. */ > + if (drm_gem_is_imported(&shmem->base)) > + return -EINVAL; > + > + switch (type) { > + case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH: > + case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE: > + break; > + > + default: > + return -EINVAL; > + } > + > + /* Don't bother if it's WC-mapped */ > + if (shmem->map_wc) > + return 0; > + > + /* Nothing to do if the size is zero. */ > + if (size == 0) > + return 0; > + > + sgt = drm_gem_shmem_get_pages_sgt(shmem); > + if (IS_ERR(sgt)) > + return PTR_ERR(sgt); > + > + for_each_sgtable_dma_sg(sgt, sgl, count) { > + if (size == 0) > + break; > + > + dma_addr_t paddr = sg_dma_address(sgl); > + size_t len = sg_dma_len(sgl); > + > + if (len <= offset) { > + offset -= len; > + continue; > + } > + > + paddr += offset; > + len -= offset; > + len = min_t(size_t, len, size); > + size -= len; > + offset = 0; > + > + /* It's unclear whether dma_sync_xxx() is the right API to do CPU > + * cache maintenance given an IOMMU can register their own > + * implementation doing more than just CPU cache flushes/invalidation, > + * and what we really care about here is CPU caches only, but that's > + * the best we have that is both arch-agnostic and does at least the > + * CPU cache maintenance on a tuple. > + * > + * Also, I wish we could do a single > + * > + * dma_sync_single_for_device(BIDIR) > + * > + * and get a flush+invalidate, but that's not how it's implemented > + * in practice (at least on arm64), so we have to make it > + * > + * dma_sync_single_for_device(TO_DEVICE) > + * dma_sync_single_for_cpu(FROM_DEVICE) > + * > + * for the flush+invalidate case. > + */ > + dma_sync_single_for_device(dev->dev, paddr, len, DMA_TO_DEVICE); > + if (type == DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE) > + dma_sync_single_for_cpu(dev->dev, paddr, len, DMA_FROM_DEVICE); > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(drm_gem_shmem_sync); > + > /** > * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs > * @shmem: shmem GEM object > diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h > index 075275d6b2fd..b0b6d0104a9a 100644 > --- a/include/drm/drm_gem_shmem_helper.h > +++ b/include/drm/drm_gem_shmem_helper.h > @@ -124,6 +124,20 @@ void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem, > struct iosys_map *map); > int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma); > > +/** > + * enum enum drm_gem_shmem_sync_type - Type of synchronization > + */ > +enum drm_gem_shmem_sync_type { > + /** DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH: Flush CPU caches */ > + DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH = 0, > + > + /** DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE: Flush and invalidate CPU caches */ > + DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE, > +}; > + > +int drm_gem_shmem_sync(struct drm_gem_shmem_object *shmem, size_t offset, > + size_t size, enum drm_gem_shmem_sync_type type); > + > int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem); > void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem); >