From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751643AbdGZVTO (ORCPT ); Wed, 26 Jul 2017 17:19:14 -0400 Received: from mail-pg0-f51.google.com ([74.125.83.51]:34172 "EHLO mail-pg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751623AbdGZVTM (ORCPT ); Wed, 26 Jul 2017 17:19:12 -0400 Date: Wed, 26 Jul 2017 14:19:08 -0700 From: Bjorn Andersson To: Arnd Bergmann Cc: Rob Clark , David Airlie , Jordan Crouse , Stanimir Varbanov , 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 v3] drm/msm: gpu: don't abuse dma_alloc for non-DMA allocations Message-ID: <20170726211908.GR20973@minitux> References: <20170726200007.2432476-1-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170726200007.2432476-1-arnd@arndb.de> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 26 Jul 12:59 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. 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") > Acked-and-Tested-by: Jordan Crouse > Signed-off-by: Arnd Bergmann Acked-by: Bjorn Andersson However... [..] > @@ -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(np, "memory-region", 0); > + if (!np) > + return -EINVAL; > + > + ret = of_address_to_resource(np, 0, &r); > + if (ret) > + return ret; > + > + mem_phys = r.start; > + mem_size = resource_size(&r); > + ...this means that it's now required that the reserved-memory region has a "reg", the prior solution supported that we just specify a "size". I have another driver where I need to figure out a mechanism of getting hold of the dynamically allocated "base" of struct reserved_mem. So I think it's appropriate to apply this fix now and loosen the restrictions on placement later. Regards, Bjorn