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 5EA9DCE8D52 for ; Fri, 14 Nov 2025 16:22:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DCD0B10EAC5; Fri, 14 Nov 2025 16:22:16 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 61FFE10EAC5; Fri, 14 Nov 2025 16:22:15 +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 5B0FC1063; Fri, 14 Nov 2025 08:22:07 -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 42B143F5A1; Fri, 14 Nov 2025 08:22:08 -0800 (PST) Message-ID: <468df8dc-0dab-449f-b48a-50470403ca3b@arm.com> Date: Fri, 14 Nov 2025 16:22:06 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 15/16] drm/panfrost: Add flag to map GEM object Write-Back Cacheable 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-16-boris.brezillon@collabora.com> From: Steven Price Content-Language: en-GB In-Reply-To: <20251030140525.366636-16-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 > > Will be used by the UMD to optimize CPU accesses to buffers > that are frequently read by the CPU, or on which the access > pattern makes non-cacheable mappings inefficient. > > Mapping buffers CPU-cached implies taking care of the CPU > cache maintenance in the UMD, unless the GPU is IO coherent. > > v2: > - Add more to the commit message > > v3: > - No changes > > v4: > - Fix the map_wc test in panfrost_ioctl_query_bo_info() > > v5: > - Drop Steve's R-b (enough has changed to justify a new review) > > Signed-off-by: Faith Ekstrand > Signed-off-by: Boris Brezillon Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++-- > drivers/gpu/drm/panfrost/panfrost_gem.c | 33 +++++++++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_gem.h | 5 ++++ > include/uapi/drm/panfrost_drm.h | 5 +++- > 4 files changed, 50 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index ba03a4420264..74b7dc75d88b 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -125,6 +125,10 @@ static int panfrost_ioctl_get_param(struct drm_device *ddev, void *data, struct > return 0; > } > > +#define PANFROST_BO_FLAGS (PANFROST_BO_NOEXEC | \ > + PANFROST_BO_HEAP | \ > + PANFROST_BO_WB_MMAP) > + > static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data, > struct drm_file *file) > { > @@ -134,8 +138,7 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data, > struct panfrost_gem_mapping *mapping; > int ret; > > - if (!args->size || args->pad || > - (args->flags & ~(PANFROST_BO_NOEXEC | PANFROST_BO_HEAP))) > + if (!args->size || args->pad || (args->flags & ~PANFROST_BO_FLAGS)) > return -EINVAL; > > /* Heaps should never be executable */ > @@ -652,6 +655,9 @@ static int panfrost_ioctl_query_bo_info(struct drm_device *dev, void *data, > > if (bo->is_heap) > args->create_flags |= PANFROST_BO_HEAP; > + > + if (!bo->base.map_wc) > + args->create_flags |= PANFROST_BO_WB_MMAP; > } > > drm_gem_object_put(gem_obj); > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 05d3f8a6fa78..1c600939c17a 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -269,6 +269,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { > .vmap = drm_gem_shmem_object_vmap, > .vunmap = drm_gem_shmem_object_vunmap, > .mmap = drm_gem_shmem_object_mmap, > + .export = drm_gem_prime_export, > .status = panfrost_gem_status, > .rss = panfrost_gem_rss, > .vm_ops = &drm_gem_shmem_vm_ops, > @@ -302,12 +303,42 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > return &obj->base.base; > } > > +static bool > +should_map_wc(struct panfrost_gem_object *bo) > +{ > + struct panfrost_device *pfdev = to_panfrost_device(bo->base.base.dev); > + > + /* We can't do uncached mappings if the device is coherent, > + * because the zeroing done by the shmem layer at page allocation > + * time happens on a cached mapping which isn't CPU-flushed (at least > + * not on Arm64 where the flush is deferred to PTE setup time, and > + * only done conditionally based on the mapping permissions). We can't > + * rely on dma_map_sgtable()/dma_sync_sgtable_for_xxx() either to flush > + * those, because they are NOPed if dma_dev_coherent() returns true. > + */ > + if (pfdev->coherent) > + return false; > + > + /* Cached mappings are explicitly requested, so no write-combine. */ > + if (bo->wb_mmap) > + return false; > + > + /* The default is write-combine. */ > + return true; > +} > + > struct panfrost_gem_object * > panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > { > struct drm_gem_shmem_object *shmem; > struct panfrost_gem_object *bo; > > + /* The heap buffer is not supposed to be CPU-visible, so don't allow > + * WB_MMAP on those. > + */ > + if ((flags & PANFROST_BO_HEAP) && (flags & PANFROST_BO_WB_MMAP)) > + return ERR_PTR(-EINVAL); > + > /* Round up heap allocations to 2MB to keep fault handling simple */ > if (flags & PANFROST_BO_HEAP) > size = roundup(size, SZ_2M); > @@ -319,6 +350,8 @@ panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags) > bo = to_panfrost_bo(&shmem->base); > bo->noexec = !!(flags & PANFROST_BO_NOEXEC); > bo->is_heap = !!(flags & PANFROST_BO_HEAP); > + bo->wb_mmap = !!(flags & PANFROST_BO_WB_MMAP); > + bo->base.map_wc = should_map_wc(bo); > > return bo; > } > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 87b918f30baa..d2d532b3007a 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -98,6 +98,11 @@ struct panfrost_gem_object { > bool noexec :1; > bool is_heap :1; > > + /* On coherent devices, this reflects the creation flags, not the true > + * cacheability attribute of the mapping. > + */ > + bool wb_mmap :1; > + > #ifdef CONFIG_DEBUG_FS > struct panfrost_gem_debugfs debugfs; > #endif > diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h > index 743c79a38f1b..82f4e69bafb4 100644 > --- a/include/uapi/drm/panfrost_drm.h > +++ b/include/uapi/drm/panfrost_drm.h > @@ -101,9 +101,12 @@ struct drm_panfrost_wait_bo { > __s64 timeout_ns; /* absolute */ > }; > > -/* Valid flags to pass to drm_panfrost_create_bo */ > +/* Valid flags to pass to drm_panfrost_create_bo. > + * PANFROST_BO_WB_MMAP can't be set if PANFROST_BO_HEAP is. > + */ > #define PANFROST_BO_NOEXEC 1 > #define PANFROST_BO_HEAP 2 > +#define PANFROST_BO_WB_MMAP 4 > > /** > * struct drm_panfrost_create_bo - ioctl argument for creating Panfrost BOs.