diff mbox series

[3/5] media: dt-bindings: Add Apple ISP

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

Commit Message

Sasha Finkelstein via B4 Relay Feb. 19, 2025, 9:26 a.m. UTC
From: Sasha Finkelstein <fnkl.kernel@gmail.com>

Add bindings for the ISP used with the webcam in Apple
ARM laptops.

Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
---
 .../devicetree/bindings/media/apple,isp.yaml       | 151 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 152 insertions(+)

Comments

Krzysztof Kozlowski Feb. 19, 2025, 9:37 a.m. UTC | #1
On 19/02/2025 10:26, Sasha Finkelstein via B4 Relay wrote:
> +  reg-names:
> +    items:
> +      - const: coproc
> +      - const: mbox
> +      - const: gpio
> +      - const: mbox2
> +
> +  iommus:
> +    description: All 3 must be kept in sync
> +    minItems: 3


Drop minItems

> +    maxItems: 3
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    minItems: 1
> +    maxItems: 20
> +    description: All necessary power domains. Driver will enable them in order
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  apple,dart-vm-size:
> +    description: Supported device memory range
> +    $ref: /schemas/types.yaml#/definitions/uint64


That's deducible from comaptible.

> +
> +  apple,platform-id:
> +    description: Platform id for firmware
> +    $ref: /schemas/types.yaml#/definitions/uint32


No, use firmware-name.

> +
> +  apple,temporal-filter:
> +    description: Whether temporal filter should be enabled in firmware
> +    $ref: /schemas/types.yaml#/definitions/uint32

And why is this not enabled always? Why this is board specific?


You miss here ports or port. ISP usually gets signal from some camera or
other block.


> +
> +  sensor-presets:
> +    additionalProperties: false
> +
> +    patternProperties:
> +      '^preset[0-9]+$':
> +        type: object
> +
> +        additionalProperties: false
> +
> +        properties:
> +          apple,config-index:
> +            description: Firmware config index
> +            $ref: /schemas/types.yaml#/definitions/uint32


No duplicated indices. You have reg for this, assuming this is index.

> +
> +          apple,input-size:
> +            $ref: /schemas/types.yaml#/definitions/uint32-array
> +            minItems: 2
> +            maxItems: 2
> +            description: Raw sensor size
> +
> +          apple,output-size:
> +            $ref: /schemas/types.yaml#/definitions/uint32-array
> +            minItems: 2
> +            maxItems: 2
> +            description: Cropped and scaled image size
> +
> +          apple,crop:
> +            $ref: /schemas/types.yaml#/definitions/uint32-array
> +            minItems: 4
> +            maxItems: 4
> +            description: Area to crop


All these do not look like hardware properties but rather configuration
of sensor which should be done runtime by OS, not by DT.

Best regards,
Krzysztof
Sasha Finkelstein Feb. 19, 2025, 9:54 a.m. UTC | #2
On Wed, 19 Feb 2025 at 10:37, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > +
> > +  apple,platform-id:
> > +    description: Platform id for firmware
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
>
> No, use firmware-name.

Not sure how is firmware-name an appropriate field, fw-name is a string
that references a firmware file, while this field is an id that is sent to the
coprocessor firmware in order to identify the platform.

> > +  apple,temporal-filter:
> > +    description: Whether temporal filter should be enabled in firmware
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> And why is this not enabled always? Why this is board specific?

Not every board has support for this feature.

> You miss here ports or port. ISP usually gets signal from some camera or
> other block.

For complex cameras - yes, but this is closer to a UVC camera connected
via a bespoke protocol. We do not need to deal with the sensor access,
all of it is managed by the coprocessor firmware.

> > +        properties:
> > +          apple,config-index:
> > +            description: Firmware config index
> > +            $ref: /schemas/types.yaml#/definitions/uint32
>
>
> No duplicated indices. You have reg for this, assuming this is index.

There are duplicated indices, see isp-imx248.dtsi in patch 5 for an example.

> All these do not look like hardware properties but rather configuration
> of sensor which should be done runtime by OS, not by DT.

