Re: [PATCH v2 40/44] media: ipu6: Map ipu7 firmware

Moi,

On Fri, Aug 21, 2026 at 02:42:58PM +0300, Antti Laakso wrote:
> Mapping of ipu7 firmware is slightly different compared to ipu6. Add
> helpers for it and call them similar to ipu6.
> 
> Signed-off-by: Antti Laakso <[email protected]>
> ---
>  drivers/media/pci/intel/ipu6/ipu6.c | 75 ++++++++++++++++++++++++++---
>  drivers/media/pci/intel/ipu6/ipu6.h |  1 +
>  2 files changed, 70 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6.c b/drivers/media/pci/intel/ipu6/ipu6.c
> index d27ffd7ac6f0..93f28ec9a74d 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6.c
> @@ -19,6 +19,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/vmalloc.h>
>  
>  #include <media/ipu-bridge.h>
>  #include <media/ipu6-pci-table.h>
> @@ -27,6 +28,7 @@
>  #include "ipu6-bus.h"
>  #include "ipu6-buttress.h"
>  #include "ipu6-cpd.h"
> +#include "ipu6-dma.h"
>  #include "ipu6-isys.h"
>  #include "ipu6-mmu.h"
>  #include "ipu6-platform-buttress-regs.h"
> @@ -514,6 +516,57 @@ static int ipu6_map_fw(struct ipu6_device *isp)
>  	return 0;
>  }
>  
> +static int __ipu7_map_fw_non_secure(struct ipu6_device *isp)
> +{
> +	int ret;
> +
> +	/*
> +	 * Allocate and map memory for running the firmware. Not
> +	 * required in secure mode, in which firmware runs in IMR.
> +	 */
> +	isp->fw_code_region = vmalloc(IPU7_FW_CODE_REGION_SIZE);
> +	if (!isp->fw_code_region)
> +		return -ENOMEM;
> +
> +	ret = ipu7_cpd_copy_binary(isp->cpd_fw->data, "isys",
> +				   isp->fw_code_region, &isp->isys->fw_entry);
> +	if (ret)
> +		return ret;
> +
> +	ret = ipu6_map_fw_region(isp->isys, isp->fw_code_region,
> +				 IPU7_FW_CODE_REGION_SIZE, DMA_BIDIRECTIONAL,
> +				 DMA_ATTR_RESERVE_REGION);
> +	if (ret)
> +		return ret;
> +
> +	ret = ipu7_cpd_copy_binary(isp->cpd_fw->data, "psys",
> +				   isp->fw_code_region, &isp->psys->fw_entry);
> +	if (ret)
> +		return ret;
> +
> +	return ipu6_map_fw_region(isp->psys, isp->fw_code_region,
> +				  IPU7_FW_CODE_REGION_SIZE, DMA_BIDIRECTIONAL,
> +				  DMA_ATTR_RESERVE_REGION);
> +}
> +
> +static int ipu7_map_fw(struct ipu6_device *isp)
> +{
> +	int ret;
> +
> +	ret = isp->secure_mode ?
> +		ipu6_map_fw_region(isp->psys, isp->cpd_fw->data,
> +				   isp->cpd_fw->size, DMA_BIDIRECTIONAL, 0) :
> +		__ipu7_map_fw_non_secure(isp);
> +

No newline needed here.

> +	if (ret) {
> +		dev_err_probe(&isp->pdev->dev, ret,
> +			      "Failed to init ipu7 firmware region\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
>  	const struct ipu6_buttress_ctrl *isys_ctrl, *psys_ctrl;
> @@ -523,6 +576,7 @@ static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	struct ipu6_device *isp;
>  	phys_addr_t phys;
>  	u32 val, version, sku_id;
> +	unsigned long dir;
>  	int ret;
>  
>  	isp = devm_kzalloc(dev, sizeof(*isp), GFP_KERNEL);
> @@ -658,7 +712,7 @@ static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		goto out_ipu6_rpm_put;
>  	}
>  
> -	ret = ipu6_map_fw(isp);
> +	ret = IS_IPU7(isp) ? ipu7_map_fw(isp) : ipu6_map_fw(isp);
>  	if (ret)
>  		goto out_ipu6_rpm_put;
>  
> @@ -701,15 +755,20 @@ static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  out_ipu6_rpm_put:
>  	pm_runtime_put_sync(&isp->psys->auxdev.dev);
>  out_ipu6_bus_del_devices:
> +	dir = IS_IPU7(isp) ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
>  	if (!IS_ERR_OR_NULL(isp->psys)) {
>  		ipu6_cpd_free_pkg_dir(isp->psys);
>  		if (isp->psys->fw_sgt.nents)
> -			ipu6_unmap_fw_region(isp->psys, DMA_TO_DEVICE);
> +			ipu6_unmap_fw_region(isp->psys, dir);
>  	}
>  	if (!IS_ERR_OR_NULL(isp->isys)) {
>  		ipu6_cpd_free_pkg_dir(isp->isys);
>  		if (isp->isys->fw_sgt.nents)
> -			ipu6_unmap_fw_region(isp->isys, DMA_TO_DEVICE);
> +			ipu6_unmap_fw_region(isp->isys, dir);
> +	}
> +	if (isp->fw_code_region) {
> +		vfree(isp->fw_code_region);
> +		isp->fw_code_region = NULL;

This can be done unconditionally -- as in the remove callback.

>  	}
>  	if (!IS_ERR_OR_NULL(isp->psys) && !IS_ERR_OR_NULL(isp->psys->mmu))
>  		ipu6_mmu_cleanup(isp->psys->mmu);
> @@ -728,18 +787,22 @@ static void ipu6_pci_remove(struct pci_dev *pdev)
>  	struct ipu6_device *isp = pci_get_drvdata(pdev);
>  	struct ipu6_mmu *isys_mmu = isp->isys->mmu;
>  	struct ipu6_mmu *psys_mmu = isp->psys->mmu;
> +	unsigned long dir;
>  
>  	devm_free_irq(&pdev->dev, pdev->irq, isp);
> -	ipu6_cpd_free_pkg_dir(isp->psys);
>  
> -	ipu6_unmap_fw_region(isp->psys, DMA_TO_DEVICE);
> +	dir = IS_IPU7(isp) ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE;

Why is there a need for a bi-directional mapping for ipu7?

> +	ipu6_cpd_free_pkg_dir(isp->psys);
> +	ipu6_unmap_fw_region(isp->psys, dir);
>  
>  	if (isp->isys) {
>  		ipu6_cpd_free_pkg_dir(isp->isys);
>  		if (isp->isys->fw_sgt.nents)
> -			ipu6_unmap_fw_region(isp->isys, DMA_TO_DEVICE);
> +			ipu6_unmap_fw_region(isp->isys, dir);
>  	}
>  
> +	vfree(isp->fw_code_region);
> +
>  	ipu6_buttress_exit(isp);
>  
>  	ipu6_bus_del_devices(pdev);
> diff --git a/drivers/media/pci/intel/ipu6/ipu6.h b/drivers/media/pci/intel/ipu6/ipu6.h
> index 6bef45a02c2b..a2992ffe8e3c 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6.h
> @@ -70,6 +70,7 @@ struct ipu6_device {
>  	bool secure_mode;
>  	u8 hw_ver;
>  	bool bus_ready_to_probe;
> +	u32 *fw_code_region;
>  };
>  
>  #define IPU_PSYS	0

-- 
Terveisin,

Sakari Ailus