| Message ID | 20230112-imx-pxp-v2-0-e2281da1db55@pengutronix.de |
|---|---|
| Headers | show |
| Series | media: imx-pxp: add support for i.MX7D | expand |
On Fr, 2023-01-13 at 10:54 +0100, Michael Tretter wrote: > Various multiplexers in the pipeline are not used with the currently > configured data path. Disable all unused multiplexers by selecting the > "no output" (3) option. > > The datasheet doesn't explicitly require this, but the PXP has been seen > to hang after processing a few hundreds of frames otherwise. > > As at it, add documentation for the multiplexers that are actually > relevant for the data path. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
On Fr, 2023-01-13 at 10:54 +0100, Michael Tretter wrote: > The LUT block is always configured in bypass mode. > > Take it entirely out of the pipeline by disabling it and routing the > data path around the LUT. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
On Fr, 2023-01-13 at 10:54 +0100, Michael Tretter wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Implement support for the VIDIOC_ENUM_FRAMESIZES ioctl. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp
On 13/01/2023 10:54, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > Changelog: > > v2: > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > - restrict number of interrupts per variant > - cleanup syntax > --- > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > 2 files changed, 82 insertions(+), 26 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > new file mode 100644 > index 000000000000..c1232689a261 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml > @@ -0,0 +1,82 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > + > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/fsl,imx6ull-pxp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale Pixel Pipeline > + > +maintainers: > + - Philipp Zabel <p.zabel@pengutronix.de> > + - Michael Tretter <m.tretter@pengutronix.de> > + > +description: > + The Pixel Pipeline (PXP) is a memory-to-memory graphics processing engine > + that supports scaling, colorspace conversion, alpha blending, rotation, and > + pixel conversion via lookup table. Different versions are present on various > + i.MX SoCs from i.MX23 to i.MX7. > + > +properties: > + compatible: > + oneOf: > + - const: fsl,imx6ul-pxp > + - const: fsl,imx6ull-pxp > + - const: fsl,imx7d-pxp These three are an enum. > + - items: > + - enum: > + - fsl,imx6sll-pxp > + - fsl,imx6sx-pxp > + - const: fsl,imx6ull-pxp > + > + reg: > + maxItems: 1 > + > + interrupts: > + minItems: 1 > + maxItems: 2 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: axi > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx6sx-pxp > + then: > + properties: > + interrupts: > + numItems: 1 That's not correct syntax... I am surprised that it works. Did you test the bindings? Best regards, Krzysztof
On Fri, 13 Jan 2023 10:54:07 +0100, Michael Tretter wrote: > Convert the bindings of the Freescale Pixel Pipeline to YAML. > > The conversion drops the previously listed compatibles for several SoCs. > It is unclear, if the PXP on these SoCs is compatible to any of the PXPs > on the existing SoCs and would allow to reuse the already defined > compatibles. The missing compatibles should be brought back when the > support for the PXP on these SoCs is added. > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > Changelog: > > v2: > > - add fsl,imx6sll-pxp and fsl,imx6sx-pxp compatibles > - restrict number of interrupts per variant > - cleanup syntax > --- > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 ++++++++++++++++++++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 ------- > 2 files changed, 82 insertions(+), 26 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml: allOf:0:else:properties:interrupts: 'anyOf' conditional failed, one must be fixed: 'numItems' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml: allOf:0:then:properties:interrupts: 'anyOf' conditional failed, one must be fixed: 'numItems' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems'] 'type' was expected from schema $id: http://devicetree.org/meta-schemas/keywords.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230112-imx-pxp-v2-1-e2281da1db55@pengutronix.de The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 13/01/2023 16:09, Michael Tretter wrote: >>> +properties: >>> + compatible: >>> + oneOf: >>> + - const: fsl,imx6ul-pxp >>> + - const: fsl,imx6ull-pxp >>> + - const: fsl,imx7d-pxp >> >> These three are an enum. > > These are alternatives to the 'items:' entry below. > > Are you suggesting to use the following statement? > > oneOf: > - enum: > - fsl,imx6ul-pxp > - fsl,imx6ull-pxp > - fsl,imx7d-pxp > - items: > - enum: > - fsl,imx6sll-pxp > - fsl,imx6sx-pxp Yes. > > Why is this better than the one that I used? Because that's the convention - use enum for enumeration which nicely groups all of them and is the easiest to read. > >> >>> + - items: >>> + - enum: >>> + - fsl,imx6sll-pxp >>> + - fsl,imx6sx-pxp >>> + - const: fsl,imx6ull-pxp >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + minItems: 1 >>> + maxItems: 2 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + clock-names: >>> + const: axi >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + - clocks >>> + - clock-names >>> + >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - fsl,imx6sx-pxp >>> + then: >>> + properties: >>> + interrupts: >>> + numItems: 1 >> >> That's not correct syntax... I am surprised that it works. Did you test >> the bindings? > > I copied this syntax from renesas,wdt.yaml and ran > > make ARCH=arm dtbs_check DT_SCHEMA_FILES=fsl,imx6ull-pxp.yaml > > with SOC_IMX7D=y, SOC_IMX6UL=y, SOC_IMX6SLL=y, and SOC_IMX6SX=y. The latter > two were not enabled in the v1, which is why it didn't catch the missing > compatibles. > > On a closer look, I just saw that the checker ignored the schema due to the > incorrect syntax and didn't produce any further errors. With the syntax fixed, > the checker now produces also a few more errors about power-domains, which I > will fix in v3. > > Is this syntax correct? > > allOf: > - if: > properties: > compatible: > contains: > enum: > - fsl,imx6sx-pxp > then: > properties: > interrupts: > minItems: 1 Drop this one > maxItems: 1 > else: > properties: > interrupts: > minItems: 2 > maxItems: 2 > > Michael Rest is ok Best regards, Krzysztof
Hi Michael, The whole series has been reviewed, and only the DT bindings need a small change. If you post a v2.1 of just that patch (--in-reply-to v2 and CC me, or I may not notice quickly), I can take the whole series and get it merged in v6.3. On Fri, Jan 13, 2023 at 10:54:07AM +0100, Michael Tretter wrote: > This is v2 of the series to add support for the PXP found on the i.MX7D to the > imx-pxp driver. > > The PXP on the i.MX7D has a few differences compared to the one on the > i.MX6ULL. Especially, it has more processing blocks and slightly different > multiplexers to route the data between the blocks. Therefore, the driver must > configure a different data path depending on the platform. > > While the PXP has a version register, the reported version is the same on the > i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to > change the driver behavior, but have to use the device tree compatible. The > driver still prints the found version to the log to help bringing up the PXP > on further platforms. > > The patches are inspired by some earlier patches [0] by Laurent to add PXP > support to the i.MX7d. Compared to the earlier patches, these patches add > different behavior depending on the platform. Furthermore, the patches disable > only the LUT block, but keep the rotator block enabled, as it may now be > configured via the V4L2 rotate control. > > In v2, I included Laurent's patch series [1], which was based on this series > anyway and added regmap support. > > Patch 1 converts the dt-binding to yaml. > > Patches 2 to 5 cleanup and refactor the driver in preparation of handling > different PXP versions. > > Patches 6 and 7 add the handling of different platforms and the i.MX7d > specific configuration. > > Patch 8 adds the device tree node for the PXP to the i.MX7d device tree. > > Patches 9 to 15 are the cleanup and enhancement patches to add media > controller support, implement enum_framesizes, and add pxp_read/pxp_write > helpers. > > Patch 16 adds regmap support to the driver. > > Michael > > [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/ > [1] https://lore.kernel.org/linux-media/20230112172507.30579-1-laurent.pinchart@ideasonboard.com > > --- > > Changelog > > v2: > > - fix device tree binding > - reduce log level of PXP version to debug > - drop fallback for missing pdata > - add cleanup and enhancement patches to series > - convert driver to regmap > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > To: linux-media@vger.kernel.org > To: devicetree@vger.kernel.org > To: Philipp Zabel <p.zabel@pengutronix.de> > To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Alexander Stein <alexander.stein@ew.tq-group.com> > Cc: kernel@pengutronix.de > Cc: linux-imx@nxp.com > Cc: linux-arm-kernel@lists.infradead.org > Cc: Michael Tretter <m.tretter@pengutronix.de> > > --- > Laurent Pinchart (7): > media: imx-pxp: Sort headers alphabetically > media: imx-pxp: Don't set bus_info manually in .querycap() > media: imx-pxp: Add media controller support > media: imx-pxp: Pass pixel format value to find_format() > media: imx-pxp: Implement frame size enumeration > media: imx-pxp: Introduce pxp_read() and pxp_write() wrappers > media: imx-pxp: Use non-threaded IRQ > > Michael Tretter (9): > dt-bindings: media: fsl-pxp: convert to yaml > media: imx-pxp: detect PXP version > media: imx-pxp: extract helper function to setup data path > media: imx-pxp: explicitly disable unused blocks > media: imx-pxp: disable LUT block > media: imx-pxp: make data_path_ctrl0 platform dependent > media: imx-pxp: add support for i.MX7D > ARM: dts: imx7d: add node for PXP > media: imx-pxp: convert to regmap > > .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 +++++ > .../devicetree/bindings/media/fsl-pxp.txt | 26 -- > arch/arm/boot/dts/imx7d.dtsi | 9 + > drivers/media/platform/nxp/imx-pxp.c | 359 +++++++++++++++------ > 4 files changed, 355 insertions(+), 121 deletions(-) > --- > base-commit: b7bfaa761d760e72a969d116517eaa12e404c262 > change-id: 20230112-imx-pxp-073008b3c857
This is v2 of the series to add support for the PXP found on the i.MX7D to the imx-pxp driver. The PXP on the i.MX7D has a few differences compared to the one on the i.MX6ULL. Especially, it has more processing blocks and slightly different multiplexers to route the data between the blocks. Therefore, the driver must configure a different data path depending on the platform. While the PXP has a version register, the reported version is the same on the i.MX6ULL and the i.MX7D. Therefore, we cannot use the version register to change the driver behavior, but have to use the device tree compatible. The driver still prints the found version to the log to help bringing up the PXP on further platforms. The patches are inspired by some earlier patches [0] by Laurent to add PXP support to the i.MX7d. Compared to the earlier patches, these patches add different behavior depending on the platform. Furthermore, the patches disable only the LUT block, but keep the rotator block enabled, as it may now be configured via the V4L2 rotate control. In v2, I included Laurent's patch series [1], which was based on this series anyway and added regmap support. Patch 1 converts the dt-binding to yaml. Patches 2 to 5 cleanup and refactor the driver in preparation of handling different PXP versions. Patches 6 and 7 add the handling of different platforms and the i.MX7d specific configuration. Patch 8 adds the device tree node for the PXP to the i.MX7d device tree. Patches 9 to 15 are the cleanup and enhancement patches to add media controller support, implement enum_framesizes, and add pxp_read/pxp_write helpers. Patch 16 adds regmap support to the driver. Michael [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/ [1] https://lore.kernel.org/linux-media/20230112172507.30579-1-laurent.pinchart@ideasonboard.com --- Changelog v2: - fix device tree binding - reduce log level of PXP version to debug - drop fallback for missing pdata - add cleanup and enhancement patches to series - convert driver to regmap Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> To: linux-media@vger.kernel.org To: devicetree@vger.kernel.org To: Philipp Zabel <p.zabel@pengutronix.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: Fabio Estevam <festevam@gmail.com> Cc: Alexander Stein <alexander.stein@ew.tq-group.com> Cc: kernel@pengutronix.de Cc: linux-imx@nxp.com Cc: linux-arm-kernel@lists.infradead.org Cc: Michael Tretter <m.tretter@pengutronix.de> --- Laurent Pinchart (7): media: imx-pxp: Sort headers alphabetically media: imx-pxp: Don't set bus_info manually in .querycap() media: imx-pxp: Add media controller support media: imx-pxp: Pass pixel format value to find_format() media: imx-pxp: Implement frame size enumeration media: imx-pxp: Introduce pxp_read() and pxp_write() wrappers media: imx-pxp: Use non-threaded IRQ Michael Tretter (9): dt-bindings: media: fsl-pxp: convert to yaml media: imx-pxp: detect PXP version media: imx-pxp: extract helper function to setup data path media: imx-pxp: explicitly disable unused blocks media: imx-pxp: disable LUT block media: imx-pxp: make data_path_ctrl0 platform dependent media: imx-pxp: add support for i.MX7D ARM: dts: imx7d: add node for PXP media: imx-pxp: convert to regmap .../devicetree/bindings/media/fsl,imx6ull-pxp.yaml | 82 +++++ .../devicetree/bindings/media/fsl-pxp.txt | 26 -- arch/arm/boot/dts/imx7d.dtsi | 9 + drivers/media/platform/nxp/imx-pxp.c | 359 +++++++++++++++------ 4 files changed, 355 insertions(+), 121 deletions(-) --- base-commit: b7bfaa761d760e72a969d116517eaa12e404c262 change-id: 20230112-imx-pxp-073008b3c857 Best regards,