Those are board-specific and not discoverable via the ISP protocol.
Laurent Pinchart Feb. 19, 2025, 10:53 a.m. UTC | #3
On Wed, Feb 19, 2025 at 10:54:31AM +0100, Sasha Finkelstein wrote:
> On Wed, 19 Feb 2025 at 10:37, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > +
> > > +  apple,platform-id:
> > > +    description: Platform id for firmware
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> >
> >
> > No, use firmware-name.
> 
> Not sure how is firmware-name an appropriate field, fw-name is a string
> that references a firmware file, while this field is an id that is sent to the
> coprocessor firmware in order to identify the platform.
> 
> > > +  apple,temporal-filter:
> > > +    description: Whether temporal filter should be enabled in firmware
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> >
> > And why is this not enabled always? Why this is board specific?
> 
> Not every board has support for this feature.
> 
> > You miss here ports or port. ISP usually gets signal from some camera or
> > other block.
> 
> For complex cameras - yes, but this is closer to a UVC camera connected
> via a bespoke protocol. We do not need to deal with the sensor access,
> all of it is managed by the coprocessor firmware.
> 
> > > +        properties:
> > > +          apple,config-index:
> > > +            description: Firmware config index
> > > +            $ref: /schemas/types.yaml#/definitions/uint32
> >
> >
> > No duplicated indices. You have reg for this, assuming this is index.
> 
> There are duplicated indices, see isp-imx248.dtsi in patch 5 for an example.
> 
> > All these do not look like hardware properties but rather configuration
> > of sensor which should be done runtime by OS, not by DT.
> 
> Those are board-specific and not discoverable via the ISP protocol.

But they are settable through the ISP protocol, aren't they ? For
instance, looking at isp-imx248.dtsi, the first four entries are

	/* 1280x720 */
	preset0 {
		apple,config-index = <0>;
		apple,input-size = <1296 736>;
		apple,output-size = <1280 720>;
		apple,crop = <8 8 1280 720>;
	};

	/* 960x720 (4:3) */
	preset1 {
		apple,config-index = <0>;
		apple,input-size = <1296 736>;
		apple,output-size = <960 720>;
		apple,crop = <168 8 960 720>;
	};

	/* 960x540 (16:9) */
	preset2 {
		apple,config-index = <0>;
		apple,input-size = <1296 736>;
		apple,output-size = <960 540>;
		apple,crop = <8 8 1280 720>;
	};

	/* 640x480 (4:3) */
	preset3 {
		apple,config-index = <0>;
		apple,input-size = <1296 736>;
		apple,output-size = <640 480>;
		apple,crop = <168 8 960 720>;
	};

But I may be interested in capturing a 640x480 frame with cropping only
and without scaling, with

input-size = 1296x736
output-size = 640x480
crop = (328,128)/640x480

Or I may want my cropped frame to be located in the upper-left corner:

input-size = 1296x736
output-size = 640x480
crop = (8,8)/640x480

If I set those parameters through the ISP protocol, won't it work ?
Sasha Finkelstein Feb. 19, 2025, 11:05 a.m. UTC | #4
On Wed, 19 Feb 2025 at 11:53, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> >
> > Those are board-specific and not discoverable via the ISP protocol.
>
> But they are settable through the ISP protocol, aren't they ? For
> instance, looking at isp-imx248.dtsi, the first four entries are
>
>         /* 1280x720 */
>         preset0 {
>                 apple,config-index = <0>;
>                 apple,input-size = <1296 736>;
>                 apple,output-size = <1280 720>;
>                 apple,crop = <8 8 1280 720>;
>         };
>
>         /* 960x720 (4:3) */
>         preset1 {
>                 apple,config-index = <0>;
>                 apple,input-size = <1296 736>;
>                 apple,output-size = <960 720>;
>                 apple,crop = <168 8 960 720>;
>         };
>
>         /* 960x540 (16:9) */
>         preset2 {
>                 apple,config-index = <0>;
>                 apple,input-size = <1296 736>;
>                 apple,output-size = <960 540>;
>                 apple,crop = <8 8 1280 720>;
>         };
>
>         /* 640x480 (4:3) */
>         preset3 {
>                 apple,config-index = <0>;
>                 apple,input-size = <1296 736>;
>                 apple,output-size = <640 480>;
>                 apple,crop = <168 8 960 720>;
>         };
>
> But I may be interested in capturing a 640x480 frame with cropping only
> and without scaling, with
>
> input-size = 1296x736
> output-size = 640x480
> crop = (328,128)/640x480
>
> Or I may want my cropped frame to be located in the upper-left corner:
>
> input-size = 1296x736
> output-size = 640x480
> crop = (8,8)/640x480
>
> If I set those parameters through the ISP protocol, won't it work ?
>
> --
> Regards,
>
> Laurent Pinchart

