From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751758AbdF2FHJ (ORCPT ); Thu, 29 Jun 2017 01:07:09 -0400 Received: from mail-pg0-f47.google.com ([74.125.83.47]:35894 "EHLO mail-pg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751554AbdF2FHE (ORCPT ); Thu, 29 Jun 2017 01:07:04 -0400 Date: Wed, 28 Jun 2017 22:06:57 -0700 From: Bjorn Andersson To: Arnd Bergmann Cc: Rob Clark , Jordan Crouse , David Airlie , Eric Anholt , Sushmita Susheelendra , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations Message-ID: <20170629050657.GX18666@tuxbook> References: <20170620201720.225593-1-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170620201720.225593-1-arnd@arndb.de> User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 20 Jun 13:16 PDT 2017, Arnd Bergmann wrote: > In zap_shader_load_mdt(), we pass a pointer to a phys_addr_t > into dmam_alloc_coherent, which the compiler warns about: > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c: In function 'zap_shader_load_mdt': > drivers/gpu/drm/msm/adreno/a5xx_gpu.c:54:50: error: passing argument 3 of 'dmam_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types] > > The returned DMA address is later passed on to a function that > takes a phys_addr_t, so it's clearly wrong to use the DMA > mapping interface here: the memory may be uncached, or the > address may be completely wrong if there is an IOMMU connected > to the device. > > My interpretation is that using dmam_alloc_coherent() had two > purposes: > > a) get a chunk of consecutive memory that may be larger than > the limit for kmalloc() > > b) use the devres infrastructure to simplify the unwinding > in the error case. > > I think ideally we'd use a devres-based version of > alloc_pages_exact() here, but since that doesn't exist, > let's use devm_get_free_pages() instead. This wastes a little > memory as the size gets rounded up to a power of two, but > is otherwise harmless. If we want to save memory here, calling > devm_free_pages() to release the memory once it is no longer > needed is probably better anyway. > > Fixes: 7c65817e6d38 ("drm/msm: gpu: Enable zap shader for A5XX") > Signed-off-by: Arnd Bergmann Acked-by: Bjorn Andersson Regards, Bjorn > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index b4b54f1c24bc..eee9ac81aaa1 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -51,11 +51,13 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname) > } > > /* Allocate memory for the firmware image */ > - mem_region = dmam_alloc_coherent(dev, mem_size, &mem_phys, GFP_KERNEL); > + mem_region = (void *)devm_get_free_pages(dev, GFP_KERNEL, > + get_order(mem_size)); > if (!mem_region) { > ret = -ENOMEM; > goto out; > } > + mem_phys = virt_to_phys(mem_region); > > /* Load the rest of the MDT */ > ret = qcom_mdt_load(dev, fw, fwname, GPU_PAS_ID, mem_region, mem_phys, > -- > 2.9.0 >