mbox series

[0/5] Driver for Apple ISP and cameras.

Message ID 20250219-isp-v1-0-6d3e89b67c31@gmail.com (mailing list archive)
Headers show
Series Driver for Apple ISP and cameras. | expand

Message

Sasha Finkelstein via B4 Relay Feb. 19, 2025, 9:26 a.m. UTC
Hi.

This series adds support for the camera and ISP system present
on Apple devices using M-series chips. This is a "simple" camera
and does not need any special userspace handling, everything
is handled by the firmware running on an ASC coprocessor.

Patches 1 and 2 add support for special handling neccesary for the
ISP power domains. The rest add the driver itself.

Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
Asahi Lina (1):
      pmdomain: apple: Add force-disable/force-reset

Eileen Yoon (2):
      media: apple: Add Apple ISP driver
      arm64: dts: apple: Add ISP nodes

Sasha Finkelstein (2):
      dt-bindings: power: apple,pmgr-pwrstate: Add force-{disable,reset} properties
      media: dt-bindings: Add Apple ISP

 .../devicetree/bindings/media/apple,isp.yaml       | 151 ++++
 .../bindings/power/apple,pmgr-pwrstate.yaml        |  10 +
 MAINTAINERS                                        |   2 +
 arch/arm64/boot/dts/apple/isp-common.dtsi          |  45 ++
 arch/arm64/boot/dts/apple/isp-imx248.dtsi          |  62 ++
 arch/arm64/boot/dts/apple/isp-imx364.dtsi          |  78 ++
 arch/arm64/boot/dts/apple/isp-imx558-cfg0.dtsi     | 101 +++
 arch/arm64/boot/dts/apple/isp-imx558.dtsi          | 102 +++
 arch/arm64/boot/dts/apple/t600x-die0.dtsi          |  48 ++
 arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi     |   6 +
 arch/arm64/boot/dts/apple/t600x-pmgr.dtsi          |  81 ++
 arch/arm64/boot/dts/apple/t8103-j293.dts           |   6 +
 arch/arm64/boot/dts/apple/t8103-j313.dts           |   6 +
 arch/arm64/boot/dts/apple/t8103-j456.dts           |   6 +
 arch/arm64/boot/dts/apple/t8103-j457.dts           |   6 +
 arch/arm64/boot/dts/apple/t8103-pmgr.dtsi          | 118 +++
 arch/arm64/boot/dts/apple/t8103.dtsi               |  50 ++
 arch/arm64/boot/dts/apple/t8112-j413.dts           |   7 +
 arch/arm64/boot/dts/apple/t8112-j493.dts           |   6 +
 arch/arm64/boot/dts/apple/t8112-pmgr.dtsi          | 118 +++
 arch/arm64/boot/dts/apple/t8112.dtsi               |  50 ++
 drivers/media/platform/Kconfig                     |   1 +
 drivers/media/platform/Makefile                    |   1 +
 drivers/media/platform/apple/Kconfig               |   5 +
 drivers/media/platform/apple/Makefile              |   3 +
 drivers/media/platform/apple/isp/Kconfig           |  16 +
 drivers/media/platform/apple/isp/Makefile          |   3 +
 drivers/media/platform/apple/isp/isp-cam.c         | 414 ++++++++++
 drivers/media/platform/apple/isp/isp-cam.h         |  21 +
 drivers/media/platform/apple/isp/isp-cmd.c         | 635 +++++++++++++++
 drivers/media/platform/apple/isp/isp-cmd.h         | 692 ++++++++++++++++
 drivers/media/platform/apple/isp/isp-drv.c         | 586 ++++++++++++++
 drivers/media/platform/apple/isp/isp-drv.h         | 284 +++++++
 drivers/media/platform/apple/isp/isp-fw.c          | 770 ++++++++++++++++++
 drivers/media/platform/apple/isp/isp-fw.h          |  24 +
 drivers/media/platform/apple/isp/isp-iommu.c       | 251 ++++++
 drivers/media/platform/apple/isp/isp-iommu.h       |  20 +
 drivers/media/platform/apple/isp/isp-ipc.c         | 258 ++++++
 drivers/media/platform/apple/isp/isp-ipc.h         |  25 +
 drivers/media/platform/apple/isp/isp-regs.h        |  56 ++
 drivers/media/platform/apple/isp/isp-v4l2.c        | 900 +++++++++++++++++++++
 drivers/media/platform/apple/isp/isp-v4l2.h        |  16 +
 drivers/pmdomain/apple/pmgr-pwrstate.c             |  43 +-
 43 files changed, 6077 insertions(+), 6 deletions(-)
---
base-commit: 2408a807bfc3f738850ef5ad5e3fd59d66168996
change-id: 20250218-isp-5da1dbc7d472