For cropping - you do not want to change those parameters, the sensor
is partially occluded, and the crop area is specified in such a way
to not expose those pixels. As for scaling - we can expose only the 1:1
scale and let userspace deal with it, but it appears that it expects
the other common output sizes to exist.
Janne Grunau Feb. 19, 2025, 11:57 a.m. UTC | #5
On Wed, Feb 19, 2025 at 12:53:26PM +0200, Laurent Pinchart wrote:
> On Wed, Feb 19, 2025 at 10:54:31AM +0100, Sasha Finkelstein wrote:
> > On Wed, 19 Feb 2025 at 10:37, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > +
> > > > +  apple,platform-id:
> > > > +    description: Platform id for firmware
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > >
> > > No, use firmware-name.
> > 
> > Not sure how is firmware-name an appropriate field, fw-name is a string
> > that references a firmware file, while this field is an id that is sent to the
> > coprocessor firmware in order to identify the platform.
> > 
> > > > +  apple,temporal-filter:
> > > > +    description: Whether temporal filter should be enabled in firmware
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > > And why is this not enabled always? Why this is board specific?
> > 
> > Not every board has support for this feature.
> > 
> > > You miss here ports or port. ISP usually gets signal from some camera or
> > > other block.
> > 
> > For complex cameras - yes, but this is closer to a UVC camera connected
> > via a bespoke protocol. We do not need to deal with the sensor access,
> > all of it is managed by the coprocessor firmware.
> > 
> > > > +        properties:
> > > > +          apple,config-index:
> > > > +            description: Firmware config index
> > > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > >
> > > No duplicated indices. You have reg for this, assuming this is index.
> > 
> > There are duplicated indices, see isp-imx248.dtsi in patch 5 for an example.
> > 
> > > All these do not look like hardware properties but rather configuration
> > > of sensor which should be done runtime by OS, not by DT.
> > 
> > Those are board-specific and not discoverable via the ISP protocol.
> 
> But they are settable through the ISP protocol, aren't they ? For
> instance, looking at isp-imx248.dtsi, the first four entries are
> 
> 	/* 1280x720 */
> 	preset0 {
> 		apple,config-index = <0>;
> 		apple,input-size = <1296 736>;
> 		apple,output-size = <1280 720>;
> 		apple,crop = <8 8 1280 720>;
> 	};
> 
> 	/* 960x720 (4:3) */
> 	preset1 {
> 		apple,config-index = <0>;
> 		apple,input-size = <1296 736>;
> 		apple,output-size = <960 720>;
> 		apple,crop = <168 8 960 720>;
> 	};
> 
> 	/* 960x540 (16:9) */
> 	preset2 {
> 		apple,config-index = <0>;
> 		apple,input-size = <1296 736>;
> 		apple,output-size = <960 540>;
> 		apple,crop = <8 8 1280 720>;
> 	};
> 
> 	/* 640x480 (4:3) */
> 	preset3 {
> 		apple,config-index = <0>;
> 		apple,input-size = <1296 736>;
> 		apple,output-size = <640 480>;
> 		apple,crop = <168 8 960 720>;
> 	};
> 
> But I may be interested in capturing a 640x480 frame with cropping only
> and without scaling, with
> 
> input-size = 1296x736
> output-size = 640x480
> crop = (328,128)/640x480
> 
> Or I may want my cropped frame to be located in the upper-left corner:
> 
> input-size = 1296x736
> output-size = 640x480
> crop = (8,8)/640x480
> 
> If I set those parameters through the ISP protocol, won't it work ?

