[PATCH 1/5] media: ipu6: not override the dma_ops of device in driver

[PATCH 1/5] media: ipu6: not override the dma_ops of device in driver

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

DMA ops are a helper for architectures and not for drivers to override
the DMA implementation.  Driver should not override the DMA
implementation. This patch remove the dma_ops override and expose the
IPU6 DMA mapping APIs.

Fixes: 9163d83573e4 ("media: intel/ipu6: add IPU6 DMA mapping API and MMU table")
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-bus.c      |   6 -
 drivers/media/pci/intel/ipu6/ipu6-buttress.c |  21 +-
 drivers/media/pci/intel/ipu6/ipu6-cpd.c      |  18 +-
 drivers/media/pci/intel/ipu6/ipu6-dma.c      | 195 +++++++++----------
 drivers/media/pci/intel/ipu6/ipu6-dma.h      |  34 +++-
 drivers/media/pci/intel/ipu6/ipu6-fw-com.c   |  14 +-
 6 files changed, 156 insertions(+), 132 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-bus.c b/drivers/media/pci/intel/ipu6/ipu6-bus.c
index 149ec098cdbf..37d88ddb6ee7 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-bus.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-bus.c
@@ -94,8 +94,6 @@ ipu6_bus_initialize_device(struct pci_dev *pdev, struct device *parent,
 	if (!adev)
 		return ERR_PTR(-ENOMEM);
 
-	adev->dma_mask = DMA_BIT_MASK(isp->secure_mode ? IPU6_MMU_ADDR_BITS :
-				      IPU6_MMU_ADDR_BITS_NON_SECURE);
 	adev->isp = isp;
 	adev->ctrl = ctrl;
 	adev->pdata = pdata;
@@ -106,10 +104,6 @@ ipu6_bus_initialize_device(struct pci_dev *pdev, struct device *parent,
 
 	auxdev->dev.parent = parent;
 	auxdev->dev.release = ipu6_bus_release;
-	auxdev->dev.dma_ops = &ipu6_dma_ops;
-	auxdev->dev.dma_mask = &adev->dma_mask;
-	auxdev->dev.dma_parms = pdev->dev.dma_parms;
-	auxdev->dev.coherent_dma_mask = adev->dma_mask;
 
 	ret = auxiliary_device_init(auxdev);
 	if (ret < 0) {
diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index e47f84c30e10..d66db537be4a 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -24,6 +24,7 @@
 
 #include "ipu6.h"
 #include "ipu6-bus.h"
+#include "ipu6-dma.h"
 #include "ipu6-buttress.h"
 #include "ipu6-platform-buttress-regs.h"
 
@@ -553,6 +554,7 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
 			       const struct firmware *fw, struct sg_table *sgt)
 {
 	bool is_vmalloc = is_vmalloc_addr(fw->data);
+	struct pci_dev *pdev = sys->isp->pdev;
 	struct page **pages;
 	const void *addr;
 	unsigned long n_pages;
@@ -588,14 +590,20 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
 		goto out;
 	}
 
-	ret = dma_map_sgtable(&sys->auxdev.dev, sgt, DMA_TO_DEVICE, 0);
-	if (ret < 0) {
-		ret = -ENOMEM;
+	ret = dma_map_sgtable(&pdev->dev, sgt, DMA_TO_DEVICE, 0);
+	if (ret) {
+		sg_free_table(sgt);
+		goto out;
+	}
+
+	ret = ipu6_dma_map_sgtable(sys, sgt, DMA_TO_DEVICE, 0);
+	if (ret) {
+		dma_unmap_sgtable(&pdev->dev, sgt, DMA_TO_DEVICE, 0);
 		sg_free_table(sgt);
 		goto out;
 	}
 
-	dma_sync_sgtable_for_device(&sys->auxdev.dev, sgt, DMA_TO_DEVICE);
+	ipu6_dma_sync_sgtable(sys, sgt);
 
 out:
 	kfree(pages);
@@ -607,7 +615,10 @@ EXPORT_SYMBOL_NS_GPL(ipu6_buttress_map_fw_image, INTEL_IPU6);
 void ipu6_buttress_unmap_fw_image(struct ipu6_bus_device *sys,
 				  struct sg_table *sgt)
 {
-	dma_unmap_sgtable(&sys->auxdev.dev, sgt, DMA_TO_DEVICE, 0);
+	struct pci_dev *pdev = sys->isp->pdev;
+
+	ipu6_dma_unmap_sgtable(sys, sgt, DMA_TO_DEVICE, 0);
+	dma_unmap_sgtable(&pdev->dev, sgt, DMA_TO_DEVICE, 0);
 	sg_free_table(sgt);
 }
 EXPORT_SYMBOL_NS_GPL(ipu6_buttress_unmap_fw_image, INTEL_IPU6);
diff --git a/drivers/media/pci/intel/ipu6/ipu6-cpd.c b/drivers/media/pci/intel/ipu6/ipu6-cpd.c
index 715b21ab4b8e..21c1c128a7ea 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-cpd.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-cpd.c
@@ -15,6 +15,7 @@
 #include "ipu6.h"
 #include "ipu6-bus.h"
 #include "ipu6-cpd.h"
+#include "ipu6-dma.h"
 
 /* 15 entries + header*/
 #define MAX_PKG_DIR_ENT_CNT		16
@@ -162,7 +163,6 @@ int ipu6_cpd_create_pkg_dir(struct ipu6_bus_device *adev, const void *src)
 {
 	dma_addr_t dma_addr_src = sg_dma_address(adev->fw_sgt.sgl);
 	const struct ipu6_cpd_ent *ent, *man_ent, *met_ent;
-	struct device *dev = &adev->auxdev.dev;
 	struct ipu6_device *isp = adev->isp;
 	unsigned int man_sz, met_sz;
 	void *pkg_dir_pos;
@@ -175,8 +175,8 @@ int ipu6_cpd_create_pkg_dir(struct ipu6_bus_device *adev, const void *src)
 	met_sz = met_ent->len;
 
 	adev->pkg_dir_size = PKG_DIR_SIZE + man_sz + met_sz;
-	adev->pkg_dir = dma_alloc_attrs(dev, adev->pkg_dir_size,
-					&adev->pkg_dir_dma_addr, GFP_KERNEL, 0);
+	adev->pkg_dir = ipu6_dma_alloc(adev, adev->pkg_dir_size,
+				       &adev->pkg_dir_dma_addr, GFP_KERNEL, 0);
 	if (!adev->pkg_dir)
 		return -ENOMEM;
 
@@ -198,8 +198,8 @@ int ipu6_cpd_create_pkg_dir(struct ipu6_bus_device *adev, const void *src)
 					 met_ent->len);
 	if (ret) {
 		dev_err(&isp->pdev->dev, "Failed to parse module data\n");
-		dma_free_attrs(dev, adev->pkg_dir_size,
-			       adev->pkg_dir, adev->pkg_dir_dma_addr, 0);
+		ipu6_dma_free(adev, adev->pkg_dir_size,
+			      adev->pkg_dir, adev->pkg_dir_dma_addr, 0);
 		return ret;
 	}
 
@@ -211,8 +211,8 @@ int ipu6_cpd_create_pkg_dir(struct ipu6_bus_device *adev, const void *src)
 	pkg_dir_pos += man_sz;
 	memcpy(pkg_dir_pos, src + met_ent->offset, met_sz);
 
-	dma_sync_single_range_for_device(dev, adev->pkg_dir_dma_addr,
-					 0, adev->pkg_dir_size, DMA_TO_DEVICE);
+	ipu6_dma_sync_single(adev, adev->pkg_dir_dma_addr,
+			     adev->pkg_dir_size);
 
 	return 0;
 }
@@ -220,8 +220,8 @@ EXPORT_SYMBOL_NS_GPL(ipu6_cpd_create_pkg_dir, INTEL_IPU6);
 
 void ipu6_cpd_free_pkg_dir(struct ipu6_bus_device *adev)
 {
-	dma_free_attrs(&adev->auxdev.dev, adev->pkg_dir_size, adev->pkg_dir,
-		       adev->pkg_dir_dma_addr, 0);
+	ipu6_dma_free(adev, adev->pkg_dir_size, adev->pkg_dir,
+		      adev->pkg_dir_dma_addr, 0);
 }
 EXPORT_SYMBOL_NS_GPL(ipu6_cpd_free_pkg_dir, INTEL_IPU6);
 
diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
index 92530a1cc90f..d03803e58881 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
@@ -39,8 +39,7 @@ static struct vm_info *get_vm_info(struct ipu6_mmu *mmu, dma_addr_t iova)
 	return NULL;
 }
 
-static void __dma_clear_buffer(struct page *page, size_t size,
-			       unsigned long attrs)
+static void __clear_buffer(struct page *page, size_t size, unsigned long attrs)
 {
 	void *ptr;
 
@@ -56,8 +55,7 @@ static void __dma_clear_buffer(struct page *page, size_t size,
 		clflush_cache_range(ptr, size);
 }
 
-static struct page **__dma_alloc_buffer(struct device *dev, size_t size,
-					gfp_t gfp, unsigned long attrs)
+static struct page **__alloc_buffer(size_t size, gfp_t gfp, unsigned long attrs)
 {
 	int count = PHYS_PFN(size);
 	int array_size = count * sizeof(struct page *);
@@ -86,7 +84,7 @@ static struct page **__dma_alloc_buffer(struct device *dev, size_t size,
 				pages[i + j] = pages[i] + j;
 		}
 
-		__dma_clear_buffer(pages[i], PAGE_SIZE << order, attrs);
+		__clear_buffer(pages[i], PAGE_SIZE << order, attrs);
 		i += 1 << order;
 		count -= 1 << order;
 	}
@@ -100,29 +98,26 @@ static struct page **__dma_alloc_buffer(struct device *dev, size_t size,
 	return NULL;
 }
 
-static void __dma_free_buffer(struct device *dev, struct page **pages,
-			      size_t size, unsigned long attrs)
+static void __free_buffer(struct page **pages, size_t size, unsigned long attrs)
 {
 	int count = PHYS_PFN(size);
 	unsigned int i;
 
 	for (i = 0; i < count && pages[i]; i++) {
-		__dma_clear_buffer(pages[i], PAGE_SIZE, attrs);
+		__clear_buffer(pages[i], PAGE_SIZE, attrs);
 		__free_pages(pages[i], 0);
 	}
 
 	kvfree(pages);
 }
 
-static void ipu6_dma_sync_single_for_cpu(struct device *dev,
-					 dma_addr_t dma_handle,
-					 size_t size,
-					 enum dma_data_direction dir)
+void ipu6_dma_sync_single(struct ipu6_bus_device *sys, dma_addr_t dma_handle,
+			  size_t size)
 {
 	void *vaddr;
 	u32 offset;
 	struct vm_info *info;
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
+	struct ipu6_mmu *mmu = sys->mmu;
 
 	info = get_vm_info(mmu, dma_handle);
 	if (WARN_ON(!info))
@@ -135,10 +130,10 @@ static void ipu6_dma_sync_single_for_cpu(struct device *dev,
 	vaddr = info->vaddr + offset;
 	clflush_cache_range(vaddr, size);
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_sync_single, INTEL_IPU6);
 
-static void ipu6_dma_sync_sg_for_cpu(struct device *dev,
-				     struct scatterlist *sglist,
-				     int nents, enum dma_data_direction dir)
+void ipu6_dma_sync_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		      int nents)
 {
 	struct scatterlist *sg;
 	int i;
@@ -146,14 +141,22 @@ static void ipu6_dma_sync_sg_for_cpu(struct device *dev,
 	for_each_sg(sglist, sg, nents, i)
 		clflush_cache_range(page_to_virt(sg_page(sg)), sg->length);
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_sync_sg, INTEL_IPU6);
 
-static void *ipu6_dma_alloc(struct device *dev, size_t size,
-			    dma_addr_t *dma_handle, gfp_t gfp,
-			    unsigned long attrs)
+void ipu6_dma_sync_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt)
 {
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
-	struct pci_dev *pdev = to_ipu6_bus_device(dev)->isp->pdev;
+	ipu6_dma_sync_sg(sys, sgt->sgl, sgt->orig_nents);
+}
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_sync_sgtable, INTEL_IPU6);
+
+void *ipu6_dma_alloc(struct ipu6_bus_device *sys, size_t size,
+		     dma_addr_t *dma_handle, gfp_t gfp,
+		     unsigned long attrs)
+{
+	struct device *dev = &sys->auxdev.dev;
+	struct pci_dev *pdev = sys->isp->pdev;
 	dma_addr_t pci_dma_addr, ipu6_iova;
+	struct ipu6_mmu *mmu = sys->mmu;
 	struct vm_info *info;
 	unsigned long count;
 	struct page **pages;
@@ -173,7 +176,7 @@ static void *ipu6_dma_alloc(struct device *dev, size_t size,
 	if (!iova)
 		goto out_kfree;
 
-	pages = __dma_alloc_buffer(dev, size, gfp, attrs);
+	pages = __alloc_buffer(size, gfp, attrs);
 	if (!pages)
 		goto out_free_iova;
 
@@ -227,7 +230,7 @@ static void *ipu6_dma_alloc(struct device *dev, size_t size,
 		ipu6_mmu_unmap(mmu->dmap->mmu_info, ipu6_iova, PAGE_SIZE);
 	}
 
-	__dma_free_buffer(dev, pages, size, attrs);
+	__free_buffer(pages, size, attrs);
 
 out_free_iova:
 	__free_iova(&mmu->dmap->iovad, iova);
@@ -236,13 +239,13 @@ static void *ipu6_dma_alloc(struct device *dev, size_t size,
 
 	return NULL;
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_alloc, INTEL_IPU6);
 
-static void ipu6_dma_free(struct device *dev, size_t size, void *vaddr,
-			  dma_addr_t dma_handle,
-			  unsigned long attrs)
+void ipu6_dma_free(struct ipu6_bus_device *sys, size_t size, void *vaddr,
+		   dma_addr_t dma_handle, unsigned long attrs)
 {
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
-	struct pci_dev *pdev = to_ipu6_bus_device(dev)->isp->pdev;
+	struct ipu6_mmu *mmu = sys->mmu;
+	struct pci_dev *pdev = sys->isp->pdev;
 	struct iova *iova = find_iova(&mmu->dmap->iovad, PHYS_PFN(dma_handle));
 	dma_addr_t pci_dma_addr, ipu6_iova;
 	struct vm_info *info;
@@ -281,7 +284,7 @@ static void ipu6_dma_free(struct device *dev, size_t size, void *vaddr,
 	ipu6_mmu_unmap(mmu->dmap->mmu_info, PFN_PHYS(iova->pfn_lo),
 		       PFN_PHYS(iova_size(iova)));
 
-	__dma_free_buffer(dev, pages, size, attrs);
+	__free_buffer(pages, size, attrs);
 
 	mmu->tlb_invalidate(mmu);
 
@@ -289,13 +292,14 @@ static void ipu6_dma_free(struct device *dev, size_t size, void *vaddr,
 
 	kfree(info);
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_free, INTEL_IPU6);
 
-static int ipu6_dma_mmap(struct device *dev, struct vm_area_struct *vma,
-			 void *addr, dma_addr_t iova, size_t size,
-			 unsigned long attrs)
+int ipu6_dma_mmap(struct ipu6_bus_device *sys, struct vm_area_struct *vma,
+		  void *addr, dma_addr_t iova, size_t size,
+		  unsigned long attrs)
 {
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
-	size_t count = PHYS_PFN(PAGE_ALIGN(size));
+	struct ipu6_mmu *mmu = sys->mmu;
+	size_t count = PFN_UP(size);
 	struct vm_info *info;
 	size_t i;
 	int ret;
@@ -323,18 +327,17 @@ static int ipu6_dma_mmap(struct device *dev, struct vm_area_struct *vma,
 	return 0;
 }
 
-static void ipu6_dma_unmap_sg(struct device *dev,
-			      struct scatterlist *sglist,
-			      int nents, enum dma_data_direction dir,
-			      unsigned long attrs)
+void ipu6_dma_unmap_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		       int nents, enum dma_data_direction dir,
+		       unsigned long attrs)
 {
-	struct pci_dev *pdev = to_ipu6_bus_device(dev)->isp->pdev;
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
+	struct device *dev = &sys->auxdev.dev;
+	struct ipu6_mmu *mmu = sys->mmu;
 	struct iova *iova = find_iova(&mmu->dmap->iovad,
 				      PHYS_PFN(sg_dma_address(sglist)));
-	int i, npages, count;
 	struct scatterlist *sg;
 	dma_addr_t pci_dma_addr;
+	unsigned int i;
 
 	if (!nents)
 		return;
@@ -342,31 +345,15 @@ static void ipu6_dma_unmap_sg(struct device *dev,
 	if (WARN_ON(!iova))
 		return;
 
-	if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
-		ipu6_dma_sync_sg_for_cpu(dev, sglist, nents, DMA_BIDIRECTIONAL);
-
-	/* get the nents as orig_nents given by caller */
-	count = 0;
-	npages = iova_size(iova);
-	for_each_sg(sglist, sg, nents, i) {
-		if (sg_dma_len(sg) == 0 ||
-		    sg_dma_address(sg) == DMA_MAPPING_ERROR)
-			break;
-
-		npages -= PHYS_PFN(PAGE_ALIGN(sg_dma_len(sg)));
-		count++;
-		if (npages <= 0)
-			break;
-	}
-
 	/*
 	 * Before IPU6 mmu unmap, return the pci dma address back to sg
 	 * assume the nents is less than orig_nents as the least granule
 	 * is 1 SZ_4K page
 	 */
-	dev_dbg(dev, "trying to unmap concatenated %u ents\n", count);
-	for_each_sg(sglist, sg, count, i) {
-		dev_dbg(dev, "ipu unmap sg[%d] %pad\n", i, &sg_dma_address(sg));
+	dev_dbg(dev, "trying to unmap concatenated %u ents\n", nents);
+	for_each_sg(sglist, sg, nents, i) {
+		dev_dbg(dev, "unmap sg[%d] %pad size %u\n", i,
+			&sg_dma_address(sg), sg_dma_len(sg));
 		pci_dma_addr = ipu6_mmu_iova_to_phys(mmu->dmap->mmu_info,
 						     sg_dma_address(sg));
 		dev_dbg(dev, "return pci_dma_addr %pad back to sg[%d]\n",
@@ -380,23 +367,21 @@ static void ipu6_dma_unmap_sg(struct device *dev,
 		       PFN_PHYS(iova_size(iova)));
 
 	mmu->tlb_invalidate(mmu);
-
-	dma_unmap_sg_attrs(&pdev->dev, sglist, nents, dir, attrs);
-
 	__free_iova(&mmu->dmap->iovad, iova);
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sg, INTEL_IPU6);
 
-static int ipu6_dma_map_sg(struct device *dev, struct scatterlist *sglist,
-			   int nents, enum dma_data_direction dir,
-			   unsigned long attrs)
+int ipu6_dma_map_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		    int nents, enum dma_data_direction dir,
+		    unsigned long attrs)
 {
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
-	struct pci_dev *pdev = to_ipu6_bus_device(dev)->isp->pdev;
+	struct device *dev = &sys->auxdev.dev;
+	struct ipu6_mmu *mmu = sys->mmu;
 	struct scatterlist *sg;
 	struct iova *iova;
 	size_t npages = 0;
 	unsigned long iova_addr;
-	int i, count;
+	int i;
 
 	for_each_sg(sglist, sg, nents, i) {
 		if (sg->offset) {
@@ -406,18 +391,12 @@ static int ipu6_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 		}
 	}
 
-	dev_dbg(dev, "pci_dma_map_sg trying to map %d ents\n", nents);
-	count  = dma_map_sg_attrs(&pdev->dev, sglist, nents, dir, attrs);
-	if (count <= 0) {
-		dev_err(dev, "pci_dma_map_sg %d ents failed\n", nents);
-		return 0;
-	}
-
-	dev_dbg(dev, "pci_dma_map_sg %d ents mapped\n", count);
-
-	for_each_sg(sglist, sg, count, i)
+	for_each_sg(sglist, sg, nents, i)
 		npages += PHYS_PFN(PAGE_ALIGN(sg_dma_len(sg)));
 
+	dev_dbg(dev, "dmamap trying to map %d ents %zu pages\n",
+		nents, npages);
+
 	iova = alloc_iova(&mmu->dmap->iovad, npages,
 			  PHYS_PFN(dma_get_mask(dev)), 0);
 	if (!iova)
@@ -427,7 +406,7 @@ static int ipu6_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 		iova->pfn_hi);
 
 	iova_addr = iova->pfn_lo;
-	for_each_sg(sglist, sg, count, i) {
+	for_each_sg(sglist, sg, nents, i) {
 		int ret;
 
 		dev_dbg(dev, "mapping entry %d: iova 0x%llx phy %pad size %d\n",
@@ -445,25 +424,48 @@ static int ipu6_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 		iova_addr += PHYS_PFN(PAGE_ALIGN(sg_dma_len(sg)));
 	}
 
-	if ((attrs & DMA_ATTR_SKIP_CPU_SYNC) == 0)
-		ipu6_dma_sync_sg_for_cpu(dev, sglist, nents, DMA_BIDIRECTIONAL);
+	dev_dbg(dev, "dmamap %d ents %zu pages mapped\n", nents, npages);
 
-	return count;
+	return nents;
 
 out_fail:
-	ipu6_dma_unmap_sg(dev, sglist, i, dir, attrs);
+	ipu6_dma_unmap_sg(sys, sglist, i, dir, attrs);
 
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_map_sg, INTEL_IPU6);
+
+int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			 enum dma_data_direction dir, unsigned long attrs)
+{
+	int nents;
+
+	nents = ipu6_dma_map_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
+	if (nents < 0)
+		return nents;
+
+	sgt->nents = nents;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_map_sgtable, INTEL_IPU6);
+
+void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			    enum dma_data_direction dir, unsigned long attrs)
+{
+	ipu6_dma_unmap_sg(sys, sgt->sgl, sgt->nents, dir, attrs);
+}
+EXPORT_SYMBOL_NS_GPL(ipu6_dma_unmap_sgtable, INTEL_IPU6);
 
 /*
  * Create scatter-list for the already allocated DMA buffer
  */
-static int ipu6_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
-				void *cpu_addr, dma_addr_t handle, size_t size,
-				unsigned long attrs)
+int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			 void *cpu_addr, dma_addr_t handle, size_t size,
+			 unsigned long attrs)
 {
-	struct ipu6_mmu *mmu = to_ipu6_bus_device(dev)->mmu;
+	struct device *dev = &sys->auxdev.dev;
+	struct ipu6_mmu *mmu = sys->mmu;
 	struct vm_info *info;
 	int n_pages;
 	int ret = 0;
@@ -483,20 +485,7 @@ static int ipu6_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
 	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
 					GFP_KERNEL);
 	if (ret)
-		dev_warn(dev, "IPU6 get sgt table failed\n");
+		dev_warn(dev, "get sgt table failed\n");
 
 	return ret;
 }
-
-const struct dma_map_ops ipu6_dma_ops = {
-	.alloc = ipu6_dma_alloc,
-	.free = ipu6_dma_free,
-	.mmap = ipu6_dma_mmap,
-	.map_sg = ipu6_dma_map_sg,
-	.unmap_sg = ipu6_dma_unmap_sg,
-	.sync_single_for_cpu = ipu6_dma_sync_single_for_cpu,
-	.sync_single_for_device = ipu6_dma_sync_single_for_cpu,
-	.sync_sg_for_cpu = ipu6_dma_sync_sg_for_cpu,
-	.sync_sg_for_device = ipu6_dma_sync_sg_for_cpu,
-	.get_sgtable = ipu6_dma_get_sgtable,
-};
diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.h b/drivers/media/pci/intel/ipu6/ipu6-dma.h
index 847ea5b7c925..b51244add9e6 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.h
@@ -5,7 +5,13 @@
 #define IPU6_DMA_H
 
 #include <linux/dma-map-ops.h>
+#include <linux/dma-mapping.h>
 #include <linux/iova.h>
+#include <linux/iova.h>
+#include <linux/scatterlist.h>
+#include <linux/types.h>
+
+#include "ipu6-bus.h"
 
 struct ipu6_mmu_info;
 
@@ -14,6 +20,30 @@ struct ipu6_dma_mapping {
 	struct iova_domain iovad;
 };
 
-extern const struct dma_map_ops ipu6_dma_ops;
-
+void ipu6_dma_sync_single(struct ipu6_bus_device *sys, dma_addr_t dma_handle,
+			  size_t size);
+void ipu6_dma_sync_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		      int nents);
+void ipu6_dma_sync_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt);
+void *ipu6_dma_alloc(struct ipu6_bus_device *sys, size_t size,
+		     dma_addr_t *dma_handle, gfp_t gfp,
+		     unsigned long attrs);
+void ipu6_dma_free(struct ipu6_bus_device *sys, size_t size, void *vaddr,
+		   dma_addr_t dma_handle, unsigned long attrs);
+int ipu6_dma_mmap(struct ipu6_bus_device *sys, struct vm_area_struct *vma,
+		  void *addr, dma_addr_t iova, size_t size,
+		  unsigned long attrs);
+int ipu6_dma_map_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		    int nents, enum dma_data_direction dir,
+		    unsigned long attrs);
+void ipu6_dma_unmap_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
+		       int nents, enum dma_data_direction dir,
+		       unsigned long attrs);
+int ipu6_dma_map_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			 enum dma_data_direction dir, unsigned long attrs);
+void ipu6_dma_unmap_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			    enum dma_data_direction dir, unsigned long attrs);
+int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
+			 void *cpu_addr, dma_addr_t handle, size_t size,
+			 unsigned long attrs);
 #endif /* IPU6_DMA_H */
diff --git a/drivers/media/pci/intel/ipu6/ipu6-fw-com.c b/drivers/media/pci/intel/ipu6/ipu6-fw-com.c
index 0b33fe9e703d..7d3d9314cb30 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-fw-com.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-fw-com.c
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 
 #include "ipu6-bus.h"
+#include "ipu6-dma.h"
 #include "ipu6-fw-com.h"
 
 /*
@@ -88,7 +89,6 @@ struct ipu6_fw_com_context {
 	void *dma_buffer;
 	dma_addr_t dma_addr;
 	unsigned int dma_size;
-	unsigned long attrs;
 
 	struct ipu6_fw_sys_queue *input_queue;	/* array of host to SP queues */
 	struct ipu6_fw_sys_queue *output_queue;	/* array of SP to host */
@@ -164,7 +164,6 @@ void *ipu6_fw_com_prepare(struct ipu6_fw_com_cfg *cfg,
 	struct ipu6_fw_com_context *ctx;
 	struct device *dev = &adev->auxdev.dev;
 	size_t sizeall, offset;
-	unsigned long attrs = 0;
 	void *specific_host_addr;
 	unsigned int i;
 
@@ -206,9 +205,8 @@ void *ipu6_fw_com_prepare(struct ipu6_fw_com_cfg *cfg,
 
 	sizeall += sizeinput + sizeoutput;
 
-	ctx->dma_buffer = dma_alloc_attrs(dev, sizeall, &ctx->dma_addr,
-					  GFP_KERNEL, attrs);
-	ctx->attrs = attrs;
+	ctx->dma_buffer = ipu6_dma_alloc(adev, sizeall, &ctx->dma_addr,
+					 GFP_KERNEL, 0);
 	if (!ctx->dma_buffer) {
 		dev_err(dev, "failed to allocate dma memory\n");
 		kfree(ctx);
@@ -239,6 +237,8 @@ void *ipu6_fw_com_prepare(struct ipu6_fw_com_cfg *cfg,
 		memcpy(specific_host_addr, cfg->specific_addr,
 		       cfg->specific_size);
 
+	ipu6_dma_sync_single(adev, ctx->config_vied_addr, sizeall);
+
 	/* initialize input queues */
 	offset += specific_size;
 	res.reg = SYSCOM_QPR_BASE_REG;
@@ -315,8 +315,8 @@ int ipu6_fw_com_release(struct ipu6_fw_com_context *ctx, unsigned int force)
 	if (!force && !ctx->cell_ready(ctx->adev))
 		return -EBUSY;
 
-	dma_free_attrs(&ctx->adev->auxdev.dev, ctx->dma_size,
-		       ctx->dma_buffer, ctx->dma_addr, ctx->attrs);
+	ipu6_dma_free(ctx->adev, ctx->dma_size,
+		      ctx->dma_buffer, ctx->dma_addr, 0);
 	kfree(ctx);
 	return 0;
 }
-- 
2.34.1

[PATCH 2/5] media: ipu6: use the IPU6 DMA mapping APIs to do mapping

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

dma_ops is removed from the IPU6 auxiliary device, ISYS driver
should use the IPU6 DMA mapping APIs directly instead of depending
on the device callbacks.

ISYS driver switch from the videobuf2 DMA contig memory allocator to
scatter/gather memory allocator.

Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/pci/intel/ipu6/Kconfig          |  2 +-
 .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 66 +++++++++++++++----
 .../media/pci/intel/ipu6/ipu6-isys-queue.h    |  1 +
 drivers/media/pci/intel/ipu6/ipu6-isys.c      | 19 +++---
 4 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig
index 49e4fb696573..ec6c1ea329ed 100644
--- a/drivers/media/pci/intel/ipu6/Kconfig
+++ b/drivers/media/pci/intel/ipu6/Kconfig
@@ -14,7 +14,7 @@ config VIDEO_INTEL_IPU6
 	select IOMMU_IOVA
 	select VIDEO_V4L2_SUBDEV_API
 	select MEDIA_CONTROLLER
-	select VIDEOBUF2_DMA_CONTIG
+	select VIDEOBUF2_DMA_SG
 	select V4L2_FWNODE
 	help
 	  This is the 6th Gen Intel Image Processing Unit, found in Intel SoCs
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 03dbb0e0ea79..bbb66b56ee88 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -13,17 +13,48 @@
 
 #include <media/media-entity.h>
 #include <media/v4l2-subdev.h>
-#include <media/videobuf2-dma-contig.h>
+#include <media/videobuf2-dma-sg.h>
 #include <media/videobuf2-v4l2.h>
 
 #include "ipu6-bus.h"
+#include "ipu6-dma.h"
 #include "ipu6-fw-isys.h"
 #include "ipu6-isys.h"
 #include "ipu6-isys-video.h"
 
-static int queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
-		       unsigned int *num_planes, unsigned int sizes[],
-		       struct device *alloc_devs[])
+static int ipu6_isys_buf_init(struct vb2_buffer *vb)
+{
+	struct ipu6_isys *isys = vb2_get_drv_priv(vb->vb2_queue);
+	struct sg_table *sg = vb2_dma_sg_plane_desc(vb, 0);
+	struct vb2_v4l2_buffer *vvb = to_vb2_v4l2_buffer(vb);
+	struct ipu6_isys_video_buffer *ivb =
+		vb2_buffer_to_ipu6_isys_video_buffer(vvb);
+	int ret;
+
+	ret = ipu6_dma_map_sgtable(isys->adev, sg, DMA_TO_DEVICE, 0);
+	if (ret)
+		return ret;
+
+	ivb->dma_addr = sg_dma_address(sg->sgl);
+
+	return 0;
+}
+
+static void ipu6_isys_buf_cleanup(struct vb2_buffer *vb)
+{
+	struct ipu6_isys *isys = vb2_get_drv_priv(vb->vb2_queue);
+	struct sg_table *sg = vb2_dma_sg_plane_desc(vb, 0);
+	struct vb2_v4l2_buffer *vvb = to_vb2_v4l2_buffer(vb);
+	struct ipu6_isys_video_buffer *ivb =
+		vb2_buffer_to_ipu6_isys_video_buffer(vvb);
+
+	ivb->dma_addr = 0;
+	ipu6_dma_unmap_sgtable(isys->adev, sg, DMA_TO_DEVICE, 0);
+}
+
+static int ipu6_isys_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
+				 unsigned int *num_planes, unsigned int sizes[],
+				 struct device *alloc_devs[])
 {
 	struct ipu6_isys_queue *aq = vb2_queue_to_isys_queue(q);
 	struct ipu6_isys_video *av = ipu6_isys_queue_to_video(aq);
@@ -207,9 +238,11 @@ ipu6_isys_buf_to_fw_frame_buf_pin(struct vb2_buffer *vb,
 				  struct ipu6_fw_isys_frame_buff_set_abi *set)
 {
 	struct ipu6_isys_queue *aq = vb2_queue_to_isys_queue(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vvb = to_vb2_v4l2_buffer(vb);
+	struct ipu6_isys_video_buffer *ivb =
+		vb2_buffer_to_ipu6_isys_video_buffer(vvb);
 
-	set->output_pins[aq->fw_output].addr =
-		vb2_dma_contig_plane_dma_addr(vb, 0);
+	set->output_pins[aq->fw_output].addr = ivb->dma_addr;
 	set->output_pins[aq->fw_output].out_buf_id = vb->index + 1;
 }
 
@@ -332,7 +365,7 @@ static void buf_queue(struct vb2_buffer *vb)
 
 	dev_dbg(dev, "queue buffer %u for %s\n", vb->index, av->vdev.name);
 
-	dma = vb2_dma_contig_plane_dma_addr(vb, 0);
+	dma = ivb->dma_addr;
 	dev_dbg(dev, "iova: iova %pad\n", &dma);
 
 	spin_lock_irqsave(&aq->lock, flags);
@@ -724,10 +757,14 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 	}
 
 	list_for_each_entry_reverse(ib, &aq->active, head) {
+		struct ipu6_isys_video_buffer *ivb;
+		struct vb2_v4l2_buffer *vvb;
 		dma_addr_t addr;
 
 		vb = ipu6_isys_buffer_to_vb2_buffer(ib);
-		addr = vb2_dma_contig_plane_dma_addr(vb, 0);
+		vvb = to_vb2_v4l2_buffer(vb);
+		ivb = vb2_buffer_to_ipu6_isys_video_buffer(vvb);
+		addr = ivb->dma_addr;
 
 		if (info->pin.addr != addr) {
 			if (first)
@@ -766,10 +803,12 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 }
 
 static const struct vb2_ops ipu6_isys_queue_ops = {
-	.queue_setup = queue_setup,
+	.queue_setup = ipu6_isys_queue_setup,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
+	.buf_init = ipu6_isys_buf_init,
 	.buf_prepare = ipu6_isys_buf_prepare,
+	.buf_cleanup = ipu6_isys_buf_cleanup,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,
 	.buf_queue = buf_queue,
@@ -779,16 +818,17 @@ int ipu6_isys_queue_init(struct ipu6_isys_queue *aq)
 {
 	struct ipu6_isys *isys = ipu6_isys_queue_to_video(aq)->isys;
 	struct ipu6_isys_video *av = ipu6_isys_queue_to_video(aq);
+	struct ipu6_bus_device *adev = isys->adev;
 	int ret;
 
 	/* no support for userptr */
 	if (!aq->vbq.io_modes)
 		aq->vbq.io_modes = VB2_MMAP | VB2_DMABUF;
 
-	aq->vbq.drv_priv = aq;
+	aq->vbq.drv_priv = isys;
 	aq->vbq.ops = &ipu6_isys_queue_ops;
 	aq->vbq.lock = &av->mutex;
-	aq->vbq.mem_ops = &vb2_dma_contig_memops;
+	aq->vbq.mem_ops = &vb2_dma_sg_memops;
 	aq->vbq.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	aq->vbq.min_queued_buffers = 1;
 	aq->vbq.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
@@ -797,8 +837,8 @@ int ipu6_isys_queue_init(struct ipu6_isys_queue *aq)
 	if (ret)
 		return ret;
 
-	aq->dev = &isys->adev->auxdev.dev;
-	aq->vbq.dev = &isys->adev->auxdev.dev;
+	aq->dev = &adev->auxdev.dev;
+	aq->vbq.dev = &adev->isp->pdev->dev;
 	spin_lock_init(&aq->lock);
 	INIT_LIST_HEAD(&aq->active);
 	INIT_LIST_HEAD(&aq->incoming);
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
index 95cfd4869d93..fe8fc796a58f 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
@@ -38,6 +38,7 @@ struct ipu6_isys_buffer {
 struct ipu6_isys_video_buffer {
 	struct vb2_v4l2_buffer vb_v4l2;
 	struct ipu6_isys_buffer ib;
+	dma_addr_t dma_addr;
 };
 
 #define IPU6_ISYS_BUFFER_LIST_FL_INCOMING	BIT(0)
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys.c b/drivers/media/pci/intel/ipu6/ipu6-isys.c
index c4aff2e2009b..c85e056cb904 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys.c
@@ -34,6 +34,7 @@
 
 #include "ipu6-bus.h"
 #include "ipu6-cpd.h"
+#include "ipu6-dma.h"
 #include "ipu6-isys.h"
 #include "ipu6-isys-csi2.h"
 #include "ipu6-mmu.h"
@@ -933,29 +934,27 @@ static const struct dev_pm_ops isys_pm_ops = {
 
 static void free_fw_msg_bufs(struct ipu6_isys *isys)
 {
-	struct device *dev = &isys->adev->auxdev.dev;
 	struct isys_fw_msgs *fwmsg, *safe;
 
 	list_for_each_entry_safe(fwmsg, safe, &isys->framebuflist, head)
-		dma_free_attrs(dev, sizeof(struct isys_fw_msgs), fwmsg,
-			       fwmsg->dma_addr, 0);
+		ipu6_dma_free(isys->adev, sizeof(struct isys_fw_msgs), fwmsg,
+			      fwmsg->dma_addr, 0);
 
 	list_for_each_entry_safe(fwmsg, safe, &isys->framebuflist_fw, head)
-		dma_free_attrs(dev, sizeof(struct isys_fw_msgs), fwmsg,
-			       fwmsg->dma_addr, 0);
+		ipu6_dma_free(isys->adev, sizeof(struct isys_fw_msgs), fwmsg,
+			      fwmsg->dma_addr, 0);
 }
 
 static int alloc_fw_msg_bufs(struct ipu6_isys *isys, int amount)
 {
-	struct device *dev = &isys->adev->auxdev.dev;
 	struct isys_fw_msgs *addr;
 	dma_addr_t dma_addr;
 	unsigned long flags;
 	unsigned int i;
 
 	for (i = 0; i < amount; i++) {
-		addr = dma_alloc_attrs(dev, sizeof(struct isys_fw_msgs),
-				       &dma_addr, GFP_KERNEL, 0);
+		addr = ipu6_dma_alloc(isys->adev, sizeof(*addr),
+				      &dma_addr, GFP_KERNEL, 0);
 		if (!addr)
 			break;
 		addr->dma_addr = dma_addr;
@@ -974,8 +973,8 @@ static int alloc_fw_msg_bufs(struct ipu6_isys *isys, int amount)
 					struct isys_fw_msgs, head);
 		list_del(&addr->head);
 		spin_unlock_irqrestore(&isys->listlock, flags);
-		dma_free_attrs(dev, sizeof(struct isys_fw_msgs), addr,
-			       addr->dma_addr, 0);
+		ipu6_dma_free(isys->adev, sizeof(struct isys_fw_msgs), addr,
+			      addr->dma_addr, 0);
 		spin_lock_irqsave(&isys->listlock, flags);
 	}
 	spin_unlock_irqrestore(&isys->listlock, flags);
-- 
2.34.1

[PATCH 3/5] media: ipu6: remove architecture DMA ops dependency in Kconfig

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

IPU6 driver doesn't override the dma_ops of device now, it doesn't
depends on the ARCH_HAS_DMA_OPS, so remove the dependency in Kconfig.

Fixes: de6c85bf918e ("dma-mapping: clearly mark DMA ops as an architecture feature")
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/pci/intel/ipu6/Kconfig | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/Kconfig b/drivers/media/pci/intel/ipu6/Kconfig
index ec6c1ea329ed..cd1c54529357 100644
--- a/drivers/media/pci/intel/ipu6/Kconfig
+++ b/drivers/media/pci/intel/ipu6/Kconfig
@@ -4,12 +4,6 @@ config VIDEO_INTEL_IPU6
 	depends on VIDEO_DEV
 	depends on X86 && X86_64 && HAS_DMA
 	depends on IPU_BRIDGE || !IPU_BRIDGE
-	#
-	# This driver incorrectly tries to override the dma_ops.  It should
-	# never have done that, but for now keep it working on architectures
-	# that use dma ops
-	#
-	depends on ARCH_HAS_DMA_OPS
 	select AUXILIARY_BUS
 	select IOMMU_IOVA
 	select VIDEO_V4L2_SUBDEV_API
-- 
2.34.1

[PATCH 4/5] Documentation: ipu6: remove the dma_ops part from the doc

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

dma_ops override is not in code anymore, so remove it.

Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 Documentation/driver-api/media/drivers/ipu6.rst | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/driver-api/media/drivers/ipu6.rst b/Documentation/driver-api/media/drivers/ipu6.rst
index 6e1dd19b36fb..88f6498e74db 100644
--- a/Documentation/driver-api/media/drivers/ipu6.rst
+++ b/Documentation/driver-api/media/drivers/ipu6.rst
@@ -98,21 +98,6 @@ The IPU6 driver exports its own DMA operations. The IPU6 driver will update the
 page table entries for each DMA operation and invalidate the MMU TLB after each
 unmap and free.
 
-.. code-block:: none
-
-    const struct dma_map_ops ipu6_dma_ops = {
-	   .alloc = ipu6_dma_alloc,
-	   .free = ipu6_dma_free,
-	   .mmap = ipu6_dma_mmap,
-	   .map_sg = ipu6_dma_map_sg,
-	   .unmap_sg = ipu6_dma_unmap_sg,
-	   ...
-    };
-
-.. Note:: IPU6 MMU works behind IOMMU so for each IPU6 DMA ops, driver will call
-	  generic PCI DMA ops to ask IOMMU to do the additional mapping if VT-d
-	  enabled.
-
 Firmware file format
 ====================
 
-- 
2.34.1

[PATCH 5/5] media: ipu6: use PFN_UP() and sg_virt() for code simplicity

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

Use PFN_UP() and sg_virt() can be used to simplify the code.

Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-buttress.c | 2 +-
 drivers/media/pci/intel/ipu6/ipu6-dma.c      | 8 ++++----
 drivers/media/pci/intel/ipu6/ipu6-mmu.c      | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index d66db537be4a..2cb828c87961 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -564,7 +564,7 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
 	if (!is_vmalloc && !virt_addr_valid(fw->data))
 		return -EDOM;
 
-	n_pages = PHYS_PFN(PAGE_ALIGN(fw->size));
+	n_pages = PFN_UP(fw->size);
 
 	pages = kmalloc_array(n_pages, sizeof(*pages), GFP_KERNEL);
 	if (!pages)
diff --git a/drivers/media/pci/intel/ipu6/ipu6-dma.c b/drivers/media/pci/intel/ipu6/ipu6-dma.c
index d03803e58881..207f67cd2b0c 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-dma.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
@@ -139,7 +139,7 @@ void ipu6_dma_sync_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
 	int i;
 
 	for_each_sg(sglist, sg, nents, i)
-		clflush_cache_range(page_to_virt(sg_page(sg)), sg->length);
+		clflush_cache_range(sg_virt(sg), sg->length);
 }
 EXPORT_SYMBOL_NS_GPL(ipu6_dma_sync_sg, INTEL_IPU6);
 
@@ -392,7 +392,7 @@ int ipu6_dma_map_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
 	}
 
 	for_each_sg(sglist, sg, nents, i)
-		npages += PHYS_PFN(PAGE_ALIGN(sg_dma_len(sg)));
+		npages += PFN_UP(sg_dma_len(sg));
 
 	dev_dbg(dev, "dmamap trying to map %d ents %zu pages\n",
 		nents, npages);
@@ -421,7 +421,7 @@ int ipu6_dma_map_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
 
 		sg_dma_address(sg) = PFN_PHYS(iova_addr);
 
-		iova_addr += PHYS_PFN(PAGE_ALIGN(sg_dma_len(sg)));
+		iova_addr += PFN_UP(sg_dma_len(sg));
 	}
 
 	dev_dbg(dev, "dmamap %d ents %zu pages mapped\n", nents, npages);
@@ -480,7 +480,7 @@ int ipu6_dma_get_sgtable(struct ipu6_bus_device *sys, struct sg_table *sgt,
 	if (WARN_ON(!info->pages))
 		return -ENOMEM;
 
-	n_pages = PHYS_PFN(PAGE_ALIGN(size));
+	n_pages = PFN_UP(size);
 
 	ret = sg_alloc_table_from_pages(sgt, info->pages, n_pages, 0, size,
 					GFP_KERNEL);
diff --git a/drivers/media/pci/intel/ipu6/ipu6-mmu.c b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
index 2d9c6fbd5da2..07935350bb0e 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-mmu.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-mmu.c
@@ -409,7 +409,7 @@ static void __ipu6_mmu_unmap(struct ipu6_mmu_info *mmu_info,
 
 static int allocate_trash_buffer(struct ipu6_mmu *mmu)
 {
-	unsigned int n_pages = PHYS_PFN(PAGE_ALIGN(IPU6_MMUV2_TRASH_RANGE));
+	unsigned int n_pages = PFN_UP(IPU6_MMUV2_TRASH_RANGE);
 	struct iova *iova;
 	unsigned int i;
 	dma_addr_t dma;
-- 
2.34.1

Re: [PATCH 1/5] media: ipu6: not override the dma_ops of device in driver

[Christoph Hellwig]

On Mon, Oct 14, 2024 at 11:47:28AM +0800, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> DMA ops are a helper for architectures and not for drivers to override
> the DMA implementation.  Driver should not override the DMA
> implementation. This patch remove the dma_ops override and expose the
> IPU6 DMA mapping APIs.

That's a good rationale, but it might make sense to mention what you're
actually changing here as well.

> +	ret = dma_map_sgtable(&pdev->dev, sgt, DMA_TO_DEVICE, 0);
> +	if (ret) {
> +		sg_free_table(sgt);
> +		goto out;
> +	}
> +
> +	ret = ipu6_dma_map_sgtable(sys, sgt, DMA_TO_DEVICE, 0);

This looks like the only user of ipu6_dma_map_sgtable, any reason
to not open code it here?

>  	kfree(pages);
> @@ -607,7 +615,10 @@ EXPORT_SYMBOL_NS_GPL(ipu6_buttress_map_fw_image, INTEL_IPU6);
>  void ipu6_buttress_unmap_fw_image(struct ipu6_bus_device *sys,
>  				  struct sg_table *sgt)
>  {
> -	dma_unmap_sgtable(&sys->auxdev.dev, sgt, DMA_TO_DEVICE, 0);
> +	struct pci_dev *pdev = sys->isp->pdev;
> +
> +	ipu6_dma_unmap_sgtable(sys, sgt, DMA_TO_DEVICE, 0);

Same for ipu6_dma_unmap_sgtable

Re: [PATCH 5/5] media: ipu6: use PFN_UP() and sg_virt() for code simplicity

[Christoph Hellwig]

On Mon, Oct 14, 2024 at 11:47:32AM +0800, bingbu.cao@intel.com wrote:
> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
> @@ -139,7 +139,7 @@ void ipu6_dma_sync_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
>  	int i;
>  
>  	for_each_sg(sglist, sg, nents, i)
> -		clflush_cache_range(page_to_virt(sg_page(sg)), sg->length);
> +		clflush_cache_range(sg_virt(sg), sg->length);

Not new in this code, but what guarantees that this driver never sees
highmem?

Re: [PATCH 1/5] media: ipu6: not override the dma_ops of device in driver

[Bingbu Cao]

Christoph,

Thanks for the review comments.

On 10/14/24 1:53 PM, Christoph Hellwig wrote:
> On Mon, Oct 14, 2024 at 11:47:28AM +0800, bingbu.cao@intel.com wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> DMA ops are a helper for architectures and not for drivers to override
>> the DMA implementation.  Driver should not override the DMA
>> implementation. This patch remove the dma_ops override and expose the
>> IPU6 DMA mapping APIs.
> 
> That's a good rationale, but it might make sense to mention what you're
> actually changing here as well.

Ack. I will reword the message.

> 
>> +	ret = dma_map_sgtable(&pdev->dev, sgt, DMA_TO_DEVICE, 0);
>> +	if (ret) {
>> +		sg_free_table(sgt);
>> +		goto out;
>> +	}
>> +
>> +	ret = ipu6_dma_map_sgtable(sys, sgt, DMA_TO_DEVICE, 0);
> 
> This looks like the only user of ipu6_dma_map_sgtable, any reason
> to not open code it here?

The Intel IPU6 driver contains 2 modules, this symbol is defined in
intel_ipu6 module. The map/unmap_sgtable are also used by another
driver - intel_ipu6_isys.

> 
>>  	kfree(pages);
>> @@ -607,7 +615,10 @@ EXPORT_SYMBOL_NS_GPL(ipu6_buttress_map_fw_image, INTEL_IPU6);
>>  void ipu6_buttress_unmap_fw_image(struct ipu6_bus_device *sys,
>>  				  struct sg_table *sgt)
>>  {
>> -	dma_unmap_sgtable(&sys->auxdev.dev, sgt, DMA_TO_DEVICE, 0);
>> +	struct pci_dev *pdev = sys->isp->pdev;
>> +
>> +	ipu6_dma_unmap_sgtable(sys, sgt, DMA_TO_DEVICE, 0);
> 
> Same for ipu6_dma_unmap_sgtable
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH 5/5] media: ipu6: use PFN_UP() and sg_virt() for code simplicity

[Bingbu Cao]

Christoph,

On 10/14/24 1:54 PM, Christoph Hellwig wrote:
> On Mon, Oct 14, 2024 at 11:47:32AM +0800, bingbu.cao@intel.com wrote:
>> +++ b/drivers/media/pci/intel/ipu6/ipu6-dma.c
>> @@ -139,7 +139,7 @@ void ipu6_dma_sync_sg(struct ipu6_bus_device *sys, struct scatterlist *sglist,
>>  	int i;
>>  
>>  	for_each_sg(sglist, sg, nents, i)
>> -		clflush_cache_range(page_to_virt(sg_page(sg)), sg->length);
>> +		clflush_cache_range(sg_virt(sg), sg->length);
> 
> Not new in this code, but what guarantees that this driver never sees
> highmem?
> 
>

The hardware is only in Intel 64-bit SoCs. And it is likely the driver
working with 64-bit kernel. I am not sure driver should handle highmem.

Any suggestions?

-- 
Best regards,
Bingbu Cao

Re: [PATCH 5/5] media: ipu6: use PFN_UP() and sg_virt() for code simplicity

[Christoph Hellwig]

On Mon, Oct 14, 2024 at 06:31:35PM +0800, Bingbu Cao wrote:
> > Not new in this code, but what guarantees that this driver never sees
> > highmem?
> > 
> >
> 
> The hardware is only in Intel 64-bit SoCs. And it is likely the driver
> working with 64-bit kernel. I am not sure driver should handle highmem.

Yes, that should be fine. Unless people bring back the unmapping
user memory from the direct map thing.