[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ca9e9a86-dbc1-4016-a71f-1f2980bd625f@nxp.com>
Date: Mon, 8 Sep 2025 15:45:48 +0200
From: Antoine Bouyer <antoine.bouyer@....com>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>
Cc: Dafna Hirschfeld <dafna@...tmail.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Keke Li <keke.li@...ogic.com>, Mauro Carvalho Chehab <mchehab@...nel.org>,
Heiko Stuebner <heiko@...ech.de>, Dan Scally <dan.scally@...asonboard.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v4 0/8] media: Introduce V4L2 extensible parameters
Hi Jacopo
On 05/09/2025 12:39, Jacopo Mondi wrote:
> Hello Antoine
>
> On Fri, Sep 05, 2025 at 12:14:39PM +0200, Antoine Bouyer wrote:
>> Hi Jacopo
>>
>> On 20/08/2025 14:58, Jacopo Mondi wrote:
>>> Extensible parameters meta formats have been introduced in the Linux
>>> kernel v6.12 initially to support different revision of the RkISP1 ISP
>>> implemented in different SoC. In order to avoid breaking userspace
>>> everytime an ISP configuration block is added or modified in the uAPI
>>> these new formats, which are versionated and extensible by their
>>> definition have been introduced.
>>>
>>> See for reference:
>>> e9d05e9d5db1 ("media: uapi: rkisp1-config: Add extensible params format")
>>> 6c53a7b68c5d ("media: rkisp1: Implement extensible params support")
>>>
>>> The Amlogic C3 ISP driver followed shortly, introducing an extensible
>>> format for the ISP configuration:
>>>
>>> 6d406187ebc0 ("media: uapi: Add stats info and parameters buffer for C3 ISP")
>>>
>>> with a very similar, if not identical, implementation of the routines to
>>> validate and handle the ISP configuration in the ISP driver in the
>>> c3-isp-params.c file.
>>>
>>> fb2e135208f3 ("media: platform: Add C3 ISP driver")
>>>
>>> With the recent upstreaming attempt of the Mali C55 ISP driver from Dan,
>>> a third user of extensible parameters is going to be itroduced in the
>>> kernel, duplicating again in the driver the procedure for validating and
>>> handling the ISP configuration blocks
>>>
>>> https://patchwork.linuxtv.org/project/linux-media/patch/20250624-c55-v10-15-54f3d4196990@ideasonboard.com/
>>>
>>> To avoid duplicating again the validation routines and common types
>>> definition, this series introduces v4l2-params.c/.h for the kAPI
>>> and v4l2-extensible-params.h for the uAPI and re-organize the RkISP1
>>> and Amlogic C3 drivers to use the common types and the helper validation
>>> routines.
>>>
>>
>> I'm currently working on another ISP driver: NXP neo, and also use extended
>> params so as RkISP1.
>>
>
> Nice to hear!
>
>> Using common types looks like really interesting approach. However, only
>> params are taken into account, while NXP neo also provides statistics. I'm
>> currently testing extended approach for both params AND statistics. But
>> v4l2-params only applies to .. params, as stated in the name.
>>
>> Is there any plan to apply the same to statistics ?
>>
>
> Not a proper plan at the moment, but I agree it would indeed be
> beneficial to have a similar approach for stats...
>
> Are you suggesting we should drop 'params' at least from the file
> names ?
Either create v4l2-extended-stats header, or a more generic name for
both params & stats. Seems Laurent already suggested v4l2-isp.h. I'm
fine with this too.
>
>>> If the here proposed approach is accepted, I propose to rebase the Mali
>>> C55 driver on top of this series, to use the new common types and
>>> helpers.
>>>
>>> I have been able to test this on RkISP1 but not on C3.
>>
>> Also tested on NXP neo locally.
>
> Great, could you maybe send a Tested-by tag ?
Actually, I further tested your libcamera patchs, but not yet the driver
ones. So indeed, I can send my Tested-By tag in libcamera series.
Best regards
Antoine
>
> Also, not sure if you have noticed this series for libcamera
> https://patchwork.libcamera.org/project/libcamera/list/?series=5413
> where I tried to generalize the extensible param helpers.
>
> Thanks
> j
>
>>
>> Best regards
>> Antoine
>>
>>>
>>> Thanks
>>> j
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@...asonboard.com>
>>> ---
>>> Changes in v4:
>>> - Fix the definition of V4L2_PARAMS_FL_PLATFORM_FLAGS
>>> - Add __counted_by() attribute to the data[] flexible-array member of
>>> v4l2_params_buffer
>>> - Minor style change
>>> - Link to v3: https://lore.kernel.org/r/20250819-extensible-parameters-validation-v3-0-9dc008348b30@ideasonboard.com
>>>
>>> Changes in v3:
>>> - Rebased on latest media-committers/next
>>> - Take in Dan's suggestion in block size validation
>>> - Documentation minor spelling fixes
>>> - Link to v2: https://lore.kernel.org/r/20250710-extensible-parameters-validation-v2-0-7ec8918ec443@ideasonboard.com
>>>
>>> Changes in v2:
>>> - Make v4l2_params_buffer directly usable
>>> - Centralize ENABLE/DISABLE flags definition and validation
>>> - Take in Dan's v4l2_params_buffer_size()
>>> - Allow blocks to only contain the header if they're going to be
>>> disabled
>>> - Documentation fixes as reported by Nicolas
>>> - Link to v1: https://lore.kernel.org/r/20250708-extensible-parameters-validation-v1-0-9fc27c9c728c@ideasonboard.com
>>>
>>> ---
>>> Jacopo Mondi (8):
>>> media: uapi: Introduce V4L2 extensible params
>>> media: uapi: Convert RkISP1 to V4L2 extensible params
>>> media: uapi: Convert Amlogic C3 to V4L2 extensible params
>>> media: Documentation: uapi: Add V4L2 extensible parameters
>>> media: v4l2-core: Introduce v4l2-params.c
>>> media: rkisp1: Use v4l2-params for validation
>>> media: amlogic-c3: Use v4l2-params for validation
>>> media: Documentation: kapi: Add v4l2 extensible parameters
>>>
>>> Documentation/driver-api/media/v4l2-core.rst | 1 +
>>> Documentation/driver-api/media/v4l2-params.rst | 5 +
>>> .../media/v4l/extensible-parameters.rst | 89 ++++++
>>> .../userspace-api/media/v4l/meta-formats.rst | 1 +
>>> MAINTAINERS | 10 +
>>> .../media/platform/amlogic/c3/isp/c3-isp-params.c | 263 ++++++----------
>>> .../media/platform/rockchip/rkisp1/rkisp1-params.c | 349 +++++++++------------
>>> drivers/media/v4l2-core/Makefile | 3 +-
>>> drivers/media/v4l2-core/v4l2-params.c | 126 ++++++++
>>> include/media/v4l2-params.h | 164 ++++++++++
>>> include/uapi/linux/media/amlogic/c3-isp-config.h | 48 +--
>>> include/uapi/linux/media/v4l2-extensible-params.h | 146 +++++++++
>>> include/uapi/linux/rkisp1-config.h | 67 ++--
>>> 13 files changed, 817 insertions(+), 455 deletions(-)
>>> ---
>>> base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
>>> change-id: 20250701-extensible-parameters-validation-c831f7f5cc0b
>>>
>>> Best regards,
Powered by blists - more mailing lists