If my memory serves me right the presets wre added as workaround for
userspace not handling V4L2_FRMSIZE_TYPE_STEPWISE well (or at all) and
the added complexity of handling the qadratic sensor with partially
occluded or outside of the usable lens diameter corners.

It is a simplified description of the hardware to make it useable for
most software which is expected simple uvc cameras.

There are still two common issues in user space software related to this
driver:
- software expects width == linesize
- resolution selection is based frame height, i.e. it prefers 1080x1920
  over 1920x1080 on devices with quadratic sensor.

ciao Janne
Laurent Pinchart Feb. 19, 2025, 1:40 p.m. UTC | #6
On Wed, Feb 19, 2025 at 12:05:29PM +0100, Sasha Finkelstein wrote:
> On Wed, 19 Feb 2025 at 11:53, Laurent Pinchart wrote:
> > >
> > > Those are board-specific and not discoverable via the ISP protocol.
> >
> > But they are settable through the ISP protocol, aren't they ? For
> > instance, looking at isp-imx248.dtsi, the first four entries are
> >
> >         /* 1280x720 */
> >         preset0 {
> >                 apple,config-index = <0>;
> >                 apple,input-size = <1296 736>;
> >                 apple,output-size = <1280 720>;
> >                 apple,crop = <8 8 1280 720>;
> >         };
> >
> >         /* 960x720 (4:3) */
> >         preset1 {
> >                 apple,config-index = <0>;
> >                 apple,input-size = <1296 736>;
> >                 apple,output-size = <960 720>;
> >                 apple,crop = <168 8 960 720>;
> >         };
> >
> >         /* 960x540 (16:9) */
> >         preset2 {
> >                 apple,config-index = <0>;
> >                 apple,input-size = <1296 736>;
> >                 apple,output-size = <960 540>;
> >                 apple,crop = <8 8 1280 720>;
> >         };
> >
> >         /* 640x480 (4:3) */
> >         preset3 {
> >                 apple,config-index = <0>;
> >                 apple,input-size = <1296 736>;
> >                 apple,output-size = <640 480>;
> >                 apple,crop = <168 8 960 720>;
> >         };
> >
> > But I may be interested in capturing a 640x480 frame with cropping only
> > and without scaling, with
> >
> > input-size = 1296x736
> > output-size = 640x480
> > crop = (328,128)/640x480
> >
> > Or I may want my cropped frame to be located in the upper-left corner:
> >
> > input-size = 1296x736
> > output-size = 640x480
> > crop = (8,8)/640x480
> >
> > If I set those parameters through the ISP protocol, won't it work ?
> 
> For cropping - you do not want to change those parameters, the sensor
> is partially occluded, and the crop area is specified in such a way
> to not expose those pixels.

Surely cropping *more* wouldn't be an issue from that point of view ?
The visible area is a device-specific property, so that could be
specified in DT, instead of presets. The problem isn't limited to this
device, so ideally the DT binding for this feature should be
standardized.

In general, I'd say that even occluded pixels should be exposed to
userspace, should an application want to capture them. If you think of
fisheye lenses, for instance, it's common to have a round image captured
from a rectangular sensor, and use a dewarping engine (it could just be
GPU shaders) to un-distord it. Restricting capture to rectangles of
fully visible pixels would result in loss of information. For this
particular device I don't think that would really be a use case, so I'm
not opposed to the driver setting restrictions on crop rectangles.

> As for scaling - we can expose only the 1:1
> scale and let userspace deal with it, but it appears that it expects
> the other common output sizes to exist.

