Re: [PATCH 2/2] drm/msm: move domain allocation into msm_iommu_new()
Hi Dmitry, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on linus/master v6.1-rc1 next-20221020] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Baryshkov/drm-msm-clean-up-iommu-interface-a-bit/20221021-033404 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20221020193157.2147112-3-dmitry.baryshkov%40linaro.org patch subject: [PATCH 2/2] drm/msm: move domain allocation into msm_iommu_new() config: hexagon-randconfig-r041-20221018 compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/195a104cef7fdbeed05031a81725b6ae7b716bb8 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Dmitry-Baryshkov/drm-msm-clean-up-iommu-interface-a-bit/20221021-033404 git checkout 195a104cef7fdbeed05031a81725b6ae7b716bb8 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/msm/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/msm/adreno/a6xx_gmu.c:11: In file included from drivers/gpu/drm/msm/adreno/a6xx_gpu.h:8: In file included from drivers/gpu/drm/msm/adreno/adreno_gpu.h:13: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) ^ In file included from drivers/gpu/drm/msm/adreno/a6xx_gmu.c:11: In file included from drivers/gpu/drm/msm/adreno/a6xx_gpu.h:8: In file included from drivers/gpu/drm/msm/adreno/adreno_gpu.h:13: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) ^ In file included from drivers/gpu/drm/msm/adreno/a6xx_gmu.c:11: In file included from drivers/gpu/drm/msm/adreno/a6xx_gpu.h:8: In file included from drivers/gpu/drm/msm/adreno/adreno_gpu.h:13: In file included from include/linux/iopoll.h:14: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); ~~ ^ >> drivers/gpu/drm/m
[PATCH 2/2] drm/msm: move domain allocation into msm_iommu_new()
After the msm_iommu instance is created, the IOMMU domain is completely
handled inside the msm_iommu code. Move the iommu_domain_alloc() call
into the msm_iommu_new() to simplify callers code.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c| 7 ---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 20 +---
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 19 ++-
drivers/gpu/drm/msm/msm_drv.c| 18 --
drivers/gpu/drm/msm/msm_iommu.c | 18 +++---
drivers/gpu/drm/msm/msm_mmu.h| 3 ++-
6 files changed, 48 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e033d6a67a20..f9a529a98879 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1216,11 +1216,12 @@ static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu)
struct iommu_domain *domain;
struct msm_mmu *mmu;
- domain = iommu_domain_alloc(_bus_type);
- if (!domain)
+ mmu = msm_iommu_new(gmu->dev);
+ if (!mmu)
return -ENODEV;
+ if (IS_ERR(mmu))
+ return PTR_ERR(mmu);
- mmu = msm_iommu_new(gmu->dev, domain);
gmu->aspace = msm_gem_address_space_create(mmu, "gmu", 0x0, 0x8000);
if (IS_ERR(gmu->aspace)) {
iommu_domain_free(domain);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index ed26b8dfc789..511ade96045e 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -200,28 +200,26 @@ struct msm_gem_address_space *
adreno_iommu_create_address_space(struct msm_gpu *gpu,
struct platform_device *pdev)
{
- struct iommu_domain *iommu;
struct msm_mmu *mmu;
struct msm_gem_address_space *aspace;
+ struct iommu_domain_geometry *geometry;
u64 start, size;
- iommu = iommu_domain_alloc(_bus_type);
- if (!iommu)
- return NULL;
-
- mmu = msm_iommu_new(>dev, iommu);
- if (IS_ERR(mmu)) {
- iommu_domain_free(iommu);
+ mmu = msm_iommu_new(>dev);
+ if (IS_ERR_OR_NULL(mmu))
return ERR_CAST(mmu);
- }
+
+ geometry = msm_iommu_get_geometry(mmu);
+ if (IS_ERR(geometry))
+ return ERR_CAST(geometry);
/*
* Use the aperture start or SZ_16M, whichever is greater. This will
* ensure that we align with the allocated pagetable range while still
* allowing room in the lower 32 bits for GMEM and whatnot
*/
- start = max_t(u64, SZ_16M, iommu->geometry.aperture_start);
- size = iommu->geometry.aperture_end - start + 1;
+ start = max_t(u64, SZ_16M, geometry->aperture_start);
+ size = geometry->aperture_end - start + 1;
aspace = msm_gem_address_space_create(mmu, "gpu",
start & GENMASK_ULL(48, 0), size);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 964573d26d26..5f7488a539f5 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -387,7 +387,7 @@ static int mdp4_kms_init(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct mdp4_kms *mdp4_kms;
struct msm_kms *kms = NULL;
- struct iommu_domain *iommu;
+ struct msm_mmu *mmu;
struct msm_gem_address_space *aspace;
int irq, ret;
u32 major, minor;
@@ -499,10 +499,15 @@ static int mdp4_kms_init(struct drm_device *dev)
mdp4_disable(mdp4_kms);
mdelay(16);
- iommu = iommu_domain_alloc(pdev->dev.bus);
- if (iommu) {
- struct msm_mmu *mmu = msm_iommu_new(>dev, iommu);
-
+ mmu = msm_iommu_new(>dev);
+ if (IS_ERR(mmu)) {
+ ret = PTR_ERR(mmu);
+ goto fail;
+ } else if (!mmu) {
+ DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys "
+ "contig buffers for scanout\n");
+ aspace = NULL;
+ } else {
aspace = msm_gem_address_space_create(mmu,
"mdp4", 0x1000, 0x1 - 0x1000);
@@ -514,10 +519,6 @@ static int mdp4_kms_init(struct drm_device *dev)
}
kms->aspace = aspace;
- } else {
- DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys "
- "contig buffers for scanout\n");
- aspace = NULL;
}
ret = modeset_init(mdp4_kms);
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 28034c21f6bc..9d7275304785 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -276,7 +276,6 @@ static int msm_drm_uninit(struct device *dev)
