On Tue, Jun 20, 2017 at 10:16:50PM +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. > > 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 <arnd@xxxxxxxx> Seems reasonable to me if Bjorn agrees. Acked-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> > --- > 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 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel