Re: [PATCH v2 00/30] media: atmel: atmel-isc: add support for xisc

[Hans Verkuil]

Hi Eugen,

On 05/04/2021 17:50, Eugen Hristev wrote:
> Hello,
> 
> This series adds support for a variant of the ISC named XISC.
> This block is present in the product named sama7g5.
> 
> I started by moving code around, the code which was specialized for sama5d2
> type of ISC, to have it inside the dedicated sama5d2 file.
> 
> I added several new pipeline elements to the code base, which would be common
> to sama5d2 and the new sama7g5, but only used by the new style pipeline.
> 
> I separated the input and output formats on a per-product separate array.
> 
> I added the new sama7g5 compatible driver for the xisc, which is similar with
> the sama5d2, but with differences in terms of DT, clocks and callbacks to
> specific operations.
> 
> I added the binding for the xisc by copying and modifying the existing
> isc one. I know that it has to be converted to yaml, and I will do that if
> it looks good.

This series looks good to me.

One thing that can be improved in a v3 is a short explanation of the various
abbreviations: CBV, RLP, HIS, DPC, etc.

Regards,

	Hans

> 
> Feedback is appreciated.
> Thanks,
> Eugen
> 
> Changes in v2:
> - Fixed krobot warnings with W=1 regarding functions with no prototype
> - Fixed new sama7g5 driver to use the new subdev fwnode API in kernel 5.12. my driver was
> based on old 5.10 style API.
> 
> Eugen Hristev (30):
>   media: atmel: atmel-isc: specialize gamma table into product specific
>   media: atmel: atmel-isc: specialize driver name constant
>   media: atmel: atmel-isc: add checks for limiting frame sizes
>   media: atmel: atmel-isc: specialize max width and max height
>   media: atmel: atmel-isc: specialize dma cfg
>   media: atmel: atmel-isc: extract CSC submodule config into separate
>     function
>   media: atmel: atmel-isc-base: add id to clock debug message
>   media: atmel: atmel-isc: create register offsets struct
>   media: atmel: atmel-isc: extract CBC submodule config into separate
>     function
>   media: atmel: atmel-isc: add CBC to the reg offsets struct
>   media: atmel: atmel-isc: add SUB422 and SUB420 to register offsets
>   media: atmel: atmel-isc: add RLP to register offsets
>   media: atmel: atmel-isc: add HIS to register offsets
>   media: atmel: atmel-isc: add DMA to register offsets
>   media: atmel: atmel-isc: add support for version register
>   media: atmel: atmel-isc: add his_entry to register offsets
>   media: atmel: atmel-isc: add register description for additional
>     modules
>   media: atmel: atmel-isc: extend pipeline with extra modules
>   media: atmel: atmel-isc: add CC initialization function
>   media: atmel: atmel-isc: create product specific v4l2 controls config
>   media: atmel: atmel-isc: create callback for DPC submodule product
>     specific
>   media: atmel: atmel-isc: create callback for GAM submodule product
>     specific
>   media: atmel: atmel-isc: create callback for RLP submodule product
>     specific
>   media: atmel: atmel-isc: move the formats list into product specific
>     code
>   media: atmel: atmel-isc: create an adapt pipeline callback for product
>     specific
>   media: atmel: atmel-isc-regs: add additional fields for sama7g5 type
>     pipeline
>   media: atmel: atmel-isc-base: add support for more formats and
>     additional pipeline modules
>   dt-bindings: media: atmel: add microchip-xisc binding
>   media: atmel: atmel-isc-sama5d2: remove duplicate define
>   media: atmel: atmel-isc: add microchip-xisc driver
> 
>  .../bindings/media/microchip-xisc.txt         |  64 ++
>  drivers/media/platform/Makefile               |   1 +
>  drivers/media/platform/atmel/Kconfig          |  11 +
>  drivers/media/platform/atmel/Makefile         |   2 +
>  drivers/media/platform/atmel/atmel-isc-base.c | 381 ++++-------
>  drivers/media/platform/atmel/atmel-isc-regs.h | 133 +++-
>  drivers/media/platform/atmel/atmel-isc.h      | 122 +++-
>  .../media/platform/atmel/atmel-sama5d2-isc.c  | 311 ++++++++-
>  .../media/platform/atmel/atmel-sama7g5-isc.c  | 643 ++++++++++++++++++
>  9 files changed, 1391 insertions(+), 277 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/microchip-xisc.txt
>  create mode 100644 drivers/media/platform/atmel/atmel-sama7g5-isc.c
>