Comments

Janne Grunau Feb. 19, 2025, 11:34 a.m. UTC | #1
Hej,

On Wed, Feb 19, 2025 at 10:27:00AM +0100, Sasha Finkelstein via B4 Relay wrote:
> From: Eileen Yoon <eyn@gmx.com>
> 
> This is the ISP and camera module present on certain Apple laptops
> 
> Signed-off-by: Eileen Yoon <eyn@gmx.com>
> Co-developed-by: Hector Martin <marcan@marcan.st>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Co-developed-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Co-developed-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
> ---
>  MAINTAINERS                                  |   1 +
>  drivers/media/platform/Kconfig               |   1 +
>  drivers/media/platform/Makefile              |   1 +
>  drivers/media/platform/apple/Kconfig         |   5 +
>  drivers/media/platform/apple/Makefile        |   3 +
>  drivers/media/platform/apple/isp/Kconfig     |  16 +
>  drivers/media/platform/apple/isp/Makefile    |   3 +
>  drivers/media/platform/apple/isp/isp-cam.c   | 414 ++++++++++++
>  drivers/media/platform/apple/isp/isp-cam.h   |  21 +
>  drivers/media/platform/apple/isp/isp-cmd.c   | 635 +++++++++++++++++++
>  drivers/media/platform/apple/isp/isp-cmd.h   | 692 ++++++++++++++++++++
>  drivers/media/platform/apple/isp/isp-drv.c   | 586 +++++++++++++++++
>  drivers/media/platform/apple/isp/isp-drv.h   | 284 +++++++++
>  drivers/media/platform/apple/isp/isp-fw.c    | 770 +++++++++++++++++++++++
>  drivers/media/platform/apple/isp/isp-fw.h    |  24 +
>  drivers/media/platform/apple/isp/isp-iommu.c | 251 ++++++++
>  drivers/media/platform/apple/isp/isp-iommu.h |  20 +
>  drivers/media/platform/apple/isp/isp-ipc.c   | 258 ++++++++
>  drivers/media/platform/apple/isp/isp-ipc.h   |  25 +
>  drivers/media/platform/apple/isp/isp-regs.h  |  56 ++
>  drivers/media/platform/apple/isp/isp-v4l2.c  | 900 +++++++++++++++++++++++++++
>  drivers/media/platform/apple/isp/isp-v4l2.h  |  16 +
>  22 files changed, 4982 insertions(+)

quick partial review

> diff --git a/MAINTAINERS b/MAINTAINERS
> index dea7239ee0f5464b31efed5a2e0e5e602bcb6439..60517f7dcee14fc942dd3f77ed5d58eae394f7fa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2248,6 +2248,7 @@ F:	drivers/i2c/busses/i2c-pasemi-platform.c
>  F:	drivers/iommu/apple-dart.c
>  F:	drivers/iommu/io-pgtable-dart.c
>  F:	drivers/irqchip/irq-apple-aic.c
> +F:	drivers/media/platform/apple/*
>  F:	drivers/nvme/host/apple.c
>  F:	drivers/nvmem/apple-efuses.c
>  F:	drivers/pinctrl/pinctrl-apple-gpio.c
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 85d2627776b6a424fbd392187669535c4159ec97..ba75cfdb57f710cca086136e4524d3e1bc1910ac 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -65,6 +65,7 @@ config VIDEO_MUX
>  source "drivers/media/platform/allegro-dvt/Kconfig"
>  source "drivers/media/platform/amlogic/Kconfig"
>  source "drivers/media/platform/amphion/Kconfig"
> +source "drivers/media/platform/apple/Kconfig"
>  source "drivers/media/platform/aspeed/Kconfig"
>  source "drivers/media/platform/atmel/Kconfig"
>  source "drivers/media/platform/broadcom/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index ace4e34483ddce6c3361479989086145dd495f29..e59e4259064bf04b718ea8d128031af859a13d2e 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -8,6 +8,7 @@
>  obj-y += allegro-dvt/
>  obj-y += amlogic/
>  obj-y += amphion/
> +obj-y += apple/
>  obj-y += aspeed/
>  obj-y += atmel/
>  obj-y += broadcom/
> diff --git a/drivers/media/platform/apple/Kconfig b/drivers/media/platform/apple/Kconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..f16508bff5242a0bc433bf8a1d8e3f29737d20d1
> --- /dev/null
> +++ b/drivers/media/platform/apple/Kconfig
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +comment "Apple media platform drivers"
> +
> +source "drivers/media/platform/apple/isp/Kconfig"
> diff --git a/drivers/media/platform/apple/Makefile b/drivers/media/platform/apple/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..d8fe985b0e6c377de6c77d30a3a796c40f3da116
> --- /dev/null
> +++ b/drivers/media/platform/apple/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-y += isp/
> diff --git a/drivers/media/platform/apple/isp/Kconfig b/drivers/media/platform/apple/isp/Kconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..8e94962990031304d51cdd7cd6190b05b05b40bb
> --- /dev/null
> +++ b/drivers/media/platform/apple/isp/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config VIDEO_APPLE_ISP
> +	tristate "Apple Silicon Image Signal Processor driver"
> +	select VIDEOBUF2_CORE
> +	select VIDEOBUF2_V4L2
> +	select VIDEOBUF2_DMA_SG
> +	depends on ARCH_APPLE || COMPILE_TEST
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV

missing dependency on DRM for drm_mm. I don't remember why iova's top
down allocation was a problem but if we can avoid drm_mm that would be
an option as well.

> +	help
> +	  Say Y here to enable support for the ISP and cameras persent
> +	  in Apple ARM laptops.
> +
> +	  To compile this driver as a module, choose M here. The module will be
> +	  called apple_isp
> diff --git a/drivers/media/platform/apple/isp/Makefile b/drivers/media/platform/apple/isp/Makefile
> new file mode 100644
> index 0000000000000000000000000000000000000000..4649f32987f025a639945a37d774d4ecdc83b02a
> --- /dev/null
> +++ b/drivers/media/platform/apple/isp/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +apple-isp-y := isp-cam.o isp-cmd.o isp-drv.o isp-fw.o isp-iommu.o isp-ipc.o isp-v4l2.o
> +obj-$(CONFIG_VIDEO_APPLE_ISP) += apple-isp.o

...

> diff --git a/drivers/media/platform/apple/isp/isp-drv.c b/drivers/media/platform/apple/isp/isp-drv.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b0c73b4f43d73f4ee29093fe62ed1d39ccfa33dd
> --- /dev/null
> +++ b/drivers/media/platform/apple/isp/isp-drv.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Apple Image Signal Processor driver
> + *
> + * Copyright (C) 2023 The Asahi Linux Contributors
> + */
> +
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/workqueue.h>
> +
> +#include "isp-cam.h"
> +#include "isp-fw.h"
> +#include "isp-iommu.h"
> +#include "isp-v4l2.h"
> +

...

> +static int apple_isp_init_iommu(struct apple_isp *isp)
> +{
> +	struct device *dev = isp->dev;
> +	phys_addr_t heap_base;
> +	size_t heap_size;
> +	u64 vm_size;
> +	int err;
> +	int size;
> +	struct device_node *mem_node;
> +	const __be32 *maps, *end;
> +
> +	isp->domain = iommu_get_domain_for_dev(isp->dev);
> +	if (!isp->domain)
> +		return -ENODEV;
> +	isp->shift = __ffs(isp->domain->pgsize_bitmap);
> +
> +	mem_node = of_parse_phandle(dev->of_node, "memory-region", 0);
> +	if (!mem_node) {
> +		dev_err(dev, "No memory-region found for heap\n");
> +		return -ENODEV;
> +	}
> +
> +	maps = of_get_property(mem_node, "iommu-addresses", &size);
> +	if (!maps || !size) {
> +		dev_err(dev, "No valid iommu-addresses found for heap\n");
> +		return -ENODEV;
> +	}
> +
> +	end = maps + size / sizeof(__be32);
> +
> +	while (maps < end) {
> +		maps++;
> +		maps = of_translate_dma_region(dev->of_node, maps, &heap_base,
> +					       &heap_size);
> +	}

The hand-rolled reserved memory parsing looks like it can be replaced
with of_iommu_get_resv_region();
> +

> +	isp->fw.heap_top = heap_base + heap_size;
> +
> +	err = of_property_read_u64(dev->of_node, "apple,dart-vm-size",
> +				   &vm_size);

This is not necessary and can be inferred from
isp->domain->geometry.aperture_{start,end}.

> +	if (err) {
> +		dev_err(dev, "failed to read 'apple,dart-vm-size': %d\n", err);
> +		return err;
> +	}
> +
> +	drm_mm_init(&isp->iovad, isp->fw.heap_top,
> +		    vm_size - (heap_base & 0xffffffff));

drm_mm probably should be replaced with something else. As I wrote above
I don't understand / remember what the problem was with relying on the
DMA api and iova. I can't imagine that the firmware cares about top-down
vs. bottom-up allocation. I could image it was related to
"apple,dart-vm-size" and iova's top down allocation only allocated
outside of the device's aperture. That should be solved by our current
downstream handling of the "vm-base" and "vm-size" properties from
Apple's devicetree.

ciao Janne