Re: [PATCH 0/8] media: imx-pxp: add support for i.MX7D

[Michael Tretter <m.tretter@pengutronix.de>]

On Fri, 06 Jan 2023 14:41:32 +0200, Laurent Pinchart wrote:
> Hi Michael,
> 
> On Thu, Jan 05, 2023 at 02:47:21PM +0100, Michael Tretter wrote:
> > This series adds 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.
> 
> Sounds good to me.
> 
> > 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.
> 
> I've reviewed the whole series and comments are mostly minor. As you're
> reminding me of the PXP, I'll take this as an opportunity to post
> patches that I've had in my tree for way too long :-) There will be
> minor conflicts with yours, so I'll first rebase them on this series,
> assuming that v2 will be similar in places where conflicts occur.

Thanks for the review! I will send a v2 to address your comments.

Thanks for your other patches as well. I will take a look and probably just
include them in my v2.

Michael

> 
> > Michael
> > 
> > [0] https://lore.kernel.org/linux-media/20200510223100.11641-1-laurent.pinchart@ideasonboard.com/
> > 
> > Michael Tretter (8):
> >   media: 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
> > 
> >  .../bindings/media/fsl,imx6ull-pxp.yaml       |  62 ++++++++
> >  .../devicetree/bindings/media/fsl-pxp.txt     |  26 ---
> >  arch/arm/boot/dts/imx7d.dtsi                  |   9 ++
> >  drivers/media/platform/nxp/imx-pxp.c          | 148 +++++++++++++++---
> >  4 files changed, 197 insertions(+), 48 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/media/fsl,imx6ull-pxp.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/media/fsl-pxp.txt