Re: [PATCH v7 0/12] media: rkisp1: Extensible parameters and companding

[Kieran Bingham <kieran.bingham@ideasonboard.com>]

Hi Jacopo,

For this series,

Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

on a Debix-SOM with both an IMX335 and IMX283. And also in particular, I
can confirm the Gamma control is now functional in v7 \o/ Thanks.

--
Kieran


Quoting Jacopo Mondi (2024-07-24 09:49:51)
> v6->v7:
> - Collect [PATCH v2 0/5] media: rkisp1: Add support for the companding block
> - Fix GOC handling
> - Fix newly introduced errors in checkstyle and documentation reported by
>   https://gitlab.freedesktop.org/linux-media/users/jmondi/-/pipelines/1231492
> 
> v5->v6:
> - Collect [v5.2 6/7] from Laurent
> - Collect Paul's tags
> - Add extra validation for unexpected data after the payload end as
>   suggested by Sakari
> 
> v4->v5:
> - Refine validation of the ext params buffer following Laurent's suggestion
> - perform memcpy of the parameters buffer after sizes validation
> 
> v3->v4:
> - Introduce 'union rkisp1_ext_params_config' to avoid casts in the block
>   handlers
> 
> v2->v3:
> - Address Laurent's comments on the uAPI:
>   - rename $block_config with  just 'config'
>   - reduce header size
>   - rename a few fields/blocks
> 
> - Address Laurent's comment on the params node:
>   - Use the plane payload for memcpy() and buffer validation
>   - drop buf_out_validate() and use buf_prepare() only
>   - validate the total buffer size against the buffer payload
>   - use const pointers where possible
> 
> v1->v2:
> - re-order patches to introduce parameters buffer caching for the existing
>   "fixed" format before introducing the "extensible" format
> - align all structures to 64-bit boundaries in the uAPI
> - remove NO_CHANGE enablement state and cache a bitmask of enabled blocks
> - address review comments in documentation
> 
> The VeriSilicon ISP8000 IP, supported through the rkisp1 driver in the Linux
> kernel, is integrated in several SoCs from different vendors. Different
> revisions of the IP differ in the number of supported features and in the
> register space location assigned to specific ISP blocks.
> 
> The current configuration parameters format, defined in
> include/uapi/linux/rkisp1-config.h is realized by a C structure (struct
> rkisp1_params_cfg) which wraps other structures that allows to configure
> specific ISP blocks. The layout of the parameters buffer is part of the Linux
> kernel uAPI and can hardly be extended or modified to adapt it to different
> revisions of the same IP.
> 
> This series proposes the introduction of a new parameters format for the RkISP1
> (without dropping support for the existing one) which is designed with the goals
> of being:
> 
> 1) versioned: can be changed without breaking existing applications
> 2) extensible: new blocks and parameters can be added without breaking the uABI
> 
> To do so, a new 'struct rkisp1_ext_params_cfg' type is introduced. It wraps an
> header and a data buffer, which userspace fills with configuration blocks
> for each ISP block it intends to configure. The parameters buffer is thus of
> different effective sizes, depending on the number of blocks userspace intends
> to configure.
> 
> The kernel driver parses the data block and decides, based on the versioning
> number and the platform it operates on, how to handle each block.
> 
> The parameters format is very similar to the parameters format implemented
> in the in-review Mali C55 ISP driver [1]
> 
> CI pipeline [2]
> 
> [1] https://lore.kernel.org/linux-media/20240529152858.183799-15-dan.scally@ideasonboard.com/
> [2] https://gitlab.freedesktop.org/linux-media/users/jmondi/-/pipelines/1231500
> 
> Jacopo Mondi (7):
>   media: uapi: rkisp1-config: Add extensible params format
>   media: uapi: videodev2: Add V4L2_META_FMT_RK_ISP1_EXT_PARAMS
>   media: rkisp1: Add struct rkisp1_params_buffer
>   media: rkisp1: Copy the parameters buffer
>   media: rkisp1: Cache the currently active format
>   media: rkisp1: Implement extensible params support
>   media: rkisp1: Implement s_fmt/try_fmt
> 
> Laurent Pinchart (2):
>   media: rkisp1: Add helper function to swap colour channels
>   media: rkisp1: Add features mask to extensible block handlers
> 
> Paul Elder (3):
>   media: rkisp1: Add register definitions for the companding block
>   media: rkisp1: Add feature flags for BLS and compand
>   media: rkisp1: Add support for the companding block
> 
>  Documentation/admin-guide/media/rkisp1.rst    |   11 +-
>  .../media/v4l/metafmt-rkisp1.rst              |   57 +-
>  .../platform/rockchip/rkisp1/rkisp1-common.c  |   14 +
>  .../platform/rockchip/rkisp1/rkisp1-common.h  |   38 +-
>  .../platform/rockchip/rkisp1/rkisp1-dev.c     |    9 +-
>  .../platform/rockchip/rkisp1/rkisp1-params.c  | 1016 +++++++++++++++--
>  .../platform/rockchip/rkisp1/rkisp1-regs.h    |   23 +
>  .../platform/rockchip/rkisp1/rkisp1-stats.c   |   51 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
>  include/uapi/linux/rkisp1-config.h            |  576 ++++++++++
>  include/uapi/linux/videodev2.h                |    1 +
>  11 files changed, 1640 insertions(+), 157 deletions(-)
> 
> --
> 2.45.2
>