Can the driver synthesize the list of scaled-down resolutions, instead
of specifying them in DT ?
Laurent Pinchart Feb. 19, 2025, 1:44 p.m. UTC | #7
On Wed, Feb 19, 2025 at 12:57:37PM +0100, Janne Grunau wrote:
> On Wed, Feb 19, 2025 at 12:53:26PM +0200, Laurent Pinchart wrote:
> > On Wed, Feb 19, 2025 at 10:54:31AM +0100, Sasha Finkelstein wrote:
> > > On Wed, 19 Feb 2025 at 10:37, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > > +
> > > > > +  apple,platform-id:
> > > > > +    description: Platform id for firmware
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > >
> > > >
> > > > No, use firmware-name.
> > > 
> > > Not sure how is firmware-name an appropriate field, fw-name is a string
> > > that references a firmware file, while this field is an id that is sent to the
> > > coprocessor firmware in order to identify the platform.
> > > 
> > > > > +  apple,temporal-filter:
> > > > > +    description: Whether temporal filter should be enabled in firmware
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > >
> > > > And why is this not enabled always? Why this is board specific?
> > > 
> > > Not every board has support for this feature.
> > > 
> > > > You miss here ports or port. ISP usually gets signal from some camera or
> > > > other block.
> > > 
> > > For complex cameras - yes, but this is closer to a UVC camera connected
> > > via a bespoke protocol. We do not need to deal with the sensor access,
> > > all of it is managed by the coprocessor firmware.
> > > 
> > > > > +        properties:
> > > > > +          apple,config-index:
> > > > > +            description: Firmware config index
> > > > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > >
> > > >
> > > > No duplicated indices. You have reg for this, assuming this is index.
> > > 
> > > There are duplicated indices, see isp-imx248.dtsi in patch 5 for an example.
> > > 
> > > > All these do not look like hardware properties but rather configuration
> > > > of sensor which should be done runtime by OS, not by DT.
> > > 
> > > Those are board-specific and not discoverable via the ISP protocol.
> > 
> > But they are settable through the ISP protocol, aren't they ? For
> > instance, looking at isp-imx248.dtsi, the first four entries are
> > 
> > 	/* 1280x720 */
> > 	preset0 {
> > 		apple,config-index = <0>;
> > 		apple,input-size = <1296 736>;
> > 		apple,output-size = <1280 720>;
> > 		apple,crop = <8 8 1280 720>;
> > 	};
> > 
> > 	/* 960x720 (4:3) */
> > 	preset1 {
> > 		apple,config-index = <0>;
> > 		apple,input-size = <1296 736>;
> > 		apple,output-size = <960 720>;
> > 		apple,crop = <168 8 960 720>;
> > 	};
> > 
> > 	/* 960x540 (16:9) */
> > 	preset2 {
> > 		apple,config-index = <0>;
> > 		apple,input-size = <1296 736>;
> > 		apple,output-size = <960 540>;
> > 		apple,crop = <8 8 1280 720>;
> > 	};
> > 
> > 	/* 640x480 (4:3) */
> > 	preset3 {
> > 		apple,config-index = <0>;
> > 		apple,input-size = <1296 736>;
> > 		apple,output-size = <640 480>;
> > 		apple,crop = <168 8 960 720>;
> > 	};
> > 
> > But I may be interested in capturing a 640x480 frame with cropping only
> > and without scaling, with
> > 
> > input-size = 1296x736
> > output-size = 640x480
> > crop = (328,128)/640x480
> > 
> > Or I may want my cropped frame to be located in the upper-left corner:
> > 
> > input-size = 1296x736
> > output-size = 640x480
> > crop = (8,8)/640x480
> > 
> > If I set those parameters through the ISP protocol, won't it work ?
> 
> If my memory serves me right the presets wre added as workaround for
> userspace not handling V4L2_FRMSIZE_TYPE_STEPWISE well (or at all) and
> the added complexity of handling the qadratic sensor with partially
> occluded or outside of the usable lens diameter corners.
> 
> It is a simplified description of the hardware to make it useable for
> most software which is expected simple uvc cameras.

