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

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

[Daniel Scally]

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.

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>

[PATCH 1/7] media: rppx1: Drop max_size param from rppx1_params()

[Daniel Scally]

The maximum size of the buffer is currently passed in from the caller
but is available in rppx1-config.h. Include the header and drop the
parameter from the function.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---

I hope this change is ok; I'm not familiar with future plans for the
library driver, but it looks like the parameters max size should be
the same for any user of the library and that not parametising it is
ok.
---
 drivers/media/platform/dreamchip/rppx1/rpp_params.c | 7 +++++--
 drivers/media/platform/renesas/rcar-isp/core-io.c   | 3 +--
 include/media/rppx1.h                               | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index 5e6727d58946405322a8dae3c8d5727fe07135e0..e804440f369b7bd6d85cc1a23a2610c1cca307fa 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -5,6 +5,8 @@
  * Copyright (C) 2026 Ragnatech AB
  */
 
+#include <linux/media/dreamchip/rppx1-config.h>
+
 #include <media/v4l2-isp.h>
 #include <media/videobuf2-v4l2.h>
 
@@ -37,7 +39,7 @@ rppx1_ext_params_blocks_info[] = {
 	RPPX1_PARAMS_BLOCK_INFO(GA_MV, ga),
 };
 
-int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
+int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 		 rppx1_reg_write write, void *priv)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -45,7 +47,8 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
 	size_t block_offset;
 	int ret;
 
-	ret = v4l2_isp_params_validate_buffer_size(rpp->dev, vb, max_size);
+	ret = v4l2_isp_params_validate_buffer_size(rpp->dev, vb,
+				v4l2_isp_buffer_size(RPPX1_PARAMS_MAX_SIZE));
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/renesas/rcar-isp/core-io.c b/drivers/media/platform/renesas/rcar-isp/core-io.c
index b60b91f43d42deed16c8feb3526bbb77a43b5b96..5eea8c721a25a01cc6c01ab49fef001ae2065a6b 100644
--- a/drivers/media/platform/renesas/rcar-isp/core-io.c
+++ b/drivers/media/platform/renesas/rcar-isp/core-io.c
@@ -536,8 +536,7 @@ static int risp_io_params_buffer_prepare(struct vb2_buffer *vb)
 	desc.count = 0;
 
 	/* Fill params body. */
-	ret = rppx1_params(io->core->rpp, vb, io->format.fmt.meta.buffersize,
-			   risp_conf_dma_prepare, &desc);
+	ret = rppx1_params(io->core->rpp, vb, risp_conf_dma_prepare, &desc);
 	if (ret)
 		return ret;
 
diff --git a/include/media/rppx1.h b/include/media/rppx1.h
index 2e761046db384a8dd17b1c66bb62c5ac14dec9a9..bb5f91f3174df5e29988a4ec3fb1d29f9db01ca6 100644
--- a/include/media/rppx1.h
+++ b/include/media/rppx1.h
@@ -28,7 +28,7 @@ int rppx1_stop(struct rppx1 *rpp);
 bool rppx1_interrupt(struct rppx1 *rpp, u32 *isc);
 
 typedef int (*rppx1_reg_write)(void *priv, u32 offset, u32 value);
-int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb, size_t max_size,
+int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 		 rppx1_reg_write write, void *priv);
 
 void rppx1_stats_fill_isr(struct rppx1 *rpp, u32 isc, void *buf);

-- 
2.43.0

[PATCH 2/7] media: v4l2-isp: Hold v4l2-isp data in central struct

[Daniel Scally]

Hold v4l2-isp related data in a new struct v4l2_isp to facilitate the
easier use of driver-defined data in v4l2-isp itself. Provide a
function to validate the driver's definition of data to make sure that
we have everything that we need.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-isp.c | 41 ++++++++++++++++++++
 include/media/v4l2-isp.h           | 79 ++++++++++++++++++++++++++++----------
 2 files changed, 99 insertions(+), 21 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 8e6c2ef326aaa63e00332bcdac8ebdf6da059f44..a979c0690d19d9956a45657561b510546b3aa8c2 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -13,6 +13,47 @@
 
 #include <media/videobuf2-core.h>
 
