From patchwork Thu Oct 29 00:16:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 319301 Delivered-To: patch@linaro.org Received: by 2002:a17:906:3bd4:0:0:0:0 with SMTP id v20csp26887ejf; Wed, 28 Oct 2020 17:18:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz9kxAL0z8ZfKgjWj7w5U+PoxxsCMzcRx1BrSdej5wQMfCB5rmi+LJ9tjpYXYvHGs5T5hL1 X-Received: by 2002:a17:90b:a0a:: with SMTP id gg10mr1320763pjb.1.1603930610384; Wed, 28 Oct 2020 17:16:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1603930610; cv=none; d=google.com; s=arc-20160816; b=N4jfz/FCiNBidftCwh+SP8IF4qsoOWLrbQqKj2V5dyMKPp7UoeSminFBGigi9d7NCm DdkI2S6Tg9WUlB6pMK55mhvhp2OV3UJwvpsdAQ2MMmrtHSlKo5rD3bix6Dvhfegkpt/6 TLRLQTn4X4aXjZE3H6756rysdeUvRAaXt4dDtAzX3WkEi4LCu5qh3EKI3skl3awxjTwm e1RzjIqD7kiuGiZ1CuA0smGd4Idy4MLznih4RzWbOuBQ7wFybjRTHhjI4NGc3CMEH+OB fSfjHlE5l8kXn7gBTNayIXq6YnDZqUna2UbrxaY4sr9BeByOSF9nbKMA/HrjFLftS+P7 l31A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:mime-version:references:in-reply-to:message-id:date :subject:to:from:dkim-signature:delivered-to; bh=Dier8YAvgZ+GACN/3+bdP9Ew5cm1s8XIYhDSnUzFVuM=; b=EjRE37Mnur5tncrE8oXOgMgeKmRKIiLUCO8kxL8kvh7wg+K5yKPerfFSBUBGpatu5r YFndYku4I+Tu1xDjsxQRlPwVLrhw6UWv2J0DAamfoGo6E1Gvoo8p0EEqd8pgqi2qBkWf YYRZGka0jQ8iwmZ6RtYbTIEamGgFsLIm0b1n8d9hLwXQu3p7kWawS2UcAFFE+s2kIFRq /OZ/1Vjr5r9dTervJvZY2+KmaQyxZYRbwReW6cypTZXzdWVrxRUz5I3jB/8PWsEpv3Nn G5xbTwyPcQnyQEHlyOsmCXC9nG70nJ2FSL6dustVtQaZ7v2CB4wRfuvf8abkaJCDNFIX pJCw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=aFsNi8r5; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from gabe.freedesktop.org (gabe.freedesktop.org. [131.252.210.177]) by mx.google.com with ESMTPS id g9si797054pjv.25.2020.10.28.17.16.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Oct 2020 17:16:50 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) client-ip=131.252.210.177; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=aFsNi8r5; spf=pass (google.com: best guess record for domain of dri-devel-bounces@lists.freedesktop.org designates 131.252.210.177 as permitted sender) smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F00D56E82D; Thu, 29 Oct 2020 00:16:41 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by gabe.freedesktop.org (Postfix) with ESMTPS id 26D5B6E827 for ; Thu, 29 Oct 2020 00:16:40 +0000 (UTC) Received: by mail-pf1-x441.google.com with SMTP id j18so865776pfa.0 for ; Wed, 28 Oct 2020 17:16:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=6oHXHsrLfgpTNXoI1E5O96yDTjCsZQKa/PUoP1mStQ0=; b=aFsNi8r5TQNkIGq4jHg8x+fZ4Sivy5fazTfvhS9Wjp0RtlIvCP17j6eIPH/EMqIidW 3e5p9sJyP3pCGwfqSjGBmPoGPusvyVJj1PHQXW50lHoxM/2zmMuIOg2KQ21kwPOKTMc9 QEvaOyX3V4+sfa1DXo+EFTdN27E3Q3xz69+7gC7ofDRvdQqQYv3cQU8BdfC6vk/OydgF AdrpNJ9HUbk4nwfgFYBehMfQIZYEMZu0+xGYGuuO+3dDV4Rls6yDpOmuR4AFu3O3OrGV H5iAOKDUrZ836HbcPrv1S3mEcHvfDoa4hBzm5VYx/7Bx9xC9oqualByhWIcbpWggXMAN gmmA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=6oHXHsrLfgpTNXoI1E5O96yDTjCsZQKa/PUoP1mStQ0=; b=pYgK3Dn0fJjt+2wSUG9I4YOQyKmtMptqwi5zlE1pZxPsJwnwCFq4XTzSvOw+iTjwoB N8xu7QtojRJmiE4K4TLAIgLGskZSJp/6Q9x69uGVZ9rCzt0yCm5rEmOnMtS4Rze03ngj YCzI4NorXHgqWeOevN4lmkw6b9ATe4X2q8w8eDhy9gywzYbp8HlJ4m/X1WYHbO43LbSn qD1YxzU1uNnBVwyyRuEj/q+wO08CbSEI42ynYJ5ma4/6IuaGBrHxIam/csPzXdrITPEG yVF+vidjrKcQSwi+zDCb7zLGxANjbyZcBv9+CQTSGKPNE/PIy3xjmNe4hB5Urv2Bkga+ CWfA== X-Gm-Message-State: AOAM531I0Fo6xu5i92HTxljBuf5IEBsrF0fpXkE5Rastb+jbRCv04YOC NYWbNsB3D2VCxWfpyShdQSdAyQ== X-Received: by 2002:a63:e553:: with SMTP id z19mr1668830pgj.102.1603930599690; Wed, 28 Oct 2020 17:16:39 -0700 (PDT) Received: from localhost.localdomain ([2601:1c2:680:1319:692:26ff:feda:3a81]) by smtp.gmail.com with ESMTPSA id u13sm727407pfl.162.2020.10.28.17.16.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Oct 2020 17:16:39 -0700 (PDT) From: John Stultz To: lkml Subject: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap Date: Thu, 29 Oct 2020 00:16:24 +0000 Message-Id: <20201029001624.17513-8-john.stultz@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20201029001624.17513-1-john.stultz@linaro.org> References: <20201029001624.17513-1-john.stultz@linaro.org> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Sandeep Patil , dri-devel@lists.freedesktop.org, Ezequiel Garcia , Robin Murphy , James Jones , Liam Mark , Laura Abbott , Chris Goldsworthy , Hridya Valsaraju , =?utf-8?q?=C3=98rjan_Eide?= , linux-media@vger.kernel.org, Suren Baghdasaryan , Daniel Mentz Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" This adds a heap that allocates non-contiguous buffers that are marked as writecombined, so they are not cached by the CPU. This is useful, as most graphics buffers are usually not touched by the CPU or only written into once by the CPU. So when mapping the buffer over and over between devices, we can skip the CPU syncing, which saves a lot of cache management overhead, greatly improving performance. For folk using ION, there was a ION_FLAG_CACHED flag, which signaled if the returned buffer should be CPU cacheable or not. With DMA-BUF heaps, we do not yet have such a flag, and by default the current heaps (system and cma) produce CPU cachable buffers. So for folks transitioning from ION to DMA-BUF Heaps, this fills in some of that missing functionality. There has been a suggestion to make this functionality a flag (DMAHEAP_FLAG_UNCACHED?) on the system heap, similar to how ION used the ION_FLAG_CACHED. But I want to make sure an _UNCACHED flag would truely be a generic attribute across all heaps. So far that has been unclear, so having it as a separate heap seemes better for now. (But I'm open to discussion on this point!) This is a rework of earlier efforts to add a uncached system heap, done utilizing the exisitng system heap, adding just a bit of logic to handle the uncached case. Feedback would be very welcome! Many thanks to Liam Mark for his help to get this working. Pending opensource users of this code include: * AOSP HiKey960 gralloc: - https://android-review.googlesource.com/c/device/linaro/hikey/+/1399519 - Visibly improves performance over the system heap * AOSP Codec2 (possibly, needs more review): - https://android-review.googlesource.com/c/platform/frameworks/av/+/1360640/17/media/codec2/vndk/C2DmaBufAllocator.cpp#325 Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v4: * Make sys_uncached_heap static, as Reported-by: kernel test robot * Fix wrong return value, caught by smatch Reported-by: kernel test robot Reported-by: Dan Carpenter * Ensure we call flush/invalidate_kernel_vmap_range() in the uncached cases to try to address feedback about VIVT caches from Christoph * Reorder a few lines as suggested by BrianS * Avoid holding the initial mapping for the lifetime of the buffer as suggested by BrianS * Fix a unlikely race between allocate and updating the dma_mask that BrianS noticed. --- drivers/dma-buf/heaps/system_heap.c | 111 ++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 16 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index ef4b2c1032df..a328c76249d2 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -22,6 +22,7 @@ #include static struct dma_heap *sys_heap; +static struct dma_heap *sys_uncached_heap; struct system_heap_buffer { struct dma_heap *heap; @@ -31,6 +32,8 @@ struct system_heap_buffer { struct sg_table sg_table; int vmap_cnt; void *vaddr; + + bool uncached; }; struct dma_heap_attachment { @@ -38,6 +41,8 @@ struct dma_heap_attachment { struct sg_table *table; struct list_head list; bool mapped; + + bool uncached; }; #define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ @@ -94,7 +99,7 @@ static int system_heap_attach(struct dma_buf *dmabuf, a->dev = attachment->dev; INIT_LIST_HEAD(&a->list); a->mapped = false; - + a->uncached = buffer->uncached; attachment->priv = a; mutex_lock(&buffer->lock); @@ -124,9 +129,13 @@ static struct sg_table *system_heap_map_dma_buf(struct dma_buf_attachment *attac { struct dma_heap_attachment *a = attachment->priv; struct sg_table *table = a->table; + int attr = 0; int ret; - ret = dma_map_sgtable(attachment->dev, table, direction, 0); + if (a->uncached) + attr = DMA_ATTR_SKIP_CPU_SYNC; + + ret = dma_map_sgtable(attachment->dev, table, direction, attr); if (ret) return ERR_PTR(ret); @@ -139,9 +148,12 @@ static void system_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, enum dma_data_direction direction) { struct dma_heap_attachment *a = attachment->priv; + int attr = 0; + if (a->uncached) + attr = DMA_ATTR_SKIP_CPU_SYNC; a->mapped = false; - dma_unmap_sgtable(attachment->dev, table, direction, 0); + dma_unmap_sgtable(attachment->dev, table, direction, attr); } static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, @@ -155,10 +167,12 @@ static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, if (buffer->vmap_cnt) invalidate_kernel_vmap_range(buffer->vaddr, buffer->len); - list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + if (!buffer->uncached) { + list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; + dma_sync_sgtable_for_cpu(a->dev, a->table, direction); + } } mutex_unlock(&buffer->lock); @@ -176,10 +190,12 @@ static int system_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, if (buffer->vmap_cnt) flush_kernel_vmap_range(buffer->vaddr, buffer->len); - list_for_each_entry(a, &buffer->attachments, list) { - if (!a->mapped) - continue; - dma_sync_sgtable_for_device(a->dev, a->table, direction); + if (!buffer->uncached) { + list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; + dma_sync_sgtable_for_device(a->dev, a->table, direction); + } } mutex_unlock(&buffer->lock); @@ -194,6 +210,9 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) struct sg_page_iter piter; int ret; + if (buffer->uncached) + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + for_each_sgtable_page(table, &piter, vma->vm_pgoff) { struct page *page = sg_page_iter_page(&piter); @@ -215,17 +234,21 @@ static void *system_heap_do_vmap(struct system_heap_buffer *buffer) struct page **pages = vmalloc(sizeof(struct page *) * npages); struct page **tmp = pages; struct sg_page_iter piter; + pgprot_t pgprot = PAGE_KERNEL; void *vaddr; if (!pages) return ERR_PTR(-ENOMEM); + if (buffer->uncached) + pgprot = pgprot_writecombine(PAGE_KERNEL); + for_each_sgtable_page(table, &piter, 0) { WARN_ON(tmp - pages >= npages); *tmp++ = sg_page_iter_page(&piter); } - vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL); + vaddr = vmap(pages, npages, VM_MAP, pgprot); vfree(pages); if (!vaddr) @@ -320,10 +343,11 @@ static struct page *alloc_largest_available(unsigned long size, return NULL; } -static int system_heap_allocate(struct dma_heap *heap, - unsigned long len, - unsigned long fd_flags, - unsigned long heap_flags) +static int system_heap_do_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags, + bool uncached) { struct system_heap_buffer *buffer; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); @@ -344,6 +368,7 @@ static int system_heap_allocate(struct dma_heap *heap, mutex_init(&buffer->lock); buffer->heap = heap; buffer->len = len; + buffer->uncached = uncached; INIT_LIST_HEAD(&pages); i = 0; @@ -393,6 +418,18 @@ static int system_heap_allocate(struct dma_heap *heap, /* just return, as put will call release and that will free */ return ret; } + + /* + * For uncached buffers, we need to initially flush cpu cache, since + * the __GFP_ZERO on the allocation means the zeroing was done by the + * cpu and thus it is likely cached. Map (and implicitly flush) and + * unmap it now so we don't get corruption later on. + */ + if (buffer->uncached) { + dma_map_sgtable(dma_heap_get_dev(heap), table, DMA_BIDIRECTIONAL, 0); + dma_unmap_sgtable(dma_heap_get_dev(heap), table, DMA_BIDIRECTIONAL, 0); + } + return ret; free_pages: @@ -410,10 +447,40 @@ static int system_heap_allocate(struct dma_heap *heap, return ret; } +static int system_heap_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) +{ + return system_heap_do_allocate(heap, len, fd_flags, heap_flags, false); +} + static const struct dma_heap_ops system_heap_ops = { .allocate = system_heap_allocate, }; +static int system_uncached_heap_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) +{ + return system_heap_do_allocate(heap, len, fd_flags, heap_flags, true); +} + +/* Dummy function to be used until we can call coerce_mask_and_coherent */ +static int system_uncached_heap_not_initialized(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) +{ + return -EBUSY; +} + +static struct dma_heap_ops system_uncached_heap_ops = { + /* After system_heap_create is complete, we will swap this */ + .allocate = system_uncached_heap_not_initialized, +}; + static int system_heap_create(void) { struct dma_heap_export_info exp_info; @@ -426,6 +493,18 @@ static int system_heap_create(void) if (IS_ERR(sys_heap)) return PTR_ERR(sys_heap); + exp_info.name = "system-uncached"; + exp_info.ops = &system_uncached_heap_ops; + exp_info.priv = NULL; + + sys_uncached_heap = dma_heap_add(&exp_info); + if (IS_ERR(sys_uncached_heap)) + return PTR_ERR(sys_uncached_heap); + + dma_coerce_mask_and_coherent(dma_heap_get_dev(sys_uncached_heap), DMA_BIT_MASK(64)); + mb(); /* make sure we only set allocate after dma_mask is set */ + system_uncached_heap_ops.allocate = system_uncached_heap_allocate; + return 0; } module_init(system_heap_create);