I understand that. Ideally userspace should be fixed, but in the
meantime, I'm fine with the driver exposing a set of presets. They
should however not be listed in DT, but computed by the driver based on
properties that describe the device (such as, for instance, the full
sensor resolution, or the visible area). Those properties could come
from DT, or be hardcoded in the driver on a per-compatible basis.

> There are still two common issues in user space software related to this
> driver:
> - software expects width == linesize
> - resolution selection is based frame height, i.e. it prefers 1080x1920
>   over 1920x1080 on devices with quadratic sensor.

I share your pain, having to fix userspace when doing kernel work isn't
always fun :-)
Asahi Lina Feb. 19, 2025, 3:36 p.m. UTC | #8
On February 19, 2025 12:05:29 PM GMT+01:00, Sasha Finkelstein <fnkl.kernel@gmail.com> wrote:
>On Wed, 19 Feb 2025 at 11:53, Laurent Pinchart
><laurent.pinchart@ideasonboard.com> wrote:
>> >
>> > Those are board-specific and not discoverable via the ISP protocol.
>>
>> But they are settable through the ISP protocol, aren't they ? For
>> instance, looking at isp-imx248.dtsi, the first four entries are
>>
>>         /* 1280x720 */
>>         preset0 {
>>                 apple,config-index = <0>;
>>                 apple,input-size = <1296 736>;
>>                 apple,output-size = <1280 720>;
>>                 apple,crop = <8 8 1280 720>;
>>         };
>>
>>         /* 960x720 (4:3) */
>>         preset1 {
>>                 apple,config-index = <0>;
>>                 apple,input-size = <1296 736>;
>>                 apple,output-size = <960 720>;
>>                 apple,crop = <168 8 960 720>;
>>         };
>>
>>         /* 960x540 (16:9) */
>>         preset2 {
>>                 apple,config-index = <0>;
>>                 apple,input-size = <1296 736>;
>>                 apple,output-size = <960 540>;
>>                 apple,crop = <8 8 1280 720>;
>>         };
>>
>>         /* 640x480 (4:3) */
>>         preset3 {
>>                 apple,config-index = <0>;
>>                 apple,input-size = <1296 736>;
>>                 apple,output-size = <640 480>;
>>                 apple,crop = <168 8 960 720>;
>>         };
>>
>> But I may be interested in capturing a 640x480 frame with cropping only
>> and without scaling, with
>>
>> input-size = 1296x736
>> output-size = 640x480
>> crop = (328,128)/640x480
>>
>> Or I may want my cropped frame to be located in the upper-left corner:
>>
>> input-size = 1296x736
>> output-size = 640x480
>> crop = (8,8)/640x480
>>
>> If I set those parameters through the ISP protocol, won't it work ?
>>
>> --
>> Regards,
>>
>> Laurent Pinchart
>
>For cropping - you do not want to change those parameters, the sensor
>is partially occluded, and the crop area is specified in such a way
>to not expose those pixels. As for scaling - we can expose only the 1:1
>scale and let userspace deal with it, but it appears that it expects
>the other common output sizes to exist.
>
>

The square sensor with the hack is the IMX558. The configs there are actually a workaround for missing ANE support.

What the driver is supposed to be doing is using the higher numbered presets, which crop internally and exclude the occluded area. But those on machines with that sensor end up requiring ANE integration for denoising and ISP crashes without it.

