From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751693AbdGZQfM (ORCPT ); Wed, 26 Jul 2017 12:35:12 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:58038 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751109AbdGZQfK (ORCPT ); Wed, 26 Jul 2017 12:35:10 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 21CEB6032D Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=jcrouse@codeaurora.org Date: Wed, 26 Jul 2017 10:35:05 -0600 From: Jordan Crouse To: Arnd Bergmann Cc: Rob Clark , Stanimir Varbanov , David Airlie , Bjorn Andersson , 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 2/2] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations Message-ID: <20170726163505.GA8482@jcrouse-lnx.qualcomm.com> Mail-Followup-To: Arnd Bergmann , Rob Clark , Stanimir Varbanov , David Airlie , Bjorn Andersson , Sushmita Susheelendra , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20170726155329.581707-1-arnd@arndb.de> <20170726155329.581707-2-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170726155329.581707-2-arnd@arndb.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 26, 2017 at 05:52:45PM +0200, 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. What the code actually wants to do is to get > the physical address from the reserved-mem node. It goes through > the dma-mapping interfaces for obscure reasons, and this > apparently only works by chance, relying on specific bugs > in the error handling of the arm64 dma-mapping implementation. > > The same problem existed in the "venus" media driver, which was > now fixed by Stanimir Varbanov after long discussions. > > In order to make some progress here, I have now ported his > approach over to the adreno driver. The patch is currently > untested, and should get a good review, but it is now much > simpler than the original, and it should be obvious what > goes wrong if I made a mistake in the port. > > See also: a6e2d36bf6b7 ("media: venus: don't abuse dma_alloc for non-DMA allocations") > Cc: Stanimir Varbanov > Fixes: 7c65817e6d38 ("drm/msm: gpu: Enable zap shader for A5XX") > Signed-off-by: Arnd Bergmann > --- > I think we want this to be applied for 4.13, as the upstream > code that was added in the merge window is seriously broken without it > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 71 ++++++++++++----------------------- > drivers/gpu/drm/msm/adreno/a5xx_gpu.h | 2 - > 2 files changed, 23 insertions(+), 50 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index 1d54c76a7778..ce545b3a9d17 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -15,7 +15,7 @@ > #include > #include > #include > -#include > +#include > #include > #include "msm_gem.h" > #include "msm_mmu.h" > @@ -29,6 +29,8 @@ static void a5xx_dump(struct msm_gpu *gpu); > static int zap_shader_load_mdt(struct device *dev, const char *fwname) > { > const struct firmware *fw; > + struct device_node *np; > + struct resource r; > phys_addr_t mem_phys; > ssize_t mem_size; > void *mem_region = NULL; > @@ -37,6 +39,21 @@ static int zap_shader_load_mdt(struct device *dev, const char *fwname) > if (!IS_ENABLED(CONFIG_ARCH_QCOM)) > return -EINVAL; > > + np = of_get_child_by_name(dev->of_node, "zap-shader"); > + if (!np) > + return -ENODEV; > + > + np = of_parse_phandle(dev->of_node, "memory-region", 0); I think this should be np = of_parse_phandle(np, "memory-region", 0); With that change this patch works great. > @@ -373,44 +393,6 @@ static int a5xx_zap_shader_resume(struct msm_gpu *gpu) > } > > /* Set up a child device to "own" the zap shader */ This now incorrect comment can be zapped (pun intended). > -static int a5xx_zap_shader_dev_init(struct device *parent, struct device *dev) > -{ > - struct device_node *node; > - int ret; > - > - if (dev->parent) > - return 0; > - With above changes, Acked-and-Tested-By: Jordan Crouse -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project