+int v4l2_isp_validate(struct v4l2_isp *isp)
+{
+	/* We require a device pointer for debugging purposes */
+	if (!isp->dev)
+		return -EINVAL;
+
+	/*
+	 * We need to know how big the data member of the params buffer can get
+	 */
+	if (!isp->params.max_size) {
+		dev_err(isp->dev, "Missing parameters buffer size data\n");
+		return -EINVAL;
+	}
+
+	/* We need to know how many block types there are */
+	if (!isp->params.num_block_types) {
+		dev_err(isp->dev, "Missing number of block types\n");
+		return -EINVAL;
+	}
+
+	/* We need a pointer to some &struct v4l2_isp_params_block_type_info */
+	if (!isp->params.types_info) {
+		dev_err(isp->dev, "Missing block types info pointer\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * A block type with a size of zero is inherently invalid, so let's
+	 * catch that early.
+	 */
+	for (unsigned int i = 0; i < isp->params.num_block_types; i++) {
+		if (!isp->params.types_info[i].size) {
+			dev_err(isp->dev, "Found block type info with 0 size\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_isp_validate);
+
 int v4l2_isp_params_validate_buffer_size(struct device *dev,
 					 struct vb2_buffer *vb,
 					 size_t max_size)
diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
index 7a54cf98c79af7a621fdc1f7098690ec7ccaa267..dc3e612ef79ea510ec1535f483fc16c6ec3d4569 100644
--- a/include/media/v4l2-isp.h
+++ b/include/media/v4l2-isp.h
@@ -31,27 +31,6 @@ struct vb2_buffer;
 #define v4l2_isp_buffer_size(max_size)			\
 	(offsetof(struct v4l2_isp_buffer, data) + (max_size))
 
-/**
- * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
- * @dev: the driver's device pointer
- * @vb: the videobuf2 buffer
- * @max_size: the maximum allowed buffer size
- *
- * This function performs validation of the size of a V4L2 ISP parameters buffer
- * before the driver can access the actual data buffer content.
- *
- * After the sizes validation, drivers should copy the buffer content to a
- * kernel-only memory area to prevent userspace from modifying it,
- * before completing validation using v4l2_isp_params_validate_buffer().
- *
- * The @vb buffer as received from the vb2 .buf_prepare() operation is checked
- * against @max_size and it's validated to be large enough to accommodate at
- * least one ISP configuration block.
- */
-int v4l2_isp_params_validate_buffer_size(struct device *dev,
-					 struct vb2_buffer *vb,
-					 size_t max_size);
-
 /**
  * struct v4l2_isp_params_block_type_info - V4L2 ISP params per-block-type info
  * @size: the block type expected size
@@ -73,6 +52,64 @@ struct v4l2_isp_params_block_type_info {
 			      const struct v4l2_isp_block_header *block);
 };
 
+/**
+ * struct v4l2_isp - V4L2 ISP processing structure
+ * @dev: pointer to the &struct device associated with the ISP
+ * @params: sub-struct holding data related to the V4L2 ISP parameters
+ * @params.types_info: pointer to array of params block information structs
+ * @params.num_block_types: number of possible params block types
+ * @params.max_size: maximum size in bytes of the data member of
+ *		     &struct v4l2_isp_params_buffer
+ *
+ * The V4L2 ISP framework is designed to ease validation of parameter and
+ * statistic buffers queued to ISP device drivers. This struct holds references
+ * to driver-defined information designed to facilitate easy validation in a
+ * central source file. The intention is for an instance of the struct to be
+ * embedded within a driver specific struct and populated before validation and
+ * use through the v4l2-isp framework.
+ */
+struct v4l2_isp {
+	struct device *dev;
+
+	struct {
+		const struct v4l2_isp_params_block_type_info *types_info;
+		size_t num_block_types;
+		size_t max_size;
+	} params;
+};
+
+/**
+ * v4l2_isp_validate - Set up a &struct v4l2_isp properly
+ * @isp: pointer to the &struct v4l2_isp to initialise
+ *
+ * This function validates that the members of @isp have been populated with
+ * information sufficient to allow it to be used with the v4l2-isp framework.
+ *
+ * @return int
+ */
+int v4l2_isp_validate(struct v4l2_isp *isp);
+
+/**
+ * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
+ * @dev: the driver's device pointer
+ * @vb: the videobuf2 buffer
+ * @max_size: the maximum allowed buffer size
+ *
+ * This function performs validation of the size of a V4L2 ISP parameters buffer
+ * before the driver can access the actual data buffer content.
+ *
+ * After the sizes validation, drivers should copy the buffer content to a
+ * kernel-only memory area to prevent userspace from modifying it,
+ * before completing validation using v4l2_isp_params_validate_buffer().
+ *
+ * The @vb buffer as received from the vb2 .buf_prepare() operation is checked
+ * against @max_size and it's validated to be large enough to accommodate at
+ * least one ISP configuration block.
+ */
+int v4l2_isp_params_validate_buffer_size(struct device *dev,
+					 struct vb2_buffer *vb,
+					 size_t max_size);
+
 /**
  * v4l2_isp_params_validate_buffer - Validate a V4L2 ISP parameters buffer
  * @dev: the driver's device pointer

-- 
2.43.0

[PATCH 3/7] media: v4l2-isp: Update users to initialise and validate new struct

[Daniel Scally]

Update users of the v4l2-isp framework to initialise and validate the
new struct v4l2_isp. This has no practical effect for now, but will
reduce the churn in follow-on patches.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/platform/amlogic/c3/isp/c3-isp-common.h  |  2 ++
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c  | 10 ++++++++++
 drivers/media/platform/arm/mali-c55/mali-c55-common.h  |  1 +
 drivers/media/platform/arm/mali-c55/mali-c55-params.c  |  9 +++++++++
 drivers/media/platform/dreamchip/rppx1/rpp_params.c    | 10 ++++++++++
 drivers/media/platform/dreamchip/rppx1/rppx1.c         |  8 ++++++++
 drivers/media/platform/dreamchip/rppx1/rppx1.h         |  5 +++++
 drivers/media/platform/rockchip/rkisp1/rkisp1-common.h |  3 +++
 drivers/media/platform/rockchip/rkisp1/rkisp1-params.c |  9 +++++++++
 9 files changed, 57 insertions(+)

diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-common.h b/drivers/media/platform/amlogic/c3/isp/c3-isp-common.h
index cb470802e61e85426c5a41150372753a405a2d32..222c2e91ce429904a50a702a9d38f286dfd7c5a6 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-common.h
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-common.h
@@ -11,6 +11,7 @@
 #include <media/media-device.h>
 #include <media/videobuf2-core.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-isp.h>
 #include <media/v4l2-subdev.h>
 #include <media/videobuf2-v4l2.h>
 
@@ -222,6 +223,7 @@ struct c3_isp_params {
 	struct video_device vdev;
 	struct v4l2_format vfmt;
 	struct media_pad pad;
+	struct v4l2_isp v4l2_isp;
 
 	struct mutex lock; /* Protects vb2_q, vdev */
 	struct c3_isp_device *isp;
diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
index ae0777a20bdaff822657c310e20b2011fc1e8ff2..25f6a7d9bad9d61d641690f43faceaae0f938da5 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
@@ -883,6 +883,16 @@ int c3_isp_params_register(struct c3_isp_device *isp)
 	params->vfmt.fmt.meta.dataformat = V4L2_META_FMT_C3ISP_PARAMS;
 	params->vfmt.fmt.meta.buffersize = sizeof(struct c3_isp_params_cfg);
 	params->isp = isp;
+
+	params->v4l2_isp.dev = isp->dev;
+	params->v4l2_isp.params.max_size = sizeof(struct c3_isp_params_cfg);
+	params->v4l2_isp.params.num_block_types = ARRAY_SIZE(c3_isp_params_block_types_info);
+	params->v4l2_isp.params.types_info = c3_isp_params_block_types_info;
+
+	ret = v4l2_isp_validate(&params->v4l2_isp);
+	if (ret)
+		return ret;
+
 	INIT_LIST_HEAD(&params->pending);
 	spin_lock_init(&params->buff_lock);
 	mutex_init(&params->lock);
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-common.h b/drivers/media/platform/arm/mali-c55/mali-c55-common.h
index 13a3e9dc424311869af581061190bf38c08fb469..c4cd200b89d12a539adb55a02a936280490dfca3 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-common.h
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-common.h
@@ -196,6 +196,7 @@ struct mali_c55_stats {
 
 struct mali_c55_params {
 	struct mali_c55 *mali_c55;
+	struct v4l2_isp v4l2_isp;
 	struct video_device vdev;
 	struct vb2_queue queue;
 	struct media_pad pad;
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index 16e895e3488ecc3ddb3808281a309901fd44103d..06509152a51a1d47355d3c7c9985112ce3c20450 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -1010,6 +1010,15 @@ int mali_c55_register_params(struct mali_c55 *mali_c55)
 	struct vb2_queue *vb2q = &params->queue;
 	int ret;
 
+	params->v4l2_isp.dev = mali_c55->dev;
+	params->v4l2_isp.params.max_size = v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE);
+	params->v4l2_isp.params.num_block_types = ARRAY_SIZE(mali_c55_params_block_types_info);
+	params->v4l2_isp.params.types_info = mali_c55_params_block_types_info;
+
+	ret = v4l2_isp_validate(&params->v4l2_isp);
+	if (ret)
+		return ret;
+
 	mutex_init(&params->lock);
 	INIT_LIST_HEAD(&params->buffers.queue);
 	spin_lock_init(&params->buffers.lock);
diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index e804440f369b7bd6d85cc1a23a2610c1cca307fa..5b8b471198ec1d540f31c0ac1b15d0d54bd808fc 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -39,6 +39,16 @@ rppx1_ext_params_blocks_info[] = {
 	RPPX1_PARAMS_BLOCK_INFO(GA_MV, ga),
 };
 
+int rppx1_params_init(struct rppx1 *rpp)
+{
+	rpp->isp.dev = rpp->dev;
+	rpp->isp.params.max_size = v4l2_isp_buffer_size(RPPX1_PARAMS_MAX_SIZE);
+	rpp->isp.params.num_block_types = ARRAY_SIZE(rppx1_ext_params_blocks_info);
+	rpp->isp.params.types_info = rppx1_ext_params_blocks_info;
+
+	return v4l2_isp_validate(&rpp->isp);
+}
+
 int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 		 rppx1_reg_write write, void *priv)
 {
diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.c b/drivers/media/platform/dreamchip/rppx1/rppx1.c
index c502be797914653d1fe6e69d6065995742255d6b..bbeb94099395edffc4b7a0ce741523a05e23b858 100644
--- a/drivers/media/platform/dreamchip/rppx1/rppx1.c
+++ b/drivers/media/platform/dreamchip/rppx1/rppx1.c
@@ -153,6 +153,7 @@ EXPORT_SYMBOL_GPL(rppx1_destroy);
 struct rppx1 *rppx1_create(void __iomem *base, struct device *dev)
 {
 	struct rppx1 *rpp;
+	int ret;
 	u32 reg;
 
 	/* Allocate library structure */
@@ -163,6 +164,13 @@ struct rppx1 *rppx1_create(void __iomem *base, struct device *dev)
 	rpp->base = base;
 	rpp->dev = dev;
 
+	ret = rppx1_params_init(rpp);
+	if (ret) {
+		dev_err(dev, "failed to initialise V4L2 ISP\n");
+		rppx1_destroy(rpp);
+		return NULL;
+	}
+
 	/* Check communication with RPP and verify it truly is a X1. */
 	reg = rppx1_read(rpp, RPPX1_RPP_HDRREGS_VERSION_REG);
 	if (reg != 3) {
diff --git a/drivers/media/platform/dreamchip/rppx1/rppx1.h b/drivers/media/platform/dreamchip/rppx1/rppx1.h
index 3bf3955afbaedeaaacdf83802d8f7af7368f039f..28dc2740509d9d89fad321ea2461368685ca154c 100644
--- a/drivers/media/platform/dreamchip/rppx1/rppx1.h
+++ b/drivers/media/platform/dreamchip/rppx1/rppx1.h
@@ -11,6 +11,8 @@
 #include <linux/device.h>
 #include <linux/types.h>
 
+#include <media/v4l2-isp.h>
+
 #include "rpp_module.h"
 
 #define RPPX1_IRQ_ID_256HIST			BIT(27)
@@ -97,9 +99,12 @@ struct rppx1 {
 
 	struct rpp_module rmap;
 	struct rpp_module rmapmeas;
+
+	struct v4l2_isp isp;
 };
 
 void rppx1_write(struct rppx1 *rpp, u32 offset, u32 value);
 u32 rppx1_read(struct rppx1 *rpp, u32 offset);
+int rppx1_params_init(struct rppx1 *rpp);
 
 #endif /* __MEDIA_RPPX1_H__ */
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 94f03310e92f110062313bf2a34afb0692d9122c..2e75372727a86dbffcfaa0666e02e201ef260395 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -19,6 +19,7 @@
 #include <media/media-entity.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-isp.h>
 #include <media/videobuf2-v4l2.h>
 
 #include "rkisp1-regs.h"
@@ -410,6 +411,7 @@ struct rkisp1_params_ops {
  * @buf.curr:		pointer to the current rkisp1_params_buffer
  * @config_lock:	locks the buffer list 'params'
  * @params:		queue of rkisp1_buffer
+ * @v4l2_isp:		pointer to the v4l2-isp central struct
  * @metafmt		the currently enabled metadata format
  * @quantization:	the quantization configured on the isp's src pad
  * @ycbcr_encoding	the YCbCr encoding
@@ -428,6 +430,7 @@ struct rkisp1_params {
 	} buf;
 
 	struct v4l2_ctrl_handler ctrls;
+	struct v4l2_isp v4l2_isp;
 
 	const struct v4l2_meta_format *metafmt;
 
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index f440e15afc74c029510e0777a9ec2f6ea4d5be88..756332d8bd154c3175ad5d563f6e5cf615f42499 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -2887,6 +2887,15 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1)
 	int ret;
 
 	params->rkisp1 = rkisp1;
+	params->v4l2_isp.dev = rkisp1->dev;
+	params->v4l2_isp.params.max_size = sizeof(struct rkisp1_ext_params_cfg);
+	params->v4l2_isp.params.num_block_types = ARRAY_SIZE(rkisp1_ext_params_block_types_info);
+	params->v4l2_isp.params.types_info = rkisp1_ext_params_block_types_info;
+
+	ret = v4l2_isp_validate(&params->v4l2_isp);
+	if (ret)
+		return ret;
+
 	mutex_init(&node->vlock);
 	INIT_LIST_HEAD(&params->buf.queue);
 	spin_lock_init(&params->buf.lock);

-- 
2.43.0

[PATCH 4/7] media: v4l2-isp: Use centrally-held data in framework

[Daniel Scally]

Update the framework's APIs to use the centrally held data rather
than require users to pass data in to each function call.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c |  9 +++------
 drivers/media/platform/arm/mali-c55/mali-c55-params.c |  8 ++------
 drivers/media/platform/dreamchip/rppx1/rpp_params.c   |  7 ++-----
 .../media/platform/rockchip/rkisp1/rkisp1-params.c    |  9 +++------
 drivers/media/v4l2-core/v4l2-isp.c                    | 19 +++++++++----------
 include/media/v4l2-isp.h                              | 18 ++++++------------
 6 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
index 25f6a7d9bad9d61d641690f43faceaae0f938da5..f0b330520f97ad3bb701de3aed35670158508829 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
@@ -809,8 +809,7 @@ static int c3_isp_params_vb2_buf_prepare(struct vb2_buffer *vb)
 	struct c3_isp_params_cfg *cfg = buf->cfg;
 	int ret;
 
-	ret = v4l2_isp_params_validate_buffer_size(params->isp->dev, vb,
-						   params->vfmt.fmt.meta.buffersize);
+	ret = v4l2_isp_params_validate_buffer_size(&params->v4l2_isp, vb);
 	if (ret)
 		return ret;
 
@@ -820,10 +819,8 @@ static int c3_isp_params_vb2_buf_prepare(struct vb2_buffer *vb)
 	 */
 	memcpy(cfg, usr_cfg, payload_size);
 
-	return v4l2_isp_params_validate_buffer(params->isp->dev, vb,
-					(struct v4l2_isp_params_buffer *)cfg,
-					c3_isp_params_block_types_info,
-					ARRAY_SIZE(c3_isp_params_block_types_info));
+	return v4l2_isp_params_validate_buffer(&params->v4l2_isp, vb,
+					(struct v4l2_isp_params_buffer *)cfg);
 }
 
 static int c3_isp_params_vb2_buf_init(struct vb2_buffer *vb)
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index 06509152a51a1d47355d3c7c9985112ce3c20450..7b9d2650ce8730ccc05eee0c9dc33a88d022acf1 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -707,14 +707,12 @@ static int mali_c55_params_buf_prepare(struct vb2_buffer *vb)
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
 	struct mali_c55_params_buf *buf = to_mali_c55_params_buf(vbuf);
 	struct v4l2_isp_params_buffer *config = vb2_plane_vaddr(vb, 0);
-	struct mali_c55 *mali_c55 = params->mali_c55;
 	int ret;
 
 	if (!config)
 		return -EFAULT;
 
-	ret = v4l2_isp_params_validate_buffer_size(mali_c55->dev, vb,
-			v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
+	ret = v4l2_isp_params_validate_buffer_size(&params->v4l2_isp, vb);
 	if (ret)
 		return ret;
 
@@ -726,9 +724,7 @@ static int mali_c55_params_buf_prepare(struct vb2_buffer *vb)
 
 	memcpy(buf->config, config, v4l2_isp_buffer_size(MALI_C55_PARAMS_MAX_SIZE));
 
-	return v4l2_isp_params_validate_buffer(mali_c55->dev, vb, buf->config,
-					       mali_c55_params_block_types_info,
-					       ARRAY_SIZE(mali_c55_params_block_types_info));
+	return v4l2_isp_params_validate_buffer(&params->v4l2_isp, vb, buf->config);
 }
 
 static void mali_c55_params_buf_queue(struct vb2_buffer *vb)
diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index 5b8b471198ec1d540f31c0ac1b15d0d54bd808fc..aff3f3a997b4134b753fd1e1faf2058216f4b939 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -57,16 +57,13 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 	size_t block_offset;
 	int ret;
 
-	ret = v4l2_isp_params_validate_buffer_size(rpp->dev, vb,
-				v4l2_isp_buffer_size(RPPX1_PARAMS_MAX_SIZE));
+	ret = v4l2_isp_params_validate_buffer_size(&rpp->isp, vb);
 	if (ret)
 		return ret;
 
 	cfg = vb2_plane_vaddr(&vbuf->vb2_buf, 0);
 
-	ret = v4l2_isp_params_validate_buffer(rpp->dev, vb, cfg,
-					      rppx1_ext_params_blocks_info,
-					      ARRAY_SIZE(rppx1_ext_params_blocks_info));
+	ret = v4l2_isp_params_validate_buffer(&rpp->isp, vb, cfg);
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index 756332d8bd154c3175ad5d563f6e5cf615f42499..8974c520d1879f3f0f642819a41984f6ac3ee01d 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -2735,8 +2735,7 @@ static int rkisp1_params_prepare_ext_params(struct rkisp1_params *params,
 		vb2_plane_vaddr(&vbuf->vb2_buf, 0);
 	int ret;
 
-	ret = v4l2_isp_params_validate_buffer_size(params->rkisp1->dev, vb,
-						   params->metafmt->buffersize);
+	ret = v4l2_isp_params_validate_buffer_size(&params->v4l2_isp, vb);
 	if (ret)
 		return ret;
 
@@ -2746,10 +2745,8 @@ static int rkisp1_params_prepare_ext_params(struct rkisp1_params *params,
 	 */
 	memcpy(cfg, usr_cfg, payload_size);
 
-	return v4l2_isp_params_validate_buffer(params->rkisp1->dev, vb,
-				(struct v4l2_isp_params_buffer *)cfg,
-				rkisp1_ext_params_block_types_info,
-				ARRAY_SIZE(rkisp1_ext_params_block_types_info));
+	return v4l2_isp_params_validate_buffer(&params->v4l2_isp, vb,
+				(struct v4l2_isp_params_buffer *)cfg);
 }
 
 static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index a979c0690d19d9956a45657561b510546b3aa8c2..925632e1037db18f0948ceb0ce7c652e167516c9 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -54,15 +54,15 @@ int v4l2_isp_validate(struct v4l2_isp *isp)
 }
 EXPORT_SYMBOL_GPL(v4l2_isp_validate);
 
-int v4l2_isp_params_validate_buffer_size(struct device *dev,
-					 struct vb2_buffer *vb,
-					 size_t max_size)
+int v4l2_isp_params_validate_buffer_size(struct v4l2_isp *isp,
+					 struct vb2_buffer *vb)
 {
 	size_t header_size = offsetof(struct v4l2_isp_params_buffer, data);
 	size_t payload_size = vb2_get_plane_payload(vb, 0);
+	struct device *dev = isp->dev;
 
 	/* Payload size can't be greater than the destination buffer size */
-	if (payload_size > max_size) {
+	if (payload_size > isp->params.max_size) {
 		dev_dbg(dev, "Payload size is too large: %zu\n", payload_size);
 		return -EINVAL;
 	}
@@ -77,13 +77,12 @@ int v4l2_isp_params_validate_buffer_size(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(v4l2_isp_params_validate_buffer_size);
 
-int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
-				    const struct v4l2_isp_params_buffer *buffer,
-				    const struct v4l2_isp_params_block_type_info *type_info,
-				    size_t num_block_types)
+int v4l2_isp_params_validate_buffer(struct v4l2_isp *isp, struct vb2_buffer *vb,
+				    const struct v4l2_isp_params_buffer *buffer)
 {
 	size_t header_size = offsetof(struct v4l2_isp_params_buffer, data);
 	size_t payload_size = vb2_get_plane_payload(vb, 0);
+	struct device *dev = isp->dev;
 	size_t block_offset = 0;
 	size_t buffer_size;
 
@@ -118,7 +117,7 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 		block = (const struct v4l2_isp_params_block_header *)
 			(buffer->data + block_offset);
 
-		if (block->type >= num_block_types) {
+		if (block->type >= isp->params.num_block_types) {
 			dev_dbg(dev,
 				"Invalid block type %u at offset %zu\n",
 				block->type, block_offset);
@@ -147,7 +146,7 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 		 * with data that a later kernel, once it implements the type,
 		 * would validate and possibly reject.
 		 */
-		info = &type_info[block->type];
+		info = &isp->params.types_info[block->type];
 		if (!info->size) {
 			dev_dbg(dev, "Unsupported block type %u at offset %zu\n",
 				block->type, block_offset);
diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
index dc3e612ef79ea510ec1535f483fc16c6ec3d4569..b758d9b0e7f5c29cd8c4197f4a4a86cf3e5334fc 100644
--- a/include/media/v4l2-isp.h
+++ b/include/media/v4l2-isp.h
@@ -91,9 +91,8 @@ int v4l2_isp_validate(struct v4l2_isp *isp);
 
 /**
  * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
- * @dev: the driver's device pointer
+ * @isp: the driver's &struct v4l2_isp pointer
  * @vb: the videobuf2 buffer
- * @max_size: the maximum allowed buffer size
  *
  * This function performs validation of the size of a V4L2 ISP parameters buffer
  * before the driver can access the actual data buffer content.
@@ -106,17 +105,14 @@ int v4l2_isp_validate(struct v4l2_isp *isp);
  * against @max_size and it's validated to be large enough to accommodate at
  * least one ISP configuration block.
  */
-int v4l2_isp_params_validate_buffer_size(struct device *dev,
-					 struct vb2_buffer *vb,
-					 size_t max_size);
+int v4l2_isp_params_validate_buffer_size(struct v4l2_isp *isp,
+					 struct vb2_buffer *vb);
 
 /**
  * v4l2_isp_params_validate_buffer - Validate a V4L2 ISP parameters buffer
- * @dev: the driver's device pointer
+ * @isp: the driver's &struct v4l2_isp pointer
  * @vb: the videobuf2 buffer
  * @buffer: the V4L2 ISP parameters buffer
- * @type_info: the array of per-block-type validation info
- * @num_block_types: the number of block types in the type_info array
  *
  * This function completes the validation of a V4L2 ISP parameters buffer,
  * verifying each configuration block correctness before the driver can use
@@ -129,10 +125,8 @@ int v4l2_isp_params_validate_buffer_size(struct device *dev,
  * to prevent userspace from modifying its content after it has been submitted
  * to the driver, and then call this function to complete validation.
  */
-int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
-				    const struct v4l2_isp_params_buffer *buffer,
-				    const struct v4l2_isp_params_block_type_info *type_info,
-				    size_t num_block_types);
+int v4l2_isp_params_validate_buffer(struct v4l2_isp *isp, struct vb2_buffer *vb,
+				    const struct v4l2_isp_params_buffer *buffer);
 
 /**
  * struct v4l2_isp_stats_block_type_info - V4L2 ISP stats per-block-type info

-- 
2.43.0

[PATCH 5/7] media: v4l2-isp: Add v4l2_isp_params_for_each_block()

[Daniel Scally]

ISP drivers using the V4L2 extensible parameters need to iterate
through parameter blocks in a buffer. Currently they all open code
this operation. Add a macro that can be used instead.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c  |  9 +++------
 drivers/media/platform/arm/mali-c55/mali-c55-params.c  | 11 +++--------
 drivers/media/platform/dreamchip/rppx1/rpp_params.c    | 10 ++++------
 drivers/media/platform/rockchip/rkisp1/rkisp1-params.c |  8 +++-----
 include/media/v4l2-isp.h                               | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
index f0b330520f97ad3bb701de3aed35670158508829..8ef8df7c033219eb11e7e9d8838c37a817153eb6 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
@@ -594,23 +594,20 @@ static_assert(ARRAY_SIZE(c3_isp_params_handlers) ==
 static void c3_isp_params_cfg_blocks(struct c3_isp_params *params)
 {
 	struct c3_isp_params_cfg *config = params->buff->cfg;
-	size_t block_offset = 0;
+	struct v4l2_isp_params_block_header *header;
 
 	if (WARN_ON(!config))
 		return;
 
 	/* Walk the list of parameter blocks and process them */
-	while (block_offset < config->data_size) {
+	v4l2_isp_params_for_each_block(config, header) {
 		const union c3_isp_params_block *block;
 		c3_isp_block_handler block_handler;
 
-		block = (const union c3_isp_params_block *)
-			 &config->data[block_offset];
+		block = (const union c3_isp_params_block *)header;
 
 		block_handler = c3_isp_params_handlers[block->header.type];
 		block_handler(params->isp, block);
-
-		block_offset += block->header.size;
 	}
 }
 
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index 7b9d2650ce8730ccc05eee0c9dc33a88d022acf1..809a3587a5eee4d9817e720ae24e37f5f54397c0 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -817,10 +817,9 @@ static const struct vb2_ops mali_c55_params_vb2_ops = {
 void mali_c55_params_write_config(struct mali_c55 *mali_c55)
 {
 	struct mali_c55_params *params = &mali_c55->params;
+	struct v4l2_isp_params_block_header *header;
 	struct v4l2_isp_params_buffer *config;
 	struct mali_c55_params_buf *buf;
-	size_t block_offset = 0;
-	size_t max_offset;
 
 	spin_lock(&params->buffers.lock);
 
@@ -836,24 +835,20 @@ void mali_c55_params_write_config(struct mali_c55 *mali_c55)
 	buf->vb.sequence = mali_c55->isp.frame_sequence;
 	config = buf->config;
 
-	max_offset = config->data_size;
-
 	/*
 	 * Walk the list of parameter blocks and process them. No validation is
 	 * done here, as the contents of the config buffer are already checked
 	 * when the buffer is queued.
 	 */
-	while (max_offset && block_offset < max_offset) {
+	v4l2_isp_params_for_each_block(config, header) {
 		union mali_c55_params_block block;
 		mali_c55_params_handler handler;
 
-		block.data = &config->data[block_offset];
+		block.header = header;
 
 		/* We checked the array index already in .buf_queue() */
 		handler = mali_c55_params_handlers[block.header->type];
 		handler(mali_c55, block);
-
-		block_offset += block.header->size;
 	}
 
 	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
diff --git a/drivers/media/platform/dreamchip/rppx1/rpp_params.c b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
index aff3f3a997b4134b753fd1e1faf2058216f4b939..b8468d5c0fe1c783216567fab3366024fd9fcc63 100644
--- a/drivers/media/platform/dreamchip/rppx1/rpp_params.c
+++ b/drivers/media/platform/dreamchip/rppx1/rpp_params.c
@@ -53,8 +53,8 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 		 rppx1_reg_write write, void *priv)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct v4l2_isp_params_block_header *header;
 	struct v4l2_isp_buffer *cfg;
-	size_t block_offset;
 	int ret;
 
 	ret = v4l2_isp_params_validate_buffer_size(&rpp->isp, vb);
@@ -68,14 +68,12 @@ int rppx1_params(struct rppx1 *rpp, struct vb2_buffer *vb,
 		return ret;
 
 	/* Walk the list of parameter blocks and process them. */
-	block_offset = 0;
-	while (block_offset < cfg->data_size) {
-		const union rppx1_params_block *block =
-			(const union rppx1_params_block *)&cfg->data[block_offset];
+	v4l2_isp_params_for_each_block(cfg, header) {
+		const union rppx1_params_block *block;
 		struct rpp_module *module;
 		int ret;
 
-		block_offset += block->header.size;
+		block = (const union rppx1_params_block *)header;
 
 		switch (block->header.type) {
 		case RPPX1_PARAMS_BLOCK_TYPE_BLS_PRE1:
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index 8974c520d1879f3f0f642819a41984f6ac3ee01d..178da1f648869b7b963f634f29f5653cfb16dd77 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -2291,19 +2291,17 @@ static void rkisp1_ext_params_config(struct rkisp1_params *params,
 				     struct rkisp1_ext_params_cfg *cfg,
 				     u32 block_group_mask)
 {
-	size_t block_offset = 0;
+	struct v4l2_isp_params_block_header *header;
 
 	if (WARN_ON(!cfg))
 		return;
 
 	/* Walk the list of parameter blocks and process them. */
-	while (block_offset < cfg->data_size) {
+	v4l2_isp_params_for_each_block(cfg, header) {
 		const struct rkisp1_ext_params_handler *block_handler;
 		const union rkisp1_ext_params_config *block;
 
-		block = (const union rkisp1_ext_params_config *)
-			&cfg->data[block_offset];
-		block_offset += block->header.size;
+		block = (const union rkisp1_ext_params_config *)header;
 
 		/*
 		 * Make sure the block is supported by the platform and in the
diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
index b758d9b0e7f5c29cd8c4197f4a4a86cf3e5334fc..5f13651b53d3c06fc676b32826daa68ad77b5cf6 100644
--- a/include/media/v4l2-isp.h
+++ b/include/media/v4l2-isp.h
@@ -31,6 +31,21 @@ struct vb2_buffer;
 #define v4l2_isp_buffer_size(max_size)			\
 	(offsetof(struct v4l2_isp_buffer, data) + (max_size))
 
+/**
+ * v4l2_isp_params_for_each_block - Iterate through parameter blocks in a buffer
+ * @buffer: Pointer to the &struct v4l2_isp_buffer to iterate over
+ * @header: Pointer to the &struct v4l2_isp_block_header to store each block in
+ *
+ * Users of the V4L2 extensible parameters will need to iterate over the blocks
+ * within a buffer. Rather than allow each driver to do that in a bespoke manner
+ * this helper should be used to ensure commonality.
+ */
+#define v4l2_isp_params_for_each_block(buffer, header)			\
+	for (size_t offset = 0;						\
+	     offset < (buffer)->data_size &&				\
+	     (header = (void *)&(buffer)->data[offset], true);		\
+	     offset += header->size)					\
+
 /**
  * struct v4l2_isp_params_block_type_info - V4L2 ISP params per-block-type info
  * @size: the block type expected size

-- 
2.43.0

[PATCH 6/7] media: v4l2-isp: Introduce "reset buffer" concept

[Daniel Scally]

The extensible parameters format defined in the v4l2-isp framework
is designed to allow userspace to pass a truncated buffer of params
for an ISP, which need not include all of its available configuration
settings. This allows heavy configuration like lens shading tables to
be passed by userspace to the driver a single time, and subsequent
per-frame buffers to be much lighter. This is problematic in the case
of suspend or hibernate and resume, in which scenario the hardware
will lose all settings, but userspace has no way of knowing that they
need to be re-applied.

Introduce the concept of a "reset buffer" to v4l2-isp. Userspace can
pass a new flag that marks a parameter block as needing to be
re-applied. Where the validation process finds that flag, the marked
block is copied to a VB2Q-independent instance of a
v4l2_isp_params_buffer. If that block has been written to the reset
buffer before then the existing copy is overwritten.

The intention with using the existing v4l2_isp_params_buffer is to
facilitate a driver easily using the normal function to apply a
params buffer to also apply the reset buffer on device reset.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 Documentation/userspace-api/media/v4l/v4l2-isp.rst | 19 +++++++
 drivers/media/v4l2-core/v4l2-isp.c                 | 66 ++++++++++++++++++++++
 include/media/v4l2-isp.h                           | 15 +++++
 include/uapi/linux/media/v4l2-isp.h                |  1 +
 4 files changed, 101 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/v4l2-isp.rst b/Documentation/userspace-api/media/v4l/v4l2-isp.rst
index 8a3f05b5e85fd335e68e349234e2cfe5e7e83099..6bd72c7741816676d22a3197ae72ae6796973a5f 100644
--- a/Documentation/userspace-api/media/v4l/v4l2-isp.rst
+++ b/Documentation/userspace-api/media/v4l/v4l2-isp.rst
@@ -56,6 +56,25 @@ returns an error.
 Extension to the parameters format can be implemented by adding new blocks
 definition without invalidating the existing ones.
 
+ISP Reset buffers
+-----------------
+The parameters buffer format described by :c:type:`v4l2_isp_params_buffer`
+allows intentionally for userspace to pass a truncated buffer of parameters for
+an ISP which does not need to include all of its available configuration
+settings. This allows heavy configuration like lens shading tables to be passed
+by userspace a single time, and subsequent per-frame buffers to be much lighter.
+
+In the case of a suspend or hibernate action followed by a system resume this
+presents a problem. The hardware will have lost all settings, but userspace has
+no way of knowing that they were lost and need to be re-applied.
+
+The V4L2 ISP framework includes the concept of a "reset buffer". As part of the
+validation process, the framework will store the "last-seen" instance of each
+configuration block type in an instance of v4l2_isp_params_buffer. Drivers can
+then simply re-apply that "reset buffer" using their normal configuration
+function when resuming from hibernation with an active stream
+
+
 ISP statistics
 ==============
 
diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 925632e1037db18f0948ceb0ce7c652e167516c9..1420aa0f48194ff04a29fbbf83d0646d3b9d23d2 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -54,6 +54,69 @@ int v4l2_isp_validate(struct v4l2_isp *isp)
 }
 EXPORT_SYMBOL_GPL(v4l2_isp_validate);
 
+int v4l2_isp_params_init_reset_buf(struct v4l2_isp *isp)
+{
+	isp->params.reset_buf = devm_kzalloc(isp->dev, isp->params.max_size,
+					     GFP_KERNEL);
+	if (!isp->params.reset_buf)
+		return -ENOMEM;
+
+	isp->params.reset_buf->version = V4L2_ISP_PARAMS_VERSION_V0;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(v4l2_isp_params_init_reset_buf);
+
+static void v4l2_isp_params_update_reset_buf(struct v4l2_isp *isp,
+					     const struct v4l2_isp_params_block_header *block,
+					     const struct v4l2_isp_params_block_type_info *info)
+{
+	struct v4l2_isp_params_buffer *buffer = isp->params.reset_buf;
+	struct v4l2_isp_params_block_header *new_block;
+	struct v4l2_isp_params_block_header *old_block;
+
+	/*
+	 * If the reset buffer was never initialised then the driver didn't ask
+	 * us to handle this and we can just do nothing.
+	 */
+	if (!buffer) {
+		dev_dbg(isp->dev, "Reset buffer is not initialised\n");
+		return;
+	}
+
+	v4l2_isp_params_for_each_block(buffer, old_block) {
+		if (old_block->type != block->type)
+			continue;
+
+		memcpy(old_block, block, block->size);
+		return;
+	}
+
+	/*
+	 * If we didn't find an existing stored copy for the given block type
+	 * then we simply add it to the end of the array.
+	 */
+	new_block = (struct v4l2_isp_params_block_header *)
+		&buffer->data[buffer->data_size];
+
+	/*
+	 * This looks awkward! We're memcpy()ing with block->size but setting
+	 * new_block->size with info->size. The reason is that the source buffer
+	 * might have only contained the block header rather than a full block.
+	 * In that case, we need to only memcpy() the amount of space that it
+	 * actually used in the source buffer.
+	 *
+	 * In future though userspace might queue a buffer with a full block for
+	 * the same block type. If we only reserved here enough space for the
+	 * header then things would need to be rearranged in the reset buffer.
+	 * Much easier to just reserve enough space for the full block right
+	 * away.
+	 */
+	memcpy(new_block, block, block->size);
+	new_block->size = info->size;
+	buffer->data_size += new_block->size;
+}
+
 int v4l2_isp_params_validate_buffer_size(struct v4l2_isp *isp,
 					 struct vb2_buffer *vb)
 {
@@ -170,6 +233,9 @@ int v4l2_isp_params_validate_buffer(struct v4l2_isp *isp, struct vb2_buffer *vb,
 		if (info->block_validate && info->block_validate(dev, block))
 			return -EINVAL;
 
+		if (block->flags & V4L2_ISP_PARAMS_FL_BLOCK_REAPPLY)
+			v4l2_isp_params_update_reset_buf(isp, block, info);
+
 		block_offset += block->size;
 		buffer_size -= block->size;
 	}
diff --git a/include/media/v4l2-isp.h b/include/media/v4l2-isp.h
index 5f13651b53d3c06fc676b32826daa68ad77b5cf6..b2ad6d98a4c266c388db623ddd549307b7fd4b09 100644
--- a/include/media/v4l2-isp.h
+++ b/include/media/v4l2-isp.h
@@ -72,6 +72,8 @@ struct v4l2_isp_params_block_type_info {
  * @dev: pointer to the &struct device associated with the ISP
  * @params: sub-struct holding data related to the V4L2 ISP parameters
  * @params.types_info: pointer to array of params block information structs
+ * @params.reset_buf: pointer to a buffer containing a copy of any block
+ *		      flagged as needing re-application on device reset
  * @params.num_block_types: number of possible params block types
  * @params.max_size: maximum size in bytes of the data member of
  *		     &struct v4l2_isp_params_buffer
@@ -88,6 +90,7 @@ struct v4l2_isp {
 
 	struct {
 		const struct v4l2_isp_params_block_type_info *types_info;
+		struct v4l2_isp_params_buffer *reset_buf;
 		size_t num_block_types;
 		size_t max_size;
 	} params;
@@ -104,6 +107,18 @@ struct v4l2_isp {
  */
 int v4l2_isp_validate(struct v4l2_isp *isp);
 
+/**
+ * v4l2_isp_params_init_reset_buf - Init a buffer to store device reset data
+ * @isp: pointer to the &struct v4l2_isp to allocate the buffer for
+ *
+ * This function allocates memory for &struct v4l2_isp.params.reset_buf, which
+ * is intended to be populated with parameter blocks that are flagged with
+ * %V4L2_ISP_PARAMS_FL_BLOCK_REAPPLY.
+ *
+ * @return 0 on success, -ENOMEM on failure
+ */
+int v4l2_isp_params_init_reset_buf(struct v4l2_isp *isp);
+
 /**
  * v4l2_isp_params_validate_buffer_size - Validate a V4L2 ISP buffer sizes
  * @isp: the driver's &struct v4l2_isp pointer
diff --git a/include/uapi/linux/media/v4l2-isp.h b/include/uapi/linux/media/v4l2-isp.h
index e4607e1217e1a720f8f0a180abf3c064a0d81bf7..43059940edf1a2cad9868a4bc049ee32211cc465 100644
--- a/include/uapi/linux/media/v4l2-isp.h
+++ b/include/uapi/linux/media/v4l2-isp.h
@@ -42,6 +42,7 @@ enum v4l2_isp_version {
 
 #define V4L2_ISP_PARAMS_FL_BLOCK_DISABLE	(1U << 0)
 #define V4L2_ISP_PARAMS_FL_BLOCK_ENABLE		(1U << 1)
+#define V4L2_ISP_PARAMS_FL_BLOCK_REAPPLY	(1U << 2)
 
 /*
  * Reserve the first 8 bits for V4L2_ISP_PARAMS_FL_* flag.

-- 
2.43.0

[PATCH 7/7] media: rkisp1: Re-apply v4l2-isp reset buffer on resume

[Daniel Scally]

When resuming from suspension or hibernation with an active stream
the hardware will have lost all settings. The v4l2_isp framework
defines a configuration buffer format that is extensible, meaning
userspace does not need to pass in a full set of parameters for each
frame. As userspace is unaware of suspend / resume it's very unlikely
to pass a full reconfiguration.

Use the v4l2-isp reset buffer to re-apply configuration blocks marked
as needing to be reapplied.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 .../media/platform/rockchip/rkisp1/rkisp1-params.c | 35 +++++++++++++++-------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index 178da1f648869b7b963f634f29f5653cfb16dd77..952c4246ccefa81aa4853a18c941a6f10f456902 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -2326,20 +2326,31 @@ static void rkisp1_ext_params_config(struct rkisp1_params *params,
 
 void rkisp1_params_reapply_config(struct rkisp1_params *params)
 {
+	struct v4l2_isp_params_buffer *buffer = params->v4l2_isp.params.reset_buf;
+	struct rkisp1_ext_params_cfg *cfg;
+
 	guard(spinlock)(&params->buf.lock);
 
-	if (!params->buf.curr)
+	if (params->buf.curr) {
+		if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
+			rkisp1_isp_isr_other_config(params, params->buf.curr->cfg);
+			rkisp1_isp_isr_lsc_config(params, params->buf.curr->cfg);
+			rkisp1_isp_isr_meas_config(params, params->buf.curr->cfg);
+		} else {
+			rkisp1_ext_params_config(params, params->buf.curr->cfg,
+						RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
+						RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
+		}
+	}
+
+	buffer = params->v4l2_isp.params.reset_buf;
+	if (!buffer)
 		return;
 
-	if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
-		rkisp1_isp_isr_other_config(params, params->buf.curr->cfg);
-		rkisp1_isp_isr_lsc_config(params, params->buf.curr->cfg);
-		rkisp1_isp_isr_meas_config(params, params->buf.curr->cfg);
-	} else {
-		rkisp1_ext_params_config(params, params->buf.curr->cfg,
-					 RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
-					 RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
-	}
+	cfg = (struct rkisp1_ext_params_cfg *)buffer;
+	rkisp1_ext_params_config(params, cfg,
+				 RKISP1_EXT_PARAMS_BLOCK_GROUP_OTHERS |
+				 RKISP1_EXT_PARAMS_BLOCK_GROUP_LSC);
 }
 
 static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
@@ -2891,6 +2902,10 @@ int rkisp1_params_register(struct rkisp1_device *rkisp1)
 	if (ret)
 		return ret;
 
+	ret = v4l2_isp_params_init_reset_buf(&params->v4l2_isp);
+	if (ret)
+		return ret;
+
 	mutex_init(&node->vlock);
 	INIT_LIST_HEAD(&params->buf.queue);
 	spin_lock_init(&params->buf.lock);

-- 
2.43.0

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

[Niklas Söderlund]

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

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

[Dan Scally]

Hi Niklas - thanks for commenting

On 04/11/2026 14:23, Niklas Söderlund wrote:
> 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 don't see any reason that would not work, and maybe I overthought this indeed. This method does 
still seem to have some advantages to me though; it means we can easily reuse the existing function 
in the driver that applies the parameters to the hardware, which should make adding suspend() and 
resume() support for a driver that uses the extensible parameters much simpler. If we were to read 
the settings from the ISP registers and store them in suspend() instead of doing it this way I think 
I'd still want to re-use the normal "write-params-buffer-to-hardware" function (for example 
rkisp1_ext_params_config() in the rkisp1 driver) to avoid the risk of ending up with differences 
between how the parameters were applied on resume() and on the normal handling of a parameters 
buffer from userspace, so I think that we would end up populating a struct v4l2_isp_params_buffer 
that was stored in the device's driver_data struct anyway - the difference would be that we'd have 
to build it in suspend() for each ISP driver instead of having the core handle it in the course of 
normal operation. I could knock up an example for the rkisp1 of creating that buffer in suspend() to 
see how it looks perhaps?

The "reset buffer" idea also means that if we extend the parameters buffer for an ISP with more 
blocks in the future there's no extra work to do in the suspend() and resume() callbacks for them - 
as long as userspace flagged the new block then they would just be reapplied "for free".

Thanks
Dan

> 
>>
>> 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>
>>
> 

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

[Niklas Söderlund]

Hi Dan,

On 2026-11-04 16:24:01 +0000, Dan Scally wrote:
> Hi Niklas - thanks for commenting
> 
> On 04/11/2026 14:23, Niklas Söderlund wrote:
> > 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 don't see any reason that would not work, and maybe I overthought this
> indeed. This method does still seem to have some advantages to me though; it
> means we can easily reuse the existing function in the driver that applies
> the parameters to the hardware, which should make adding suspend() and
> resume() support for a driver that uses the extensible parameters much
> simpler. If we were to read the settings from the ISP registers and store
> them in suspend() instead of doing it this way I think I'd still want to
> re-use the normal "write-params-buffer-to-hardware" function (for example
> rkisp1_ext_params_config() in the rkisp1 driver) to avoid the risk of ending
> up with differences between how the parameters were applied on resume() and
> on the normal handling of a parameters buffer from userspace, so I think
> that we would end up populating a struct v4l2_isp_params_buffer that was
> stored in the device's driver_data struct anyway - the difference would be
> that we'd have to build it in suspend() for each ISP driver instead of
> having the core handle it in the course of normal operation. I could knock
> up an example for the rkisp1 of creating that buffer in suspend() to see how
> it looks perhaps?

I think what made me comment on this is the involvement of user-space in 
how suspend/resume will act. Normally I assume the kernel will deal with 
it all, and after resume it will restore (to the extent possible) the 
hardware state as it where at suspend time. I'm sure this approach exist 
elsewhere in the kernel, I'm just not aware of it.

My worry is that suspend/resume will work as expected for libcamera 
users, but then somebody does something else and expect the kernel to 
handle it correctly like it do for other types of devices.

Maybe I'm being too causations. Putting the user-space involvement to 
the side for a moment, having v4l2-isp provide helpers to apply buffers 
at resume time to save on boiler plate code in drivers is of course a 
good idea.

IIRC this is what is done for V4L2 controls already. The V4L2 core sets 
all controls at resume time so the drivers don't need to deal with it 
directly in their handlers and the existing set/get callbacks are used.

Maybe we need a get params callback to v4l2-isp? And have the core do it 
the same way here? I guess this is one fun area where we mixing V4L2 
classic controls, controls in buffers (v4l2-isp) and per frame controls 
into yet another control abstraction, so maybe not go there ;-)

I'm happy to hear what others think of this. I do see the value of what 
you are trying todo, I'm just a tad worried about involving user-space.

> 
> The "reset buffer" idea also means that if we extend the parameters buffer
> for an ISP with more blocks in the future there's no extra work to do in the
> suspend() and resume() callbacks for them - as long as userspace flagged the
> new block then they would just be reapplied "for free".

I understand and this would be appealing.

> 
> Thanks
> Dan
> 
> > 
> > > 
> > > 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

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

[Dan Scally]

Hi Niklas

On 04/11/2026 17:06, Niklas Söderlund wrote:
> Hi Dan,
> 
> On 2026-11-04 16:24:01 +0000, Dan Scally wrote:
>> Hi Niklas - thanks for commenting
>>
>> On 04/11/2026 14:23, Niklas Söderlund wrote:
>>> 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 don't see any reason that would not work, and maybe I overthought this
>> indeed. This method does still seem to have some advantages to me though; it
>> means we can easily reuse the existing function in the driver that applies
>> the parameters to the hardware, which should make adding suspend() and
>> resume() support for a driver that uses the extensible parameters much
>> simpler. If we were to read the settings from the ISP registers and store
>> them in suspend() instead of doing it this way I think I'd still want to
>> re-use the normal "write-params-buffer-to-hardware" function (for example
>> rkisp1_ext_params_config() in the rkisp1 driver) to avoid the risk of ending
>> up with differences between how the parameters were applied on resume() and
>> on the normal handling of a parameters buffer from userspace, so I think
>> that we would end up populating a struct v4l2_isp_params_buffer that was
>> stored in the device's driver_data struct anyway - the difference would be
>> that we'd have to build it in suspend() for each ISP driver instead of
>> having the core handle it in the course of normal operation. I could knock
>> up an example for the rkisp1 of creating that buffer in suspend() to see how
>> it looks perhaps?
> 
> I think what made me comment on this is the involvement of user-space in
> how suspend/resume will act. Normally I assume the kernel will deal with
> it all, and after resume it will restore (to the extent possible) the
> hardware state as it where at suspend time. I'm sure this approach exist
> elsewhere in the kernel, I'm just not aware of it.
> 
> My worry is that suspend/resume will work as expected for libcamera
> users, but then somebody does something else and expect the kernel to
> handle it correctly like it do for other types of devices.

Ah; yes I understand. The motivation with the flag was just to avoid dealing with _every_ block, 
because on most frames, all that comes in are settings that change rapidly like the white balance 
gains. It doesn't seem worthwhile even handling those because the scene will probably have changed 
when the device resumes anyway so whatever they were set to originally is unlikely to still be 
appropriate anyway. In the cover letter I mentioned the alternative to just record the "last seen" 
copy of every block and reapply that, but that implies a performance hit that I think is undesirable 
given it's probably a waste of time to reapply those kinds of settings.

Perhaps instead of the flag coming from userspace we could add a flags field to struct 
v4l2_isp_params_block_type_info, an array of which is already passed to the v4l2-isp core by each 
driver - then the driver can decide whether or not a block is flagged as needing to be tracked by 
the core and reapplied on resume, and the whole thing is completely transparent to userspace.


> 
> Maybe I'm being too causations. Putting the user-space involvement to
> the side for a moment, having v4l2-isp provide helpers to apply buffers
> at resume time to save on boiler plate code in drivers is of course a
> good idea.
> 
> IIRC this is what is done for V4L2 controls already. The V4L2 core sets
> all controls at resume time so the drivers don't need to deal with it
> directly in their handlers and the existing set/get callbacks are used.
> 
> Maybe we need a get params callback to v4l2-isp? And have the core do it
> the same way here? I guess this is one fun area where we mixing V4L2
> classic controls, controls in buffers (v4l2-isp) and per frame controls
> into yet another control abstraction, so maybe not go there ;-)

Haha well I did wonder if the block handlers could also be added to struct 
v4l2_isp_params_block_type_info and a function from v4l2-isp core do the walking of the blocks and 
calling of the handlers...I haven't looked into it too seriously yet, but maybe it's worthwhile.

> 
> I'm happy to hear what others think of this. I do see the value of what
> you are trying todo, I'm just a tad worried about involving user-space.

I understand - thank you for raising it!

Dan

> 
>>
>> The "reset buffer" idea also means that if we extend the parameters buffer
>> for an ISP with more blocks in the future there's no extra work to do in the
>> suspend() and resume() callbacks for them - as long as userspace flagged the
>> new block then they would just be reapplied "for free".
> 
> I understand and this would be appealing.
> 
>>
>> Thanks
>> Dan
>>
>>>
>>>>
>>>> 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>
>>>>
>>>
>>
>