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