Re: [PATCH 0/7] Introduce v4l2-isp reset buffers

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hi Daniel,

On 2026-11-04 12:28:15 +0000, Daniel Scally wrote:
> The V4L2 generic ISP parameters buffers described in
> Documentation/driver-api/media/v4l2-isp.rst has a problem in that if
> a driver using the parameters format is running a stream and the
> device is suspended to RAM and later resumed, userspace will not tend
> to reconfigure it with _all_ of the possible parameter blocks. The
> format allows only a subset to be populated into the buffer queued to
> the driver and this is the common practice in libcamera. The effect
> will be that a resumed device will have lost any configuration that
> tends to be sent only a single time at the start of a stream, for
> example lens shading table settings.
> 
> This series performs some preliminary cleanup and then adds the "reset
> buffer" concept to v4l2-isp. This is a mechanism through which user
> space can flag a struct v4l2_isp_block_header.flags as needing to be
> reapplied on device reset, such that the v4l2-isp framework will store
> that block into a buffer allocated for the purpose when it validates
> the block. 
> 
> The last patch in the series updates the rkisp1 driver to use the
> v4l2-isp reset buffer to reapply that saved configuration on resume.
> 
> An alternative approach might simply be to store the "last seen" copy
> of any block in the reset buffer - this would be a simple change.
> That implies a lot more processing on the majority of frames, as we'll
> then always copy at least something, but it reduces the risk of losing
> a setting. That amount of processing each frame seems very undesirable
> to me though, particularly given the scene will presumably have changed
> after resume and so the settings like AWB gains that are applied for
> most frames are probably out of date anyway. On the other hand we may
> end up flagging a lot of blocks, as there's parameters that might be
> changed mid-stream (like gamma) but would not be so regularly changed
> that we could just rely on them being sent naturally on resume, so a
> third approach might be to invert the meaning of the flag and store
> any block that _isn't_ flagged as _not_ needing to be reapplied.

Is there any reason not saving and restoring all relevant ISP registers 
directly in the suspend() / resume () callbacks? AFIK this is what most 
drivers do here and it's not clear to me why this would not work here?

> 
> I'm open to opinions on how best to set the balance!
> 
> Please note that the changes for the C3 and Dreamchip are only compile
> tested.
> 
> Thanks
> Dan
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Daniel Scally (7):
>       media: rppx1: Drop max_size param from rppx1_params()
>       media: v4l2-isp: Hold v4l2-isp data in central struct
>       media: v4l2-isp: Update users to initialise and validate new struct
>       media: v4l2-isp: Use centrally-held data in framework
>       media: v4l2-isp: Add v4l2_isp_params_for_each_block()
>       media: v4l2-isp: Introduce "reset buffer" concept
>       media: rkisp1: Re-apply v4l2-isp reset buffer on resume
> 
>  Documentation/userspace-api/media/v4l/v4l2-isp.rst |  19 ++++
>  .../media/platform/amlogic/c3/isp/c3-isp-common.h  |   2 +
>  .../media/platform/amlogic/c3/isp/c3-isp-params.c  |  28 +++--
>  .../media/platform/arm/mali-c55/mali-c55-common.h  |   1 +
>  .../media/platform/arm/mali-c55/mali-c55-params.c  |  28 ++---
>  .../media/platform/dreamchip/rppx1/rpp_params.c    |  30 +++--
>  drivers/media/platform/dreamchip/rppx1/rppx1.c     |   8 ++
>  drivers/media/platform/dreamchip/rppx1/rppx1.h     |   5 +
>  drivers/media/platform/renesas/rcar-isp/core-io.c  |   3 +-
>  .../media/platform/rockchip/rkisp1/rkisp1-common.h |   3 +
>  .../media/platform/rockchip/rkisp1/rkisp1-params.c |  61 ++++++----
>  drivers/media/v4l2-core/v4l2-isp.c                 | 126 +++++++++++++++++++--
>  include/media/rppx1.h                              |   2 +-
>  include/media/v4l2-isp.h                           | 109 ++++++++++++++----
>  include/uapi/linux/media/v4l2-isp.h                |   1 +
>  15 files changed, 331 insertions(+), 95 deletions(-)
> ---
> base-commit: 00178eda305a332f302882849ff7374d4169be8b
> change-id: 20261030-v4l2-isp-resume-41fad731df28
> prerequisite-change-id: 20261028-rkisp1-suspend-and-resume-7d4f388a9a61:v1
> prerequisite-patch-id: 8f6b499ef7898229c5b02e987822dd676a0cfd74
> prerequisite-patch-id: 56d0852983c6f46537c03ca3bec71e0ce4046431
> prerequisite-patch-id: 0b9edd21d24a25c620792dfa692d91346ceccdde
> prerequisite-patch-id: 3b9425028cd62b60b3dc3dbd2fc3b5a8912a1fb4
> prerequisite-patch-id: 771f986e2754383e7b5a920b25bf1609dc96f0b4
> prerequisite-patch-id: 79c95d6375d662a4f75cccd8650b3fab05e4e66c
> prerequisite-patch-id: e2ff2463db4934e707afbf0a239270d26dbf8189
> prerequisite-patch-id: 5a923b7606f7cc60fd6b75be2d73231470043bcb
> prerequisite-patch-id: fb80748a1a24316cdd77481b60936f716fd0aa71
> prerequisite-patch-id: 1450f7d1f4b1a41e7ce65ea6d2fbd450d3a198ad
> prerequisite-patch-id: d1d2ba9927536b110f7fbc0c68698d08afb74c27
> prerequisite-patch-id: c2ae6f91cb9662307f90070523945ee03a79c4c3
> prerequisite-patch-id: 87b425bde44fe889367a93302629dc011a3ec88a
> prerequisite-patch-id: 25e4c9ee767dc61406adcf1e1463796a79832795
> prerequisite-patch-id: 82388f762700dbcf39fdc43e6ba56c1feb55733a
> prerequisite-patch-id: a4ad2fb2c7f8659c6d9bdcd9718a7ee69f124838
> 
> Best regards,
> -- 
> Daniel Scally <dan.scally@ideasonboard.com>
> 

-- 
Kind Regards,
Niklas Söderlund