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

From: Laurent Pinchart
Date: Wed Feb 19 2025 - 05:53:56 EST


On Wed, Feb 19, 2025 at 10:54:31AM +0100, Sasha Finkelstein wrote:
> On Wed, 19 Feb 2025 at 10:37, Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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 ?

--
Regards,

Laurent Pinchart