Config #0 doesn't use ANE, so the crops are re-creations of the other configs so it behaves the same minus the denoising.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/apple,isp.yaml b/Documentation/devicetree/bindings/media/apple,isp.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..67d536b61851af30fcc5bc452a761138876c6b18
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/apple,isp.yaml
@@ -0,0 +1,151 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/apple,isp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: An ISP block used in Apple products
+
+maintainers:
+  - Sasha Finkelstein <fnkl.kernel@gmail.com>
+
+description:
+  The ISP in charge of webcams on ARM Apple laptops
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - apple,t8103-isp
+          - apple,t8112-isp
+          - apple,t6000-isp
+          - apple,t6020-isp
+      - const: apple,isp
+
+  reg:
+    items:
+      - description: ASC coprocessor control
+      - description: Peripheral to host mailbox
+      - description: General-purpose ASC IO registers
+      - description: Host to peripheral mailbox
+
+  reg-names:
+    items:
+      - const: coproc
+      - const: mbox
+      - const: gpio
+      - const: mbox2
+
+  iommus:
+    description: All 3 must be kept in sync
+    minItems: 3
+    maxItems: 3
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    minItems: 1
+    maxItems: 20
+    description: All necessary power domains. Driver will enable them in order
+
+  memory-region:
+    maxItems: 1
+
+  apple,dart-vm-size:
+    description: Supported device memory range
+    $ref: /schemas/types.yaml#/definitions/uint64
+
+  apple,platform-id:
+    description: Platform id for firmware
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  apple,temporal-filter:
+    description: Whether temporal filter should be enabled in firmware
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  sensor-presets:
+    additionalProperties: false
+
+    patternProperties:
+      '^preset[0-9]+$':
+        type: object
+
+        additionalProperties: false
+
+        properties:
+          apple,config-index:
+            description: Firmware config index
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          apple,input-size:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 2
+            maxItems: 2
+            description: Raw sensor size
+
+          apple,output-size:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 2
+            maxItems: 2
+            description: Cropped and scaled image size
+
+          apple,crop:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            minItems: 4
+            maxItems: 4
+            description: Area to crop
+
+        required:
+          - apple,config-index
+          - apple,input-size
+          - apple,output-size
+          - apple,crop
+
+required:
+  - compatible
+  - reg
+  - iommus
+  - interrupts
+  - power-domains
+  - memory-region
+  - apple,dart-vm-size
+  - apple,platform-id
+  - apple,temporal-filter
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/apple-aic.h>
+    isp: isp@2a000000 {
+        compatible = "apple,t8103-isp", "apple,isp";
+        reg = <0x2a000000 0x2000000>,
+              <0x2c104000 0x100>,
+              <0x2c104170 0x100>,
+              <0x2c1043f0 0x100>;
+        reg-names = "coproc", "mbox", "gpio", "mbox2";
+        iommus = <&isp_dart0 0>, <&isp_dart1 0>, <&isp_dart2 0>;
+        interrupt-parent = <&aic>;
+        interrupts = <AIC_IRQ 246 IRQ_TYPE_LEVEL_HIGH>;
+        power-domains = <&ps_isp_sys>, <&ps_isp_set0>,
+                        <&ps_isp_set1>, <&ps_isp_set2>, <&ps_isp_fe>,
+                        <&ps_isp_set4>, <&ps_isp_set5>, <&ps_isp_set6>,
+                        <&ps_isp_set7>, <&ps_isp_set8>, <&ps_isp_set9>,
+                        <&ps_isp_set10>, <&ps_isp_set11>,
+                        <&ps_isp_set12>;
+        memory-region = <&isp_heap>;
+        apple,dart-vm-size = <0x0 0xa0000000>;
+        apple,platform-id = <1>;
+        apple,temporal-filter = <0>;
+
+        sensor-presets {
+            preset0 {
+                apple,config-index = <0>;
+                apple,input-size = <1296 736>;
+                apple,output-size = <1280 720>;
+                apple,crop = <8 8 1280 720>;
+            };
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index efee40ea589f70bc5e4a390072a4543234616743..dea7239ee0f5464b31efed5a2e0e5e602bcb6439 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2228,6 +2228,7 @@  F:	Documentation/devicetree/bindings/interrupt-controller/apple,*
 F:	Documentation/devicetree/bindings/iommu/apple,dart.yaml
 F:	Documentation/devicetree/bindings/iommu/apple,sart.yaml
 F:	Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
+F:	Documentation/devicetree/bindings/media/apple,isp.yaml
 F:	Documentation/devicetree/bindings/net/bluetooth/brcm,bcm4377-bluetooth.yaml
 F:	Documentation/devicetree/bindings/nvme/apple,nvme-ans.yaml
 F:	Documentation/devicetree/bindings/nvmem/apple,efuses.yaml