[PATCH v3 0/3] Implement Region of Interest(ROI) support

The Linux Kernel Mailing List
 help / color / mirror / Atom feed

* [PATCH v3 0/3] Implement Region of Interest(ROI) support
@ 2026-08-04 20:05 ` Deepa Guthyappa Madivalara
  2026-08-04 20:05   ` [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
                     ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-04 20:05 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Vikash Garodia, Dikshita Agarwal,
	Abhinav Kumar, Bryan O'Donoghue
  Cc: linux-media, linux-kernel, linux-arm-msm,
	Deepa Guthyappa Madivalara, kernel test robot

Hi all,

This patch set implements region of interest(ROI) support for
video encoder to be configured as a MB based delta_qp for
the whole frame. Custom controls V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP
and V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE are implemented to achieve this. 
Corresponding support is added in iris encoder to implement this
feature.

This submission and design is based on the feedback received
from community for the RFC[1] series posted earlier. 
Initial testing is done by adding support to v4l2-ctl[2] to test this
control setting and its functionality.

[1]https://lore.kernel.org/linux-media/20260113-iris_enc_roi-v1-0-6c86eba38587@oss.qualcomm.com
[2]https://lore.kernel.org/linux-media/20260616-enc_roi_enable-v1-1-252792d27894@oss.qualcomm.com

Results:

v4l2-ctl -d /dev/video1 --list-ctrls
...
enc_mb_roi_delta_qp 0x00990b92 (s8)     : min=-31 max=30 step=1 default=0 dims=[139264] flags=has-payload
        roi_mb_size 0x00990b93 (u8)     : min=16 max=32 step=1 default=16 value=16 flags=has-payload

v4l2-compliance -d /dev/video1
...
Buffer ioctls:
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_REMOVE_BUFS: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)
        test blocking wait: OK

Total for iris_driver device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0

./v4l2-ctl --verbose -d /dev/video1 --set-fmt-video-out=width=176,height=144,pixelformat=NV12 --set-fmt-video=pixelformat=H264 --set-ctrl=enc_mb_roi_delta_qp=-20,-2,-22,9,-16,-2,14,-9,-19,3,-26,19,-23,30,1,0,-6,-7,7,2,-2,15,-15,-24,-24,-26,-26,5,-1,-1,9,-11,15,-25,-11,26,-11,7,-26,-27,-8,-18,7,-4,-12,-23,-11,22,-7,7,-14,24,6,-8,-7,2,7,-26,2,-25,-1,-3,-4,-6,-26,-11,7,16,-30,-22,-15,30,-14,-29,-11,-8,8,6,19,15,-17,21,-27,30,-17,-28,-10,-20,1,22,29,-19,-2,28,10,12,-10,7,-4 --stream-mmap --stream-out-mmap --stream-from=/media/foreman_qcif_nv12.yuv --stream-to=roi_try.h264 --stream-count=10

Validated on SM8550 MTP.

Thanks,
Deepa

Signed-off-by: Deepa Guthyappa Madivalara
<deepa.madivalara@oss.qualcomm.com>

Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
---
Changes in v3:
- Updated Documentation (Hans) 
- Reuse ROUND_TO_RANGE_RANGE for p_s8 (Hans)
- Rebase onto media-committers next.
- Link to v2: https://lore.kernel.org/r/20260714-enc_roi_enable-v2-0-63683f9dbcef@oss.qualcomm.com

Changes in v2:
- Fix issues detected by Media CI robot and kernel test robot.
  Reported-by: kernel test robot <lkp@intel.com>
  Closes: https://lore.kernel.org/oe-kbuild-all/202606171554.03yt9utn-lkp@intel.com/
- Rebase onto media-committers next.
- Update to handle p_s8 in std_validate_elem correctly. 
- Link to v1: https://lore.kernel.org/r/20260616-enc_roi_enable-v1-0-fefcc4e76f33@oss.qualcomm.com

---
Deepa Guthyappa Madivalara (3):
      media: uapi: Introduce new control for video encoder ROI
      media: v4l2-core: Add support for video encoder ROI control
      media: iris: Add ROI support framework for iris video encoder

 .../userspace-api/media/v4l/ext-ctrls-codec.rst    |  19 ++++
 .../media/v4l/videodev2.h.rst.exceptions           |   1 +
 .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst |   4 +
 .../userspace-api/media/v4l/vidioc-queryctrl.rst   |   6 ++
 drivers/media/platform/qcom/iris/iris_buffer.c     | 101 ++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_buffer.h     |  22 +++++
 drivers/media/platform/qcom/iris/iris_ctrls.c      | 104 ++++++++++++++++++++-
 drivers/media/platform/qcom/iris/iris_ctrls.h      |   3 +
 drivers/media/platform/qcom/iris/iris_hfi_common.h |   1 +
 drivers/media/platform/qcom/iris/iris_hfi_gen2.c   |  14 +++
 .../platform/qcom/iris/iris_hfi_gen2_command.c     |  62 ++++++++++++
 .../platform/qcom/iris/iris_hfi_gen2_defines.h     |   3 +
 .../platform/qcom/iris/iris_hfi_gen2_packet.c      |   6 +-
 .../platform/qcom/iris/iris_hfi_gen2_packet.h      |   3 +
 .../platform/qcom/iris/iris_hfi_gen2_response.c    |  29 ++++++
 .../platform/qcom/iris/iris_platform_common.h      |   5 +
 drivers/media/platform/qcom/iris/iris_venc.c       |   4 +
 drivers/media/platform/qcom/iris/iris_venc.h       |   2 +
 drivers/media/platform/qcom/iris/iris_vidc.c       |   2 +
 drivers/media/v4l2-core/v4l2-ctrls-api.c           |   1 +
 drivers/media/v4l2-core/v4l2-ctrls-core.c          |  25 ++++-
 drivers/media/v4l2-core/v4l2-ctrls-defs.c          |  10 ++
 include/media/v4l2-ctrls.h                         |   2 +
 include/uapi/linux/v4l2-controls.h                 |   2 +
 include/uapi/linux/videodev2.h                     |   3 +
 25 files changed, 428 insertions(+), 6 deletions(-)
---
base-commit: 31152f5b0f8719f92063b8c6196cd5e34106c73d
change-id: 20260616-enc_roi_enable-4d385c4783ae

Best regards,
-- 
Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI
  2026-08-04 20:05 ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
@ 2026-08-04 20:05   ` Deepa Guthyappa Madivalara
  2026-08-05  3:49     ` Bryan O'Donoghue
  2026-08-04 20:05   ` [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control Deepa Guthyappa Madivalara
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-04 20:05 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Vikash Garodia, Dikshita Agarwal,
	Abhinav Kumar, Bryan O'Donoghue
  Cc: linux-media, linux-kernel, linux-arm-msm,
	Deepa Guthyappa Madivalara, kernel test robot

Add custom control, V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP for
video encoder Region of Interest to allow applications to specify
different quality levels for specific regions in video frames. Define
V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE to retrieve supported MB size based on
codec from the hardware encoder and set the corresponding ROI MB delta_qp
map that adjusts quantization relative to the frame QP's base value.

This enables use cases like prioritizing quality for faces in video
conferencing or important objects in surveillance footage while reducing
bitrate for less critical areas.

Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
---
 .../userspace-api/media/v4l/ext-ctrls-codec.rst       | 19 +++++++++++++++++++
 .../media/v4l/videodev2.h.rst.exceptions              |  1 +
 .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst    |  4 ++++
 .../userspace-api/media/v4l/vidioc-queryctrl.rst      |  6 ++++++
 include/media/v4l2-ctrls.h                            |  1 +
 include/uapi/linux/v4l2-controls.h                    |  2 ++
 include/uapi/linux/videodev2.h                        |  3 +++
 7 files changed, 36 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index ab865a1a6ba929f1725ce2ab0fd3aeee4a69329a..0f8179de8f742253a5152231592fb8b4355298f7 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -1674,6 +1674,25 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     Codecs need to always use the specified range, rather then a HW custom range.
     Applicable to encoders
 
+``V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP (__s8)``
+    This custom control is used to set ROI MB map delta_Qp for whole frame.
+    The frame is divided into a grid of MB Size by MB Size pixels (MB Size is
+    obtained from querying ``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE``). Each block
+    is configured with delta_Qp in raster order. The valid range for delta_Qp
+    is encoder dependent.
+    Applicable to encoders.
+
+``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE`` (integer)
+    A read-only control that reports the MB Size used for the ROI delta_QP map.
+    The supported MB Size depends on the encoder codec.
+    Only square macroblocks are supported. The value returned represents the
+    width and height of each block in pixels. The ROI delta_QP map provided
+    through the ROI control must use the MB Size reported by this control.
+    The number of entries in the ROI delta_QP array is determined by the
+    frame dimensions and the reported MB Size. For example, for a QCIF frame
+    (176x144) if reported MB Size is 16, the delta_QP array must contain
+    99 entries (176 x 144 / 16 x 16).
+
 .. raw:: latex
 
     \normalsize
diff --git a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
index 6182b4e2d2ee002c0d3562e1cc3bc2fd9c4286d6..b036ed3ec2f0744175cad1cbc4e73fe828e0c955 100644
--- a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
@@ -141,6 +141,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_U16 :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_U32 :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_U8 :c:type:`V4L.v4l2_ctrl_type`
+replace symbol V4L2_CTRL_TYPE_S8 :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_MPEG2_SEQUENCE :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_MPEG2_PICTURE :c:type:`V4L.v4l2_ctrl_type`
 replace symbol V4L2_CTRL_TYPE_MPEG2_QUANTISATION :c:type:`V4L.v4l2_ctrl_type`
diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
index d78328152b7552f8f0d30a9e2f7c104bfd4b70d3..fe5f8992389c32d8f35f432229ef2f734002644e 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -188,6 +188,10 @@ still cause this situation.
       - ``p_u32``
       - A pointer to a matrix control of unsigned 32-bit values. Valid if
 	this control is of type ``V4L2_CTRL_TYPE_U32``.
+    * - __s8 *
+      - ``p_s8``
+      - A pointer to a matrix control of signed 8-bit values. Valid if
+	this control is of type ``V4L2_CTRL_TYPE_S8``.
     * - __s32 *
       - ``p_s32``
       - A pointer to a matrix control of signed 32-bit values. Valid if
diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
index 82c8b52e771ce9301491b3f752e501a8fe7e2434..57e1652ae72e0324742142446acf32a16b94941c 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
@@ -425,6 +425,12 @@ See also the examples in :ref:`control`.
       - any
       - An unsigned 32-bit valued control ranging from minimum to maximum
 	inclusive. The step value indicates the increment between values.
+    * - ``V4L2_CTRL_TYPE_S8``
+      - any
+      - any
+      - any
+      - A signed 8-bit valued control ranging from minimum to maximum
+	inclusive. The step value indicates the increment between values.
     * - ``V4L2_CTRL_TYPE_MPEG2_QUANTISATION``
       - n/a
       - n/a
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 327976b14d50b73a631e4d2ed95d9f4d146c45ab..a1806ddbc797efa52e83cd3f685ef70d5b5483d2 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -64,6 +64,7 @@ union v4l2_ctrl_ptr {
 	s32 *p_s32;
 	s64 *p_s64;
 	u8 *p_u8;
+	s8 *p_s8;
 	u16 *p_u16;
 	u32 *p_u32;
 	char *p_char;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index affec0ab4781507be761cb791ff737460acf9cb7..00a2c48630bd198faa9a2b833a9aa346d8357555 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -920,6 +920,8 @@ enum v4l2_mpeg_video_av1_level {
 };
 
 #define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
+#define V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP  (V4L2_CID_CODEC_BASE + 658)
+#define V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE  (V4L2_CID_CODEC_BASE + 659)
 
 /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
 #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 5373dba640fae3d19247ab1c74ba336e05dc0fe0..3ed343f8c81e0c9240d5a7e5b2dfe365a529fb6c 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1884,6 +1884,7 @@ struct v4l2_ext_control {
 		__s64 value64;
 		char __user *string;
 		__u8 __user *p_u8;
+		__s8 __user *p_s8;
 		__u16 __user *p_u16;
 		__u32 __user *p_u32;
 		__s32 __user *p_s32;
@@ -1961,6 +1962,7 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_U8	     = 0x0100,
 	V4L2_CTRL_TYPE_U16	     = 0x0101,
 	V4L2_CTRL_TYPE_U32	     = 0x0102,
+	V4L2_CTRL_TYPE_S8		 = 0x0103,
 	V4L2_CTRL_TYPE_AREA          = 0x0106,
 	V4L2_CTRL_TYPE_RECT	     = 0x0107,
 
@@ -1997,6 +1999,7 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY = 0x281,
 	V4L2_CTRL_TYPE_AV1_FRAME	    = 0x282,
 	V4L2_CTRL_TYPE_AV1_FILM_GRAIN	    = 0x283,
+
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control
  2026-08-04 20:05 ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
  2026-08-04 20:05   ` [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
@ 2026-08-04 20:05   ` Deepa Guthyappa Madivalara
  2026-08-05  3:51     ` Bryan O'Donoghue
  2026-08-04 20:05   ` [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder Deepa Guthyappa Madivalara
  2026-08-05  4:09   ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Bryan O'Donoghue
  3 siblings, 1 reply; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-04 20:05 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Vikash Garodia, Dikshita Agarwal,
	Abhinav Kumar, Bryan O'Donoghue
  Cc: linux-media, linux-kernel, linux-arm-msm,
	Deepa Guthyappa Madivalara, kernel test robot

Add necessary support for controls V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP
and V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE.

Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
---
 drivers/media/v4l2-core/v4l2-ctrls-api.c  |  1 +
 drivers/media/v4l2-core/v4l2-ctrls-core.c | 25 +++++++++++++++++++++++--
 drivers/media/v4l2-core/v4l2-ctrls-defs.c | 10 ++++++++++
 include/media/v4l2-ctrls.h                |  3 ++-
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c
index 93d8d4012d0f4fef004e417d0aee2ae44b1b30bd..7d41cfd7378baaa929d4da0266c45f731bb54285 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
@@ -980,6 +980,7 @@ int __v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
 	case V4L2_CTRL_TYPE_U8:
 	case V4L2_CTRL_TYPE_U16:
 	case V4L2_CTRL_TYPE_U32:
+	case V4L2_CTRL_TYPE_S8:
 		if (ctrl->is_array)
 			return -EINVAL;
 		ret = check_range(ctrl->type, min, max, step, def);
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 5b8a594fb9e24e16128c9c763a3b1dd311fad2ba..5c0e23500f99bc820dc4b36dc809a6a3c8691d95 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -290,6 +290,14 @@ static void __v4l2_ctrl_type_op_init(const struct v4l2_ctrl *ctrl, u32 from_idx,
 			memset(ptr.p_u32 + from_idx, 0, elems * sizeof(u32));
 		}
 		break;
+	case V4L2_CTRL_TYPE_S8:
+		if (value) {
+			for (i = from_idx; i < tot_elems; i++)
+				ptr.p_s8[i] = value;
+		} else {
+			memset(ptr.p_s8 + from_idx, 0, elems * sizeof(s8));
+		}
+		break;
 	default:
 		for (i = from_idx; i < tot_elems; i++) {
 			switch (which) {
@@ -370,6 +378,9 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
 	case V4L2_CTRL_TYPE_U32:
 		pr_cont("%u", (unsigned)*ptr.p_u32);
 		break;
+	case V4L2_CTRL_TYPE_S8:
+		pr_cont("%d", *ptr.p_s8);
+		break;
 	case V4L2_CTRL_TYPE_AREA:
 		pr_cont("%ux%u", ptr.p_area->width, ptr.p_area->height);
 		break;
@@ -1397,6 +1408,8 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 			return -EINVAL;
 		break;
 
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -1410,6 +1423,7 @@ static int std_validate_elem(const struct v4l2_ctrl *ctrl, u32 idx,
 	size_t len;
 	u64 offset;
 	s64 val;
+	s32 tmp;
 
 	switch ((u32)ctrl->type) {
 	case V4L2_CTRL_TYPE_INTEGER:
@@ -1435,7 +1449,11 @@ static int std_validate_elem(const struct v4l2_ctrl *ctrl, u32 idx,
 		return ROUND_TO_RANGE(ptr.p_u16[idx], u16, ctrl);
 	case V4L2_CTRL_TYPE_U32:
 		return ROUND_TO_RANGE(ptr.p_u32[idx], u32, ctrl);
-
+	case V4L2_CTRL_TYPE_S8:
+		tmp = ptr.p_s8[idx];
+		ROUND_TO_RANGE_RANGE(tmp, s32, ctrl);
+		ptr.p_s8[idx] = (s8)tmp;
+		return 0;
 	case V4L2_CTRL_TYPE_BOOLEAN:
 		ptr.p_s32[idx] = !!ptr.p_s32[idx];
 		return 0;
@@ -1588,6 +1606,7 @@ void cur_to_new(struct v4l2_ctrl *ctrl)
 		return;
 	if (ctrl->is_dyn_array)
 		ctrl->new_elems = ctrl->elems;
+
 	ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new, ctrl->new_elems);
 }
 
@@ -2030,6 +2049,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_U32:
 		elem_size = sizeof(u32);
 		break;
+	case V4L2_CTRL_TYPE_S8:
+		elem_size = sizeof(s8);
+		break;
 	case V4L2_CTRL_TYPE_MPEG2_SEQUENCE:
 		elem_size = sizeof(struct v4l2_ctrl_mpeg2_sequence);
 		break;
@@ -2247,7 +2269,6 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 
 	if (flags & V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX) {
 		void *ptr = ctrl->p_def.p;
-
 		if (p_min.p_const) {
 			ptr += elem_size;
 			ctrl->p_min.p = ptr;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index e062f2088490470c42d6c579ff7675be454a29b0..8f895060799ea13443143edd398dfb97f4ba0085 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -974,6 +974,8 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_AVERAGE_QP:			return "Average QP Value";
 	case V4L2_CID_FWHT_I_FRAME_QP:				return "FWHT I-Frame QP Value";
 	case V4L2_CID_FWHT_P_FRAME_QP:				return "FWHT P-Frame QP Value";
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:		return "Encoder ROI MB Delta QP";
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE:			return "Encoder ROI MB Size";
 
 	/* VPX controls */
 	case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS:		return "VPX Number of Partitions";
@@ -1622,6 +1624,14 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_COLORIMETRY_HDR10_MASTERING_DISPLAY:
 		*type = V4L2_CTRL_TYPE_HDR10_MASTERING_DISPLAY;
 		break;
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:
+		*type = V4L2_CTRL_TYPE_S8;
+		*flags |= V4L2_CTRL_FLAG_DYNAMIC_ARRAY;
+		break;
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE:
+		*type = V4L2_CTRL_TYPE_U8;
+		*flags |= V4L2_CTRL_FLAG_READ_ONLY;
+		break;
 	default:
 		*type = V4L2_CTRL_TYPE_INTEGER;
 		break;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index a1806ddbc797efa52e83cd3f685ef70d5b5483d2..9a68a3555e349f1e55aa01af5c10f08201d62bae 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -27,6 +27,7 @@ struct video_device;
 
 /**
  * union v4l2_ctrl_ptr - A pointer to a control value.
+ * @p_s8:			Pointer to a 8-bit signed value.
  * @p_s32:			Pointer to a 32-bit signed value.
  * @p_s64:			Pointer to a 64-bit signed value.
  * @p_u8:			Pointer to a 8-bit unsigned value.
@@ -61,10 +62,10 @@ struct video_device;
  * @p_const:			Pointer to a constant compound value.
  */
 union v4l2_ctrl_ptr {
+	s8 *p_s8;
 	s32 *p_s32;
 	s64 *p_s64;
 	u8 *p_u8;
-	s8 *p_s8;
 	u16 *p_u16;
 	u32 *p_u32;
 	char *p_char;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder
  2026-08-04 20:05 ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
  2026-08-04 20:05   ` [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
  2026-08-04 20:05   ` [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control Deepa Guthyappa Madivalara
@ 2026-08-04 20:05   ` Deepa Guthyappa Madivalara
  2026-08-05  4:06     ` Bryan O'Donoghue
  2026-08-05  4:09   ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Bryan O'Donoghue
  3 siblings, 1 reply; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-04 20:05 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Vikash Garodia, Dikshita Agarwal,
	Abhinav Kumar, Bryan O'Donoghue
  Cc: linux-media, linux-kernel, linux-arm-msm,
	Deepa Guthyappa Madivalara, kernel test robot

Add ROI support in the iris driver, including control structures
and default parameters. Extend support to set ROI parameters
using custom control V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP.
Implement internal buffer list support for BUF_ROIMB_DELTAQP that holds
ROI MB based delta_qp as expected by the firmware. When an input
arrives queue the corresponding ROI MB delta_qp buffer to firmware.

Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_buffer.c     | 101 ++++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_buffer.h     |  22 +++++
 drivers/media/platform/qcom/iris/iris_ctrls.c      | 104 ++++++++++++++++++++-
 drivers/media/platform/qcom/iris/iris_ctrls.h      |   3 +
 drivers/media/platform/qcom/iris/iris_hfi_common.h |   1 +
 drivers/media/platform/qcom/iris/iris_hfi_gen2.c   |  14 +++
 .../platform/qcom/iris/iris_hfi_gen2_command.c     |  62 ++++++++++++
 .../platform/qcom/iris/iris_hfi_gen2_defines.h     |   3 +
 .../platform/qcom/iris/iris_hfi_gen2_packet.c      |   6 +-
 .../platform/qcom/iris/iris_hfi_gen2_packet.h      |   3 +
 .../platform/qcom/iris/iris_hfi_gen2_response.c    |  29 ++++++
 .../platform/qcom/iris/iris_platform_common.h      |   5 +
 drivers/media/platform/qcom/iris/iris_venc.c       |   4 +
 drivers/media/platform/qcom/iris/iris_venc.h       |   2 +
 drivers/media/platform/qcom/iris/iris_vidc.c       |   2 +
 15 files changed, 357 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index eb8de60c1177f5e1ab83b90a3c8c80e0b4d1f02e..8186a7d011460af4dfcab0f204da08e19b027b33 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -5,11 +5,13 @@
 
 #include <media/v4l2-event.h>
 #include <media/v4l2-mem2mem.h>
+#include <linux/slab.h>
 
 #include "iris_buffer.h"
 #include "iris_instance.h"
 #include "iris_power.h"
 #include "iris_vpu_buffer.h"
+#include "iris_hfi_gen2_defines.h"
 
 #define PIXELS_4K 4096
 #define MAX_WIDTH 4096
@@ -705,6 +707,23 @@ int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane)
 	return iris_destroy_internal_buffers(inst, plane, false);
 }
 
+int iris_destroy_roi_metadata_buffers(struct iris_inst *inst)
+{
+	struct iris_buffer *buf, *next;
+	struct iris_buffers *buffers;
+	int ret = 0;
+
+	if (inst->domain == ENCODER) {
+		buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
+		list_for_each_entry_safe(buf, next, &buffers->list, list) {
+			ret = iris_destroy_internal_buffer(inst, buf);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return ret;
+}
 static int iris_release_internal_buffers(struct iris_inst *inst,
 					 enum iris_buffer_type buffer_type)
 {
@@ -928,3 +947,85 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
 
 	return 0;
 }
+
+static int iris_fill_roi_data(struct iris_inst *inst, struct iris_buffer *buffer)
+{
+	s8 *p_array = (s8 *)inst->fw_caps[ROI_PARAMS].p_array;
+	u32 array_size = inst->fw_caps[ROI_PARAMS].elems;
+	struct metabuf_header *mbuf_hdr = buffer->kvaddr;
+	struct metapayload_header *mbuf_payload_hdr;
+	s16 *p_16;
+	u32 payload_offset;
+
+	memset(mbuf_hdr, 0, sizeof(struct metabuf_header));
+	mbuf_hdr->count = 1;
+	mbuf_hdr->size = sizeof(struct metabuf_header) +
+			 sizeof(struct metapayload_header);
+	mbuf_hdr->version = 1 << 16;
+	mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr + 1);
+	payload_offset = sizeof(struct metabuf_header) +
+			 sizeof(struct metapayload_header);
+
+	memset(mbuf_payload_hdr, 0, sizeof(struct metapayload_header));
+	mbuf_payload_hdr->type = HFI_PROP_ROI_INFO;
+	mbuf_payload_hdr->size = array_size * 2;
+	mbuf_payload_hdr->version = 1 << 16;
+	mbuf_payload_hdr->offset = ALIGN(payload_offset, (u32)256);
+	mbuf_payload_hdr->flags = 0;
+
+	/* Firmware expects 2bytes of delta_Qp, int16_t */
+	p_16 = buffer->kvaddr + mbuf_payload_hdr->offset;
+	for (int i = 0; i < array_size; i++)
+		p_16[i] = p_array[i];
+
+	return 0;
+}
+
+int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst)
+{
+	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
+	struct iris_core *core = inst->core;
+	struct iris_buffer *buffer, *first_buffer, *next;
+	bool found = false;
+	int ret = 0;
+
+	if (!buffers->size)
+		return 0;
+
+	list_for_each_entry_safe(buffer, next, &buffers->list, list) {
+		if (buffer->attr & BUF_ATTR_DEQUEUED) {
+			buffer->attr &= ~BUF_ATTR_DEQUEUED;
+			list_move(&buffer->list, &buffers->list);
+			found = true;
+			break;
+		}
+	}
+	if (!found) {
+		buffer = kzalloc_obj(*buffer);
+		if (!buffer)
+			return -ENOMEM;
+
+		INIT_LIST_HEAD(&buffer->list);
+		buffer->type = BUF_ROIMB_DELTAQP;
+		buffer->index++;
+		buffer->buffer_size = buffers->size;
+		buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE;
+
+		buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
+						 &buffer->device_addr, GFP_KERNEL,
+						 buffer->dma_attrs);
+
+		if (!buffer->kvaddr) {
+			kfree(buffer);
+			return -ENOMEM;
+		}
+		list_add(&buffer->list, &buffers->list);
+	}
+
+	first_buffer = list_first_entry(&buffers->list, struct iris_buffer, list);
+	ret = iris_fill_roi_data(inst, first_buffer);
+	if (ret)
+		return ret;
+
+	return ret;
+}
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h
index ab8e5d953101a786ade20540ee3c3ed226160cbe..df9d018493f3ec37fd5e6b0ba049dba0b5ae76e9 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.h
+++ b/drivers/media/platform/qcom/iris/iris_buffer.h
@@ -28,6 +28,7 @@ struct iris_inst;
  * @BUF_SCRATCH_2: buffer to store encoding context data for HW
  * @BUF_VPSS: buffer to store VPSS context data for HW
  * @BUF_PARTIAL: buffer for AV1 IBC data
+ * @BUF_ROIMB_DELTAQP: metadata buffer for ROI MB DeltaQp
  * @BUF_TYPE_MAX: max buffer types
  */
 enum iris_buffer_type {
@@ -44,6 +45,7 @@ enum iris_buffer_type {
 	BUF_SCRATCH_2,
 	BUF_VPSS,
 	BUF_PARTIAL,
+	BUF_ROIMB_DELTAQP,
 	BUF_TYPE_MAX,
 };
 
@@ -107,6 +109,24 @@ struct iris_buffers {
 	u32			size;
 };
 
+/* Metadata buffer header */
+struct metabuf_header {
+	u32 count;
+	u32 size;
+	u32 version;
+	u32 reserved[5];
+};
+
+/* Metadata buffer payload header */
+struct metapayload_header {
+	u32 type;
+	u32 size;
+	u32 version;
+	u32 offset;
+	u32 flags;
+	u32 reserved[3];
+};
+
 int iris_get_buffer_size(struct iris_inst *inst, enum iris_buffer_type buffer_type);
 void iris_get_internal_buffers(struct iris_inst *inst, u32 plane);
 int iris_create_internal_buffers(struct iris_inst *inst, u32 plane);
@@ -121,5 +141,7 @@ int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf);
 int iris_queue_deferred_buffers(struct iris_inst *inst, enum iris_buffer_type buf_type);
 int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf);
 void iris_vb2_queue_error(struct iris_inst *inst);
+int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst);
+int iris_destroy_roi_metadata_buffers(struct iris_inst *inst);
 
 #endif
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
index bf17d310eac081ffd3a4ad4842c2255ad798d4d8..4da24a29567f7824f91b4d68a8310b248ab62176 100644
--- a/drivers/media/platform/qcom/iris/iris_ctrls.c
+++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
@@ -156,6 +156,10 @@ static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id)
 		return LAYER5_BITRATE_HEVC;
 	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
 		return REQUEST_SYNC_FRAME;
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:
+		return ROI_PARAMS;
+	case V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE:
+		return MB_SIZE;
 	default:
 		return INST_FW_CAP_MAX;
 	}
@@ -301,6 +305,10 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id)
 		return V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR;
 	case REQUEST_SYNC_FRAME:
 		return V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME;
+	case ROI_PARAMS:
+		return V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP;
+	case MB_SIZE:
+		return V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE;
 	default:
 		return 0;
 	}
@@ -327,6 +335,14 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
 
 	inst->fw_caps[cap_id].value = ctrl->val;
 
+	if (inst->fw_caps[cap_id].flags & CAP_FLAG_CUSTOM) {
+		if (cap_id == ROI_PARAMS) {
+			inst->fw_caps[cap_id].p_array =
+				(const void *)ctrl->p_new.p;
+			inst->fw_caps[cap_id].elems = ctrl->new_elems;
+		}
+	}
+
 	if (vb2_is_streaming(q)) {
 		if (cap[cap_id].set)
 			cap[cap_id].set(inst, cap_id);
@@ -335,8 +351,52 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
 	return 0;
 }
 
+static int iris_get_roi_mb_size(struct iris_inst *inst)
+{
+	return inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;
+}
+
+static int iris_op_g_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct iris_inst *inst = container_of(ctrl->handler, struct iris_inst, ctrl_handler);
+	enum platform_inst_fw_cap_type cap_id;
+
+	cap_id = iris_get_cap_id(ctrl->id);
+	if (!iris_valid_cap_id(cap_id))
+		return -EINVAL;
+
+	if (cap_id == MB_SIZE)
+		ctrl->val = iris_get_roi_mb_size(inst);
+
+	return 0;
+}
+
 static const struct v4l2_ctrl_ops iris_ctrl_ops = {
 	.s_ctrl = iris_op_s_ctrl,
+	.g_volatile_ctrl = iris_op_g_ctrl,
+};
+
+const struct v4l2_ctrl_config roi_mbqp_cfg = {
+	.ops = &iris_ctrl_ops,
+	.id = V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP,
+	.name = "Enc Mb ROI Delta QP",
+	.type = V4L2_CTRL_TYPE_S8,
+	.dims = {139264}, /* Max MBPF = 8192 * 4352 / 256 */
+	.min = -31,
+	.max = 30,
+	.def = 0,
+	.step = 1,
+};
+
+static const struct v4l2_ctrl_config roi_mbqp_size = {
+	.ops = &iris_ctrl_ops,
+	.id = V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE,
+	.name = "ROI Mb size",
+	.type = V4L2_CTRL_TYPE_U8,
+	.min = 16,
+	.max = 32,
+	.def = 16,
+	.step = 1,
 };
 
 int iris_ctrls_init(struct iris_inst *inst)
@@ -361,7 +421,7 @@ int iris_ctrls_init(struct iris_inst *inst)
 		return ret;
 
 	for (idx = 1; idx < INST_FW_CAP_MAX; idx++) {
-		struct v4l2_ctrl *ctrl;
+		struct v4l2_ctrl *ctrl = NULL;
 
 		v4l2_id = iris_get_v4l2_id(cap[idx].cap_id);
 		if (!v4l2_id)
@@ -379,6 +439,13 @@ int iris_ctrls_init(struct iris_inst *inst)
 						      cap[idx].max,
 						      ~(cap[idx].step_or_mask),
 						      cap[idx].value);
+		} else if (cap[idx].flags & CAP_FLAG_CUSTOM) {
+			if (cap[idx].cap_id == ROI_PARAMS)
+				ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
+							    &roi_mbqp_cfg, NULL);
+			if (cap[idx].cap_id == MB_SIZE)
+				ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
+							    &roi_mbqp_size, NULL);
 		} else {
 			ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler,
 						 &iris_ctrl_ops,
@@ -1540,3 +1607,38 @@ int iris_set_properties(struct iris_inst *inst, u32 plane)
 
 	return 0;
 }
+
+int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane)
+{
+	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
+	int ret = 0;
+
+	/*subscribe to metadata delivery only if ROI is enabled */
+	if (!inst->fw_caps[ROI_PARAMS].p_array)
+		return ret;
+
+	ret = hfi_ops->session_subscribe_metadata_delivery(inst, plane);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+int iris_set_roi_params(struct iris_inst *inst, u32 plane)
+{
+	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
+	u32 metadata_header_bytes = 256;
+	u32 size = 0;
+	int ret = 0;
+
+	if (!inst->fw_caps[ROI_PARAMS].p_array)
+		return -EINVAL;
+
+	size = inst->fw_caps[ROI_PARAMS].elems * 2 + metadata_header_bytes;
+	buffers->size = ALIGN(size, 4096);
+	iris_hfi_gen2_session_alloc_roi_metadata_buffer(inst);
+	if (ret)
+		return ret;
+
+	return 0;
+}
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.h b/drivers/media/platform/qcom/iris/iris_ctrls.h
index 5180d53d3c904cad460b2760913b475d0ff1bb55..df8fa957a6127338ae63bdd44871e363deeb48fc 100644
--- a/drivers/media/platform/qcom/iris/iris_ctrls.h
+++ b/drivers/media/platform/qcom/iris/iris_ctrls.h
@@ -50,5 +50,8 @@ int iris_set_layer_bitrate(struct iris_inst *inst, enum platform_inst_fw_cap_typ
 int iris_set_req_sync_frame(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id);
 int iris_set_time_delta_based_rc(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id);
 int iris_set_properties(struct iris_inst *inst, u32 plane);
+int iris_set_roi_params(struct iris_inst *inst, u32 plane);
+int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane);
+int iris_set_roi_mb_size(struct iris_inst *inst);
 
 #endif
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
index a27447eb2519962cb958b0e330a6d018310c3450..5692375cdd357f9b00760053ffff5af4bd9cce9c 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
@@ -131,6 +131,7 @@ struct iris_hfi_session_ops {
 	int (*session_drain)(struct iris_inst *inst, u32 plane);
 	int (*session_resume_drain)(struct iris_inst *inst, u32 plane);
 	int (*session_close)(struct iris_inst *inst);
+	int (*session_subscribe_metadata_delivery)(struct iris_inst *inst, u32 plane);
 };
 
 struct hfi_subscription_params {
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
index 3b8fbefb8b93576962abd3850215f7b7fb364930..4d40d6733e5877a302ce63fb0b4a61b8f284e6dc 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
@@ -1782,6 +1782,20 @@ static const struct platform_inst_fw_cap inst_fw_cap_sm8550_enc[] = {
 		.flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_DYNAMIC_ALLOWED,
 		.set = iris_set_req_sync_frame,
 	},
+	{
+		.cap_id = ROI_PARAMS,
+		.step_or_mask = 1,
+		.p_array = NULL,
+		.hfi_id = HFI_PROP_ROI_INFO,
+		.flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_CUSTOM |
+			 CAP_FLAG_DYNAMIC_ALLOWED,
+		.set = iris_set_roi_params,
+	},
+	{
+		.cap_id = MB_SIZE,
+		.step_or_mask = 1,
+		.flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_CUSTOM,
+	},
 };
 
 static const u32 sm8550_vdec_input_config_params_default[] = {
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
index 388a36ff2b07b7bcd8db21d4345bc900356b4ec3..4581a6ea74a708b0d493bc71d2d1822eda078860 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -134,6 +134,7 @@ static u32 iris_hfi_gen2_get_port_from_buf_type(struct iris_inst *inst,
 		switch (buffer_type) {
 		case BUF_INPUT:
 		case BUF_VPSS:
+		case BUF_ROIMB_DELTAQP:
 			return HFI_PORT_RAW;
 		case BUF_OUTPUT:
 		case BUF_BIN:
@@ -1267,6 +1268,8 @@ static u32 iris_hfi_gen2_buf_type_from_driver(u32 domain, enum iris_buffer_type
 		return HFI_BUFFER_VPSS;
 	case BUF_PARTIAL:
 		return HFI_BUFFER_PARTIAL_DATA;
+	case BUF_ROIMB_DELTAQP:
+		return HFI_BUFFER_METADATA;
 	default:
 		return 0;
 	}
@@ -1307,10 +1310,29 @@ static void iris_hfi_gen2_get_buffer(u32 domain, struct iris_buffer *buffer,
 	buf->timestamp = buffer->timestamp;
 }
 
+static struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
+						       enum iris_buffer_type buffer_type, u32 index)
+{
+	struct iris_buffers *buffers = &inst->buffers[buffer_type];
+	struct iris_buffer *buffer = NULL;
+
+	if (list_empty(&buffers->list))
+		return NULL;
+
+	buffer = list_first_entry(&buffers->list, typeof(*buffer), list);
+	if ((buffer->attr & BUF_ATTR_QUEUED) || (buffer->attr & BUF_ATTR_DEQUEUED))
+		return NULL;
+
+	buffer->index = index;
+
+	return buffer;
+}
+
 static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
 {
 	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
 	struct iris_hfi_buffer hfi_buffer;
+	struct iris_hfi_buffer hfi_meta_buffer;
 	u32 port;
 	int ret;
 
@@ -1331,6 +1353,25 @@ static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iri
 					     &hfi_buffer,
 					     sizeof(hfi_buffer));
 
+	/* check if any metadata buffer is available not queued, queueit */
+	if (port == HFI_PORT_RAW) {
+		buffer = iris_queue_metadata_buffers(inst, BUF_ROIMB_DELTAQP, buffer->index);
+		if (buffer) {
+			iris_hfi_gen2_get_buffer(inst->domain, buffer, &hfi_meta_buffer);
+			port = iris_hfi_gen2_get_port_from_buf_type(inst, buffer->type);
+			iris_hfi_gen2_create_packet(inst_hfi_gen2->packet,
+						    HFI_CMD_BUFFER,
+						    HFI_HOST_FLAGS_INTR_REQUIRED,
+						    HFI_PAYLOAD_STRUCTURE,
+						    port,
+						    inst->core->packet_id++,
+						    &hfi_meta_buffer,
+						    sizeof(hfi_meta_buffer));
+
+			buffer->attr |= BUF_ATTR_QUEUED;
+			buffer->attr &= ~BUF_ATTR_DEQUEUED;
+		}
+	}
 	return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
 					inst_hfi_gen2->packet->size);
 }
@@ -1359,6 +1400,26 @@ static int iris_hfi_gen2_session_release_buffer(struct iris_inst *inst, struct i
 					inst_hfi_gen2->packet->size);
 }
 
+static int iris_hfi_gen2_subscribe_metadata_delivery(struct iris_inst *inst, u32 plane)
+{
+	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
+	u32 port = iris_hfi_gen2_get_port(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+	u32 payload[2] = {HFI_MODE_METADATA, HFI_PROP_ROI_INFO};
+
+	iris_hfi_gen2_packet_session_command(inst,
+					     HFI_CMD_DELIVERY_MODE,
+					     (HFI_HOST_FLAGS_RESPONSE_REQUIRED |
+					      HFI_HOST_FLAGS_INTR_REQUIRED),
+					     port,
+					     inst->session_id,
+					     HFI_PAYLOAD_U32_ARRAY,
+					     &payload,
+					     sizeof(u32) * 2);
+
+	return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
+					inst_hfi_gen2->packet->size);
+}
+
 static const struct iris_hfi_session_ops iris_hfi_gen2_session_ops = {
 	.session_open = iris_hfi_gen2_session_open,
 	.session_set_config_params = iris_hfi_gen2_session_set_config_params,
@@ -1372,6 +1433,7 @@ static const struct iris_hfi_session_ops iris_hfi_gen2_session_ops = {
 	.session_drain = iris_hfi_gen2_session_drain,
 	.session_resume_drain = iris_hfi_gen2_session_resume_drain,
 	.session_close = iris_hfi_gen2_session_close,
+	.session_subscribe_metadata_delivery = iris_hfi_gen2_subscribe_metadata_delivery,
 };
 
 static struct iris_inst *iris_hfi_gen2_get_instance(void)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
index f43aea10090d8f1d6ae5c20547e5f6321b2ca203..d0661f5fe6a6f39295dad8950edb2f1a85893216 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
@@ -20,6 +20,7 @@
 #define HFI_CMD_DRAIN				0x01000007
 #define HFI_CMD_RESUME				0x01000008
 #define HFI_CMD_BUFFER				0x01000009
+#define HFI_CMD_DELIVERY_MODE		        0x0100000A
 #define HFI_CMD_SUBSCRIBE_MODE			0x0100000B
 #define HFI_CMD_SETTINGS_CHANGE			0x0100000C
 #define HFI_CMD_PAUSE				0x01000011
@@ -133,6 +134,7 @@ enum hfi_flip {
 #define HFI_PROP_DEC_START_FROM_RAP_FRAME	0x03000169
 #define HFI_PROP_NO_OUTPUT			0x0300016a
 #define HFI_PROP_BUFFER_MARK			0x0300016c
+#define HFI_PROP_ROI_INFO			0x03000173
 #define HFI_PROP_WORST_COMPRESSION_RATIO	0x03000174
 #define HFI_PROP_WORST_COMPLEXITY_FACTOR	0x03000175
 #define HFI_PROP_RAW_RESOLUTION		0x03000178
@@ -174,6 +176,7 @@ enum hfi_flip {
 enum hfi_property_mode_type {
 	HFI_MODE_PORT_SETTINGS_CHANGE		= 0x00000001,
 	HFI_MODE_PROPERTY			= 0x00000002,
+	HFI_MODE_METADATA			= 0x00000004,
 };
 
 enum hfi_color_format {
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
index 0d05dd2afc07d830cc8502ab5f28001312991ba8..aeb0426a05694f219f82145cd84a28287ed3075e 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
@@ -100,9 +100,9 @@ static void iris_hfi_gen2_create_header(struct iris_hfi_header *hdr,
 	hdr->num_packets = 0;
 }
 
-static void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
-					u32 pkt_flags, u32 payload_type, u32 port,
-					u32 packet_id, void *payload, u32 payload_size)
+void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
+				 u32 pkt_flags, u32 payload_type, u32 port,
+				 u32 packet_id, void *payload, u32 payload_size)
 {
 	struct iris_hfi_packet *pkt = (struct iris_hfi_packet *)((u8 *)hdr + hdr->size);
 	u32 pkt_size = sizeof(*pkt) + payload_size;
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
index 25b9582349ca1a0ce6efc0b146a3abb798485c45..613eb500609f745daebdcbdf9a25b85cb9465a79 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
@@ -121,5 +121,8 @@ void iris_hfi_gen2_packet_session_property(struct iris_inst *inst,
 void iris_hfi_gen2_packet_sys_interframe_powercollapse(struct iris_core *core,
 						       struct iris_hfi_header *hdr);
 void iris_hfi_gen2_packet_sys_pc_prep(struct iris_core *core, struct iris_hfi_header *hdr);
+void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
+				 u32 pkt_flags, u32 payload_type, u32 port,
+				 u32 packet_id, void *payload, u32 payload_size);
 
 #endif
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
index 8c2644c7f6e85983d7ad7584fc0cb570e4813ae4..f63e0a7723e8583da818c2b004e367c1dd3d94f7 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
@@ -58,6 +58,8 @@ static u32 iris_hfi_gen2_buf_type_to_driver(struct iris_inst *inst,
 		return BUF_PARTIAL;
 	case HFI_BUFFER_VPSS:
 		return BUF_VPSS;
+	case HFI_BUFFER_METADATA:
+		return BUF_ROIMB_DELTAQP;
 	default:
 		return 0;
 	}
@@ -77,6 +79,7 @@ static bool iris_hfi_gen2_is_valid_hfi_buffer_type(u32 buffer_type)
 	case HFI_BUFFER_PERSIST:
 	case HFI_BUFFER_VPSS:
 	case HFI_BUFFER_PARTIAL_DATA:
+	case HFI_BUFFER_METADATA:
 		return true;
 	default:
 		return false;
@@ -452,6 +455,30 @@ static int iris_hfi_gen2_handle_release_internal_buffer(struct iris_inst *inst,
 	return 0;
 }
 
+static int iris_hfi_gen2_handle_output_metadata_buffer(struct iris_inst *inst,
+						       struct iris_hfi_buffer *buffer)
+{
+	u32 buf_type = iris_hfi_gen2_buf_type_to_driver(inst, HFI_BUFFER_METADATA);
+	struct iris_buffers *buffers = &inst->buffers[buf_type];
+	struct iris_buffer *buf, *iter;
+	bool found = false;
+
+	list_for_each_entry(iter, &buffers->list, list) {
+		if (iter->device_addr == buffer->base_address) {
+			found = true;
+			buf = iter;
+			break;
+		}
+	}
+	if (!found)
+		return -EINVAL;
+
+	buf->attr &= ~BUF_ATTR_QUEUED;
+	buf->attr |= BUF_ATTR_DEQUEUED;
+
+	return 0;
+}
+
 static int iris_hfi_gen2_handle_session_stop(struct iris_inst *inst,
 					     struct iris_hfi_packet *pkt)
 {
@@ -499,6 +526,8 @@ static int iris_hfi_gen2_handle_session_buffer(struct iris_inst *inst,
 			return iris_hfi_gen2_handle_input_buffer(inst, buffer);
 		else if (buffer->type == HFI_BUFFER_BITSTREAM)
 			return iris_hfi_gen2_handle_output_buffer(inst, buffer);
+		else if (buffer->type == HFI_BUFFER_METADATA)
+			return iris_hfi_gen2_handle_output_metadata_buffer(inst, buffer);
 		else
 			return iris_hfi_gen2_handle_release_internal_buffer(inst, buffer);
 	}
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 9748095091461ba13443c63955a42906fa4f050c..9910eb5514b3ae0a6b5349adb071d0a56375102c 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -186,6 +186,8 @@ enum platform_inst_fw_cap_type {
 	LAYER5_BITRATE_HEVC,
 	REQUEST_SYNC_FRAME,
 	TIME_DELTA_BASED_RC,
+	ROI_PARAMS,
+	MB_SIZE,
 	INST_FW_CAP_MAX,
 };
 
@@ -197,6 +199,7 @@ enum platform_inst_fw_cap_flags {
 	CAP_FLAG_CLIENT_SET		= BIT(4),
 	CAP_FLAG_BITMASK		= BIT(5),
 	CAP_FLAG_VOLATILE		= BIT(6),
+	CAP_FLAG_CUSTOM			= BIT(7),
 };
 
 struct platform_inst_fw_cap {
@@ -206,6 +209,8 @@ struct platform_inst_fw_cap {
 	s64 step_or_mask;
 	s64 value;
 	u32 hfi_id;
+	const void *p_array;
+	u32 elems;
 	enum platform_inst_fw_cap_flags flags;
 	int (*set)(struct iris_inst *inst,
 		   enum platform_inst_fw_cap_type cap_id);
diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
index a945992f63aa8a0c40b8d6bc473d0bc90270e6cd..21ba9639d17b2c64e8fa33b4aacbd1946fc4dae2 100644
--- a/drivers/media/platform/qcom/iris/iris_venc.c
+++ b/drivers/media/platform/qcom/iris/iris_venc.c
@@ -498,6 +498,10 @@ int iris_venc_streamon_output(struct iris_inst *inst)
 	if (ret)
 		goto error;
 
+	ret = iris_set_metadata_delivery(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
+	if (ret)
+		goto error;
+
 	ret = iris_alloc_and_queue_persist_bufs(inst, BUF_ARP);
 	if (ret)
 		return ret;
diff --git a/drivers/media/platform/qcom/iris/iris_venc.h b/drivers/media/platform/qcom/iris/iris_venc.h
index 00c1716b2747c7e840c2a3317800d83663744bf0..df97011636740be6cdfe5867f273c5f93408f570 100644
--- a/drivers/media/platform/qcom/iris/iris_venc.h
+++ b/drivers/media/platform/qcom/iris/iris_venc.h
@@ -22,5 +22,7 @@ int iris_venc_streamon_output(struct iris_inst *inst);
 int iris_venc_qbuf(struct iris_inst *inst, struct vb2_v4l2_buffer *vbuf);
 int iris_venc_start_cmd(struct iris_inst *inst);
 int iris_venc_stop_cmd(struct iris_inst *inst);
+struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
+						enum iris_buffer_type buffer_type, u32 index);
 
 #endif
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index fcbc60016beec693f2ce27927a09a2d51494bc38..fae3d113a9dc990f06167ee0b7fb8fab24a0552c 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -180,6 +180,7 @@ int iris_open(struct file *filp)
 	INIT_LIST_HEAD(&inst->buffers[BUF_SCRATCH_2].list);
 	INIT_LIST_HEAD(&inst->buffers[BUF_VPSS].list);
 	INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
+	INIT_LIST_HEAD(&inst->buffers[BUF_ROIMB_DELTAQP].list);
 	init_completion(&inst->completion);
 	init_completion(&inst->flush_completion);
 
@@ -299,6 +300,7 @@ int iris_close(struct file *filp)
 	iris_destroy_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
 	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
+	iris_destroy_roi_metadata_buffers(inst);
 	iris_remove_session(inst);
 	mutex_unlock(&inst->lock);
 	mutex_destroy(&inst->ctx_q_lock);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI
  2026-08-04 20:05   ` [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
@ 2026-08-05  3:49     ` Bryan O'Donoghue
  2026-08-05 16:37       ` Deepa Guthyappa Madivalara
  0 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-08-05  3:49 UTC (permalink / raw)
  To: Deepa Guthyappa Madivalara, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot

On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
> Add custom control, V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP for
> video encoder Region of Interest to allow applications to specify
> different quality levels for specific regions in video frames. Define
> V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE to retrieve supported MB size based on
> codec from the hardware encoder and set the corresponding ROI MB delta_qp
> map that adjusts quantization relative to the frame QP's base value.
> 
> This enables use cases like prioritizing quality for faces in video
> conferencing or important objects in surveillance footage while reducing
> bitrate for less critical areas.
> 
> Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
> ---
>   .../userspace-api/media/v4l/ext-ctrls-codec.rst       | 19 +++++++++++++++++++
>   .../media/v4l/videodev2.h.rst.exceptions              |  1 +
>   .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst    |  4 ++++
>   .../userspace-api/media/v4l/vidioc-queryctrl.rst      |  6 ++++++
>   include/media/v4l2-ctrls.h                            |  1 +
>   include/uapi/linux/v4l2-controls.h                    |  2 ++
>   include/uapi/linux/videodev2.h                        |  3 +++
>   7 files changed, 36 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index ab865a1a6ba929f1725ce2ab0fd3aeee4a69329a..0f8179de8f742253a5152231592fb8b4355298f7 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -1674,6 +1674,25 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
>       Codecs need to always use the specified range, rather then a HW custom range.
>       Applicable to encoders
> 
> +``V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP (__s8)``
> +    This custom control is used to set ROI MB map delta_Qp for whole frame.

"a whole frame" or "the whole frame"

> +    The frame is divided into a grid of MB Size by MB Size pixels (MB Size is
> +    obtained from querying ``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE``). Each block
> +    is configured with delta_Qp in raster order. The valid range for delta_Qp
> +    is encoder dependent.
> +    Applicable to encoders.
> +
> +``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE`` (integer)
> +    A read-only control that reports the MB Size used for the ROI delta_QP map.
> +    The supported MB Size depends on the encoder codec.
> +    Only square macroblocks are supported. The value returned represents the

macro-blocks ?

> +    width and height of each block in pixels. The ROI delta_QP map provided
> +    through the ROI control must use the MB Size reported by this control.
> +    The number of entries in the ROI delta_QP array is determined by the
> +    frame dimensions and the reported MB Size. For example, for a QCIF frame
> +    (176x144) if reported MB Size is 16, the delta_QP array must contain
> +    99 entries (176 x 144 / 16 x 16).
> +
>   .. raw:: latex
> 
>       \normalsize
> diff --git a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> index 6182b4e2d2ee002c0d3562e1cc3bc2fd9c4286d6..b036ed3ec2f0744175cad1cbc4e73fe828e0c955 100644
> --- a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
> @@ -141,6 +141,7 @@ replace symbol V4L2_CTRL_TYPE_STRING :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_U16 :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_U32 :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_U8 :c:type:`V4L.v4l2_ctrl_type`
> +replace symbol V4L2_CTRL_TYPE_S8 :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_MPEG2_SEQUENCE :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_MPEG2_PICTURE :c:type:`V4L.v4l2_ctrl_type`
>   replace symbol V4L2_CTRL_TYPE_MPEG2_QUANTISATION :c:type:`V4L.v4l2_ctrl_type`
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index d78328152b7552f8f0d30a9e2f7c104bfd4b70d3..fe5f8992389c32d8f35f432229ef2f734002644e 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -188,6 +188,10 @@ still cause this situation.
>         - ``p_u32``
>         - A pointer to a matrix control of unsigned 32-bit values. Valid if
>   	this control is of type ``V4L2_CTRL_TYPE_U32``.
> +    * - __s8 *
> +      - ``p_s8``
> +      - A pointer to a matrix control of signed 8-bit values. Valid if
> +	this control is of type ``V4L2_CTRL_TYPE_S8``.
>       * - __s32 *
>         - ``p_s32``
>         - A pointer to a matrix control of signed 32-bit values. Valid if
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> index 82c8b52e771ce9301491b3f752e501a8fe7e2434..57e1652ae72e0324742142446acf32a16b94941c 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
> @@ -425,6 +425,12 @@ See also the examples in :ref:`control`.
>         - any
>         - An unsigned 32-bit valued control ranging from minimum to maximum
>   	inclusive. The step value indicates the increment between values.
> +    * - ``V4L2_CTRL_TYPE_S8``
> +      - any
> +      - any
> +      - any
> +      - A signed 8-bit valued control ranging from minimum to maximum
> +	inclusive. The step value indicates the increment between values.
>       * - ``V4L2_CTRL_TYPE_MPEG2_QUANTISATION``
>         - n/a
>         - n/a
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 327976b14d50b73a631e4d2ed95d9f4d146c45ab..a1806ddbc797efa52e83cd3f685ef70d5b5483d2 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -64,6 +64,7 @@ union v4l2_ctrl_ptr {
>   	s32 *p_s32;
>   	s64 *p_s64;
>   	u8 *p_u8;
> +	s8 *p_s8;
>   	u16 *p_u16;
>   	u32 *p_u32;
>   	char *p_char;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index affec0ab4781507be761cb791ff737460acf9cb7..00a2c48630bd198faa9a2b833a9aa346d8357555 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -920,6 +920,8 @@ enum v4l2_mpeg_video_av1_level {
>   };
> 
>   #define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
> +#define V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP  (V4L2_CID_CODEC_BASE + 658)
> +#define V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE  (V4L2_CID_CODEC_BASE + 659)
> 
>   /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>   #define V4L2_CID_CODEC_CX2341X_BASE				(V4L2_CTRL_CLASS_CODEC | 0x1000)
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5373dba640fae3d19247ab1c74ba336e05dc0fe0..3ed343f8c81e0c9240d5a7e5b2dfe365a529fb6c 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1884,6 +1884,7 @@ struct v4l2_ext_control {
>   		__s64 value64;
>   		char __user *string;
>   		__u8 __user *p_u8;
> +		__s8 __user *p_s8;
>   		__u16 __user *p_u16;
>   		__u32 __user *p_u32;
>   		__s32 __user *p_s32;
> @@ -1961,6 +1962,7 @@ enum v4l2_ctrl_type {
>   	V4L2_CTRL_TYPE_U8	     = 0x0100,
>   	V4L2_CTRL_TYPE_U16	     = 0x0101,
>   	V4L2_CTRL_TYPE_U32	     = 0x0102,
> +	V4L2_CTRL_TYPE_S8		 = 0x0103,
>   	V4L2_CTRL_TYPE_AREA          = 0x0106,
>   	V4L2_CTRL_TYPE_RECT	     = 0x0107,
> 
> @@ -1997,6 +1999,7 @@ enum v4l2_ctrl_type {
>   	V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY = 0x281,
>   	V4L2_CTRL_TYPE_AV1_FRAME	    = 0x282,
>   	V4L2_CTRL_TYPE_AV1_FILM_GRAIN	    = 0x283,
> +

stray \n

>   };
> 
>   /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> 
> --
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control
  2026-08-04 20:05   ` [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control Deepa Guthyappa Madivalara
@ 2026-08-05  3:51     ` Bryan O'Donoghue
  2026-08-05 16:37       ` Deepa Guthyappa Madivalara
  0 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-08-05  3:51 UTC (permalink / raw)
  To: Deepa Guthyappa Madivalara, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot

On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
> @@ -1588,6 +1606,7 @@ void cur_to_new(struct v4l2_ctrl *ctrl)
>   		return;
>   	if (ctrl->is_dyn_array)
>   		ctrl->new_elems = ctrl->elems;
> +
>   	ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new, ctrl->new_elems);
>   }
> 
> @@ -2030,6 +2049,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
>   	case V4L2_CTRL_TYPE_U32:
>   		elem_size = sizeof(u32);
>   		break;
> +	case V4L2_CTRL_TYPE_S8:
> +		elem_size = sizeof(s8);
> +		break;
>   	case V4L2_CTRL_TYPE_MPEG2_SEQUENCE:
>   		elem_size = sizeof(struct v4l2_ctrl_mpeg2_sequence);
>   		break;
> @@ -2247,7 +2269,6 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> 
>   	if (flags & V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX) {
>   		void *ptr = ctrl->p_def.p;
> -
>   		if (p_min.p_const) {
>   			ptr += elem_size;
>   			ctrl->p_min.p = ptr;

You're adding and subtracting newlines here in code you otherwise aren't 
touching.

---
bod


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder
  2026-08-04 20:05   ` [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder Deepa Guthyappa Madivalara
@ 2026-08-05  4:06     ` Bryan O'Donoghue
  2026-08-06 18:36       ` Deepa Guthyappa Madivalara
  0 siblings, 1 reply; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-08-05  4:06 UTC (permalink / raw)
  To: Deepa Guthyappa Madivalara, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot

On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
> Add ROI support in the iris driver, including control structures
> and default parameters. Extend support to set ROI parameters
> using custom control V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP.
> Implement internal buffer list support for BUF_ROIMB_DELTAQP that holds
> ROI MB based delta_qp as expected by the firmware. When an input
> arrives queue the corresponding ROI MB delta_qp buffer to firmware.
> 
> Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_buffer.c     | 101 ++++++++++++++++++++
>   drivers/media/platform/qcom/iris/iris_buffer.h     |  22 +++++
>   drivers/media/platform/qcom/iris/iris_ctrls.c      | 104 ++++++++++++++++++++-
>   drivers/media/platform/qcom/iris/iris_ctrls.h      |   3 +
>   drivers/media/platform/qcom/iris/iris_hfi_common.h |   1 +
>   drivers/media/platform/qcom/iris/iris_hfi_gen2.c   |  14 +++
>   .../platform/qcom/iris/iris_hfi_gen2_command.c     |  62 ++++++++++++
>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |   3 +
>   .../platform/qcom/iris/iris_hfi_gen2_packet.c      |   6 +-
>   .../platform/qcom/iris/iris_hfi_gen2_packet.h      |   3 +
>   .../platform/qcom/iris/iris_hfi_gen2_response.c    |  29 ++++++
>   .../platform/qcom/iris/iris_platform_common.h      |   5 +
>   drivers/media/platform/qcom/iris/iris_venc.c       |   4 +
>   drivers/media/platform/qcom/iris/iris_venc.h       |   2 +
>   drivers/media/platform/qcom/iris/iris_vidc.c       |   2 +
>   15 files changed, 357 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177f5e1ab83b90a3c8c80e0b4d1f02e..8186a7d011460af4dfcab0f204da08e19b027b33 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -5,11 +5,13 @@
> 
>   #include <media/v4l2-event.h>
>   #include <media/v4l2-mem2mem.h>
> +#include <linux/slab.h>
> 
>   #include "iris_buffer.h"
>   #include "iris_instance.h"
>   #include "iris_power.h"
>   #include "iris_vpu_buffer.h"
> +#include "iris_hfi_gen2_defines.h"
> 
>   #define PIXELS_4K 4096
>   #define MAX_WIDTH 4096
> @@ -705,6 +707,23 @@ int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane)
>   	return iris_destroy_internal_buffers(inst, plane, false);
>   }
> 
> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst)
> +{
> +	struct iris_buffer *buf, *next;
> +	struct iris_buffers *buffers;
> +	int ret = 0;
> +
> +	if (inst->domain == ENCODER) {
> +		buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +		list_for_each_entry_safe(buf, next, &buffers->list, list) {
> +			ret = iris_destroy_internal_buffer(inst, buf);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
>   static int iris_release_internal_buffers(struct iris_inst *inst,
>   					 enum iris_buffer_type buffer_type)
>   {
> @@ -928,3 +947,85 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
> 
>   	return 0;
>   }
> +
> +static int iris_fill_roi_data(struct iris_inst *inst, struct iris_buffer *buffer)
> +{
> +	s8 *p_array = (s8 *)inst->fw_caps[ROI_PARAMS].p_array;
> +	u32 array_size = inst->fw_caps[ROI_PARAMS].elems;
> +	struct metabuf_header *mbuf_hdr = buffer->kvaddr;
> +	struct metapayload_header *mbuf_payload_hdr;
> +	s16 *p_16;
> +	u32 payload_offset;
> +
> +	memset(mbuf_hdr, 0, sizeof(struct metabuf_header));
> +	mbuf_hdr->count = 1;
> +	mbuf_hdr->size = sizeof(struct metabuf_header) +
> +			 sizeof(struct metapayload_header);
> +	mbuf_hdr->version = 1 << 16;

That's a weird way to set a header version number though isn't it.

Either say mbuf_hdr->version = 0x10000 or just BIT(16)

> +	mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr + 1);
mbuf_hdr++;
mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr);

would also work

> +	payload_offset = sizeof(struct metabuf_header) +
> +			 sizeof(struct metapayload_header);
> +
> +	memset(mbuf_payload_hdr, 0, sizeof(struct metapayload_header));

> +	mbuf_payload_hdr->type = HFI_PROP_ROI_INFO;
> +	mbuf_payload_hdr->size = array_size * 2;

Instead of * 2 it should be * sizeof(datatype);

BTW the name "metadata" seems strangely generic.

> +	mbuf_payload_hdr->version = 1 << 16;
> +	mbuf_payload_hdr->offset = ALIGN(payload_offset, (u32)256);
> +	mbuf_payload_hdr->flags = 0;
> +
> +	/* Firmware expects 2bytes of delta_Qp, int16_t */
> +	p_16 = buffer->kvaddr + mbuf_payload_hdr->offset;
> +	for (int i = 0; i < array_size; i++)
> +		p_16[i] = p_array[i];
> +
> +	return 0;
> +}
> +
> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst)
> +{
> +	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +	struct iris_core *core = inst->core;
> +	struct iris_buffer *buffer, *first_buffer, *next;
> +	bool found = false;
> +	int ret = 0;
> +
> +	if (!buffers->size)
> +		return 0;
> +
> +	list_for_each_entry_safe(buffer, next, &buffers->list, list) {
> +		if (buffer->attr & BUF_ATTR_DEQUEUED) {
> +			buffer->attr &= ~BUF_ATTR_DEQUEUED;
> +			list_move(&buffer->list, &buffers->list);
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		buffer = kzalloc_obj(*buffer);
> +		if (!buffer)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&buffer->list);
> +		buffer->type = BUF_ROIMB_DELTAQP;
> +		buffer->index++;
> +		buffer->buffer_size = buffers->size;
> +		buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE;
> +
> +		buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
> +						 &buffer->device_addr, GFP_KERNEL,
> +						 buffer->dma_attrs);
> +
> +		if (!buffer->kvaddr) {
> +			kfree(buffer);
> +			return -ENOMEM;
> +		}
> +		list_add(&buffer->list, &buffers->list);
> +	}
> +
> +	first_buffer = list_first_entry(&buffers->list, struct iris_buffer, list);
> +	ret = iris_fill_roi_data(inst, first_buffer);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h
> index ab8e5d953101a786ade20540ee3c3ed226160cbe..df9d018493f3ec37fd5e6b0ba049dba0b5ae76e9 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.h
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.h
> @@ -28,6 +28,7 @@ struct iris_inst;
>    * @BUF_SCRATCH_2: buffer to store encoding context data for HW
>    * @BUF_VPSS: buffer to store VPSS context data for HW
>    * @BUF_PARTIAL: buffer for AV1 IBC data
> + * @BUF_ROIMB_DELTAQP: metadata buffer for ROI MB DeltaQp
>    * @BUF_TYPE_MAX: max buffer types
>    */
>   enum iris_buffer_type {
> @@ -44,6 +45,7 @@ enum iris_buffer_type {
>   	BUF_SCRATCH_2,
>   	BUF_VPSS,
>   	BUF_PARTIAL,
> +	BUF_ROIMB_DELTAQP,
>   	BUF_TYPE_MAX,
>   };
> 
> @@ -107,6 +109,24 @@ struct iris_buffers {
>   	u32			size;
>   };
> 
> +/* Metadata buffer header */
> +struct metabuf_header {
> +	u32 count;
> +	u32 size;
> +	u32 version;
> +	u32 reserved[5];
> +};
> +
> +/* Metadata buffer payload header */
> +struct metapayload_header {
> +	u32 type;
> +	u32 size;
> +	u32 version;
> +	u32 offset;
> +	u32 flags;
> +	u32 reserved[3];
> +};

But these are roi metadata buffers not generic metadata buffers right ?

So the name should describe what it is an roi_medatadata_thing not a 
metadata_thing.

OTOH if these are meant to be generic metadata buffers then the addition 
of these data-structures and the associated logic to support them should 
live in their own patch - describing metadata buffers.

> +
>   int iris_get_buffer_size(struct iris_inst *inst, enum iris_buffer_type buffer_type);
>   void iris_get_internal_buffers(struct iris_inst *inst, u32 plane);
>   int iris_create_internal_buffers(struct iris_inst *inst, u32 plane);
> @@ -121,5 +141,7 @@ int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf);
>   int iris_queue_deferred_buffers(struct iris_inst *inst, enum iris_buffer_type buf_type);
>   int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf);
>   void iris_vb2_queue_error(struct iris_inst *inst);
> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst);
> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst);
> 
>   #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
> index bf17d310eac081ffd3a4ad4842c2255ad798d4d8..4da24a29567f7824f91b4d68a8310b248ab62176 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
> @@ -156,6 +156,10 @@ static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id)
>   		return LAYER5_BITRATE_HEVC;
>   	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
>   		return REQUEST_SYNC_FRAME;
> +	case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:
> +		return ROI_PARAMS;
> +	case V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE:
> +		return MB_SIZE;
>   	default:
>   		return INST_FW_CAP_MAX;
>   	}
> @@ -301,6 +305,10 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id)
>   		return V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR;
>   	case REQUEST_SYNC_FRAME:
>   		return V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME;
> +	case ROI_PARAMS:
> +		return V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP;
> +	case MB_SIZE:
> +		return V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE;
>   	default:
>   		return 0;
>   	}
> @@ -327,6 +335,14 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
> 
>   	inst->fw_caps[cap_id].value = ctrl->val;
> 
> +	if (inst->fw_caps[cap_id].flags & CAP_FLAG_CUSTOM) {
> +		if (cap_id == ROI_PARAMS) {
> +			inst->fw_caps[cap_id].p_array =
> +				(const void *)ctrl->p_new.p;
> +			inst->fw_caps[cap_id].elems = ctrl->new_elems;
> +		}
> +	}
> +
>   	if (vb2_is_streaming(q)) {
>   		if (cap[cap_id].set)
>   			cap[cap_id].set(inst, cap_id);
> @@ -335,8 +351,52 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
>   	return 0;
>   }
> 
> +static int iris_get_roi_mb_size(struct iris_inst *inst)
> +{
> +	return inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;
> +}
> +
> +static int iris_op_g_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct iris_inst *inst = container_of(ctrl->handler, struct iris_inst, ctrl_handler);
> +	enum platform_inst_fw_cap_type cap_id;
> +
> +	cap_id = iris_get_cap_id(ctrl->id);
> +	if (!iris_valid_cap_id(cap_id))
> +		return -EINVAL;
> +
> +	if (cap_id == MB_SIZE)
> +		ctrl->val = iris_get_roi_mb_size(inst);
> +
> +	return 0;
> +}
> +
>   static const struct v4l2_ctrl_ops iris_ctrl_ops = {
>   	.s_ctrl = iris_op_s_ctrl,
> +	.g_volatile_ctrl = iris_op_g_ctrl,
> +};
> +
> +const struct v4l2_ctrl_config roi_mbqp_cfg = {
> +	.ops = &iris_ctrl_ops,
> +	.id = V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP,
> +	.name = "Enc Mb ROI Delta QP",
> +	.type = V4L2_CTRL_TYPE_S8,
> +	.dims = {139264}, /* Max MBPF = 8192 * 4352 / 256 */
> +	.min = -31,
> +	.max = 30,
> +	.def = 0,
> +	.step = 1,
> +};
> +
> +static const struct v4l2_ctrl_config roi_mbqp_size = {
> +	.ops = &iris_ctrl_ops,
> +	.id = V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE,
> +	.name = "ROI Mb size",
> +	.type = V4L2_CTRL_TYPE_U8,
> +	.min = 16,
> +	.max = 32,
> +	.def = 16,
> +	.step = 1,
>   };
> 
>   int iris_ctrls_init(struct iris_inst *inst)
> @@ -361,7 +421,7 @@ int iris_ctrls_init(struct iris_inst *inst)
>   		return ret;
> 
>   	for (idx = 1; idx < INST_FW_CAP_MAX; idx++) {
> -		struct v4l2_ctrl *ctrl;
> +		struct v4l2_ctrl *ctrl = NULL;
> 
>   		v4l2_id = iris_get_v4l2_id(cap[idx].cap_id);
>   		if (!v4l2_id)
> @@ -379,6 +439,13 @@ int iris_ctrls_init(struct iris_inst *inst)
>   						      cap[idx].max,
>   						      ~(cap[idx].step_or_mask),
>   						      cap[idx].value);
> +		} else if (cap[idx].flags & CAP_FLAG_CUSTOM) {
> +			if (cap[idx].cap_id == ROI_PARAMS)
> +				ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
> +							    &roi_mbqp_cfg, NULL);
> +			if (cap[idx].cap_id == MB_SIZE)
> +				ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
> +							    &roi_mbqp_size, NULL);
>   		} else {
>   			ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler,
>   						 &iris_ctrl_ops,
> @@ -1540,3 +1607,38 @@ int iris_set_properties(struct iris_inst *inst, u32 plane)
> 
>   	return 0;
>   }
> +
> +int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane)
> +{
> +	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
> +	int ret = 0;
> +
> +	/*subscribe to metadata delivery only if ROI is enabled */
> +	if (!inst->fw_caps[ROI_PARAMS].p_array)
> +		return ret;
> +
> +	ret = hfi_ops->session_subscribe_metadata_delivery(inst, plane);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> +
> +int iris_set_roi_params(struct iris_inst *inst, u32 plane)
> +{
> +	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +	u32 metadata_header_bytes = 256;
> +	u32 size = 0;
> +	int ret = 0;
> +
> +	if (!inst->fw_caps[ROI_PARAMS].p_array)
> +		return -EINVAL;
> +
> +	size = inst->fw_caps[ROI_PARAMS].elems * 2 + metadata_header_bytes;
> +	buffers->size = ALIGN(size, 4096);
> +	iris_hfi_gen2_session_alloc_roi_metadata_buffer(inst);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.h b/drivers/media/platform/qcom/iris/iris_ctrls.h
> index 5180d53d3c904cad460b2760913b475d0ff1bb55..df8fa957a6127338ae63bdd44871e363deeb48fc 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.h
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.h
> @@ -50,5 +50,8 @@ int iris_set_layer_bitrate(struct iris_inst *inst, enum platform_inst_fw_cap_typ
>   int iris_set_req_sync_frame(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id);
>   int iris_set_time_delta_based_rc(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id);
>   int iris_set_properties(struct iris_inst *inst, u32 plane);
> +int iris_set_roi_params(struct iris_inst *inst, u32 plane);
> +int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane);
> +int iris_set_roi_mb_size(struct iris_inst *inst);
> 
>   #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> index a27447eb2519962cb958b0e330a6d018310c3450..5692375cdd357f9b00760053ffff5af4bd9cce9c 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
> @@ -131,6 +131,7 @@ struct iris_hfi_session_ops {
>   	int (*session_drain)(struct iris_inst *inst, u32 plane);
>   	int (*session_resume_drain)(struct iris_inst *inst, u32 plane);
>   	int (*session_close)(struct iris_inst *inst);
> +	int (*session_subscribe_metadata_delivery)(struct iris_inst *inst, u32 plane);
>   };
> 
>   struct hfi_subscription_params {
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> index 3b8fbefb8b93576962abd3850215f7b7fb364930..4d40d6733e5877a302ce63fb0b4a61b8f284e6dc 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> @@ -1782,6 +1782,20 @@ static const struct platform_inst_fw_cap inst_fw_cap_sm8550_enc[] = {
>   		.flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_DYNAMIC_ALLOWED,
>   		.set = iris_set_req_sync_frame,
>   	},
> +	{
> +		.cap_id = ROI_PARAMS,
> +		.step_or_mask = 1,
> +		.p_array = NULL,
> +		.hfi_id = HFI_PROP_ROI_INFO,
> +		.flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_CUSTOM |
> +			 CAP_FLAG_DYNAMIC_ALLOWED,
> +		.set = iris_set_roi_params,
> +	},
> +	{
> +		.cap_id = MB_SIZE,
> +		.step_or_mask = 1,
> +		.flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_CUSTOM,
> +	},

The enumeration of the platform and the implementation of the logic 
should be separated into different patches.

>   };
> 
>   static const u32 sm8550_vdec_input_config_params_default[] = {
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> index 388a36ff2b07b7bcd8db21d4345bc900356b4ec3..4581a6ea74a708b0d493bc71d2d1822eda078860 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -134,6 +134,7 @@ static u32 iris_hfi_gen2_get_port_from_buf_type(struct iris_inst *inst,
>   		switch (buffer_type) {
>   		case BUF_INPUT:
>   		case BUF_VPSS:
> +		case BUF_ROIMB_DELTAQP:
>   			return HFI_PORT_RAW;
>   		case BUF_OUTPUT:
>   		case BUF_BIN:
> @@ -1267,6 +1268,8 @@ static u32 iris_hfi_gen2_buf_type_from_driver(u32 domain, enum iris_buffer_type
>   		return HFI_BUFFER_VPSS;
>   	case BUF_PARTIAL:
>   		return HFI_BUFFER_PARTIAL_DATA;
> +	case BUF_ROIMB_DELTAQP:
> +		return HFI_BUFFER_METADATA;
>   	default:
>   		return 0;
>   	}
> @@ -1307,10 +1310,29 @@ static void iris_hfi_gen2_get_buffer(u32 domain, struct iris_buffer *buffer,
>   	buf->timestamp = buffer->timestamp;
>   }
> 
> +static struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
> +						       enum iris_buffer_type buffer_type, u32 index)
> +{
> +	struct iris_buffers *buffers = &inst->buffers[buffer_type];
> +	struct iris_buffer *buffer = NULL;
> +
> +	if (list_empty(&buffers->list))
> +		return NULL;
> +
> +	buffer = list_first_entry(&buffers->list, typeof(*buffer), list);
> +	if ((buffer->attr & BUF_ATTR_QUEUED) || (buffer->attr & BUF_ATTR_DEQUEUED))
> +		return NULL;
> +
> +	buffer->index = index;
> +
> +	return buffer;
> +}
> +
>   static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
>   {
>   	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
>   	struct iris_hfi_buffer hfi_buffer;
> +	struct iris_hfi_buffer hfi_meta_buffer;
>   	u32 port;
>   	int ret;
> 
> @@ -1331,6 +1353,25 @@ static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iri
>   					     &hfi_buffer,
>   					     sizeof(hfi_buffer));
> 
> +	/* check if any metadata buffer is available not queued, queueit */
> +	if (port == HFI_PORT_RAW) {
> +		buffer = iris_queue_metadata_buffers(inst, BUF_ROIMB_DELTAQP, buffer->index);
> +		if (buffer) {
> +			iris_hfi_gen2_get_buffer(inst->domain, buffer, &hfi_meta_buffer);
> +			port = iris_hfi_gen2_get_port_from_buf_type(inst, buffer->type);
> +			iris_hfi_gen2_create_packet(inst_hfi_gen2->packet,
> +						    HFI_CMD_BUFFER,
> +						    HFI_HOST_FLAGS_INTR_REQUIRED,
> +						    HFI_PAYLOAD_STRUCTURE,
> +						    port,
> +						    inst->core->packet_id++,
> +						    &hfi_meta_buffer,
> +						    sizeof(hfi_meta_buffer));
> +
> +			buffer->attr |= BUF_ATTR_QUEUED;
> +			buffer->attr &= ~BUF_ATTR_DEQUEUED;
> +		}
> +	}
>   	return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
>   					inst_hfi_gen2->packet->size);
>   }
> @@ -1359,6 +1400,26 @@ static int iris_hfi_gen2_session_release_buffer(struct iris_inst *inst, struct i
>   					inst_hfi_gen2->packet->size);
>   }
> 
> +static int iris_hfi_gen2_subscribe_metadata_delivery(struct iris_inst *inst, u32 plane)
> +{
> +	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
> +	u32 port = iris_hfi_gen2_get_port(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> +	u32 payload[2] = {HFI_MODE_METADATA, HFI_PROP_ROI_INFO};
> +
> +	iris_hfi_gen2_packet_session_command(inst,
> +					     HFI_CMD_DELIVERY_MODE,
> +					     (HFI_HOST_FLAGS_RESPONSE_REQUIRED |
> +					      HFI_HOST_FLAGS_INTR_REQUIRED),
> +					     port,
> +					     inst->session_id,
> +					     HFI_PAYLOAD_U32_ARRAY,
> +					     &payload,
> +					     sizeof(u32) * 2);
> +
> +	return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
> +					inst_hfi_gen2->packet->size);
> +}
> +
>   static const struct iris_hfi_session_ops iris_hfi_gen2_session_ops = {
>   	.session_open = iris_hfi_gen2_session_open,
>   	.session_set_config_params = iris_hfi_gen2_session_set_config_params,
> @@ -1372,6 +1433,7 @@ static const struct iris_hfi_session_ops iris_hfi_gen2_session_ops = {
>   	.session_drain = iris_hfi_gen2_session_drain,
>   	.session_resume_drain = iris_hfi_gen2_session_resume_drain,
>   	.session_close = iris_hfi_gen2_session_close,
> +	.session_subscribe_metadata_delivery = iris_hfi_gen2_subscribe_metadata_delivery,
>   };
> 
>   static struct iris_inst *iris_hfi_gen2_get_instance(void)
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> index f43aea10090d8f1d6ae5c20547e5f6321b2ca203..d0661f5fe6a6f39295dad8950edb2f1a85893216 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
> @@ -20,6 +20,7 @@
>   #define HFI_CMD_DRAIN				0x01000007
>   #define HFI_CMD_RESUME				0x01000008
>   #define HFI_CMD_BUFFER				0x01000009
> +#define HFI_CMD_DELIVERY_MODE		        0x0100000A
>   #define HFI_CMD_SUBSCRIBE_MODE			0x0100000B
>   #define HFI_CMD_SETTINGS_CHANGE			0x0100000C
>   #define HFI_CMD_PAUSE				0x01000011
> @@ -133,6 +134,7 @@ enum hfi_flip {
>   #define HFI_PROP_DEC_START_FROM_RAP_FRAME	0x03000169
>   #define HFI_PROP_NO_OUTPUT			0x0300016a
>   #define HFI_PROP_BUFFER_MARK			0x0300016c
> +#define HFI_PROP_ROI_INFO			0x03000173
>   #define HFI_PROP_WORST_COMPRESSION_RATIO	0x03000174
>   #define HFI_PROP_WORST_COMPLEXITY_FACTOR	0x03000175
>   #define HFI_PROP_RAW_RESOLUTION		0x03000178
> @@ -174,6 +176,7 @@ enum hfi_flip {
>   enum hfi_property_mode_type {
>   	HFI_MODE_PORT_SETTINGS_CHANGE		= 0x00000001,
>   	HFI_MODE_PROPERTY			= 0x00000002,
> +	HFI_MODE_METADATA			= 0x00000004,
>   };
> 
>   enum hfi_color_format {
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
> index 0d05dd2afc07d830cc8502ab5f28001312991ba8..aeb0426a05694f219f82145cd84a28287ed3075e 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
> @@ -100,9 +100,9 @@ static void iris_hfi_gen2_create_header(struct iris_hfi_header *hdr,
>   	hdr->num_packets = 0;
>   }
> 
> -static void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
> -					u32 pkt_flags, u32 payload_type, u32 port,
> -					u32 packet_id, void *payload, u32 payload_size)
> +void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
> +				 u32 pkt_flags, u32 payload_type, u32 port,
> +				 u32 packet_id, void *payload, u32 payload_size)
>   {
>   	struct iris_hfi_packet *pkt = (struct iris_hfi_packet *)((u8 *)hdr + hdr->size);
>   	u32 pkt_size = sizeof(*pkt) + payload_size;
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
> index 25b9582349ca1a0ce6efc0b146a3abb798485c45..613eb500609f745daebdcbdf9a25b85cb9465a79 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
> @@ -121,5 +121,8 @@ void iris_hfi_gen2_packet_session_property(struct iris_inst *inst,
>   void iris_hfi_gen2_packet_sys_interframe_powercollapse(struct iris_core *core,
>   						       struct iris_hfi_header *hdr);
>   void iris_hfi_gen2_packet_sys_pc_prep(struct iris_core *core, struct iris_hfi_header *hdr);
> +void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 pkt_type,
> +				 u32 pkt_flags, u32 payload_type, u32 port,
> +				 u32 packet_id, void *payload, u32 payload_size);
> 
>   #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> index 8c2644c7f6e85983d7ad7584fc0cb570e4813ae4..f63e0a7723e8583da818c2b004e367c1dd3d94f7 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> @@ -58,6 +58,8 @@ static u32 iris_hfi_gen2_buf_type_to_driver(struct iris_inst *inst,
>   		return BUF_PARTIAL;
>   	case HFI_BUFFER_VPSS:
>   		return BUF_VPSS;
> +	case HFI_BUFFER_METADATA:
> +		return BUF_ROIMB_DELTAQP;
>   	default:
>   		return 0;
>   	}
> @@ -77,6 +79,7 @@ static bool iris_hfi_gen2_is_valid_hfi_buffer_type(u32 buffer_type)
>   	case HFI_BUFFER_PERSIST:
>   	case HFI_BUFFER_VPSS:
>   	case HFI_BUFFER_PARTIAL_DATA:
> +	case HFI_BUFFER_METADATA:
>   		return true;
>   	default:
>   		return false;
> @@ -452,6 +455,30 @@ static int iris_hfi_gen2_handle_release_internal_buffer(struct iris_inst *inst,
>   	return 0;
>   }
> 
> +static int iris_hfi_gen2_handle_output_metadata_buffer(struct iris_inst *inst,
> +						       struct iris_hfi_buffer *buffer)
> +{
> +	u32 buf_type = iris_hfi_gen2_buf_type_to_driver(inst, HFI_BUFFER_METADATA);
> +	struct iris_buffers *buffers = &inst->buffers[buf_type];
> +	struct iris_buffer *buf, *iter;
> +	bool found = false;
> +
> +	list_for_each_entry(iter, &buffers->list, list) {
> +		if (iter->device_addr == buffer->base_address) {
> +			found = true;
> +			buf = iter;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		return -EINVAL;
> +
> +	buf->attr &= ~BUF_ATTR_QUEUED;
> +	buf->attr |= BUF_ATTR_DEQUEUED;
> +
> +	return 0;
> +}
> +
>   static int iris_hfi_gen2_handle_session_stop(struct iris_inst *inst,
>   					     struct iris_hfi_packet *pkt)
>   {
> @@ -499,6 +526,8 @@ static int iris_hfi_gen2_handle_session_buffer(struct iris_inst *inst,
>   			return iris_hfi_gen2_handle_input_buffer(inst, buffer);
>   		else if (buffer->type == HFI_BUFFER_BITSTREAM)
>   			return iris_hfi_gen2_handle_output_buffer(inst, buffer);
> +		else if (buffer->type == HFI_BUFFER_METADATA)
> +			return iris_hfi_gen2_handle_output_metadata_buffer(inst, buffer);
>   		else
>   			return iris_hfi_gen2_handle_release_internal_buffer(inst, buffer);
>   	}
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 9748095091461ba13443c63955a42906fa4f050c..9910eb5514b3ae0a6b5349adb071d0a56375102c 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -186,6 +186,8 @@ enum platform_inst_fw_cap_type {
>   	LAYER5_BITRATE_HEVC,
>   	REQUEST_SYNC_FRAME,
>   	TIME_DELTA_BASED_RC,
> +	ROI_PARAMS,
> +	MB_SIZE,
>   	INST_FW_CAP_MAX,
>   };
> 
> @@ -197,6 +199,7 @@ enum platform_inst_fw_cap_flags {
>   	CAP_FLAG_CLIENT_SET		= BIT(4),
>   	CAP_FLAG_BITMASK		= BIT(5),
>   	CAP_FLAG_VOLATILE		= BIT(6),
> +	CAP_FLAG_CUSTOM			= BIT(7),
>   };
> 
>   struct platform_inst_fw_cap {
> @@ -206,6 +209,8 @@ struct platform_inst_fw_cap {
>   	s64 step_or_mask;
>   	s64 value;
>   	u32 hfi_id;
> +	const void *p_array;
> +	u32 elems;
>   	enum platform_inst_fw_cap_flags flags;
>   	int (*set)(struct iris_inst *inst,
>   		   enum platform_inst_fw_cap_type cap_id);
> diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
> index a945992f63aa8a0c40b8d6bc473d0bc90270e6cd..21ba9639d17b2c64e8fa33b4aacbd1946fc4dae2 100644
> --- a/drivers/media/platform/qcom/iris/iris_venc.c
> +++ b/drivers/media/platform/qcom/iris/iris_venc.c
> @@ -498,6 +498,10 @@ int iris_venc_streamon_output(struct iris_inst *inst)
>   	if (ret)
>   		goto error;
> 
> +	ret = iris_set_metadata_delivery(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (ret)
> +		goto error;
> +
>   	ret = iris_alloc_and_queue_persist_bufs(inst, BUF_ARP);
>   	if (ret)
>   		return ret;
> diff --git a/drivers/media/platform/qcom/iris/iris_venc.h b/drivers/media/platform/qcom/iris/iris_venc.h
> index 00c1716b2747c7e840c2a3317800d83663744bf0..df97011636740be6cdfe5867f273c5f93408f570 100644
> --- a/drivers/media/platform/qcom/iris/iris_venc.h
> +++ b/drivers/media/platform/qcom/iris/iris_venc.h
> @@ -22,5 +22,7 @@ int iris_venc_streamon_output(struct iris_inst *inst);
>   int iris_venc_qbuf(struct iris_inst *inst, struct vb2_v4l2_buffer *vbuf);
>   int iris_venc_start_cmd(struct iris_inst *inst);
>   int iris_venc_stop_cmd(struct iris_inst *inst);
> +struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
> +						enum iris_buffer_type buffer_type, u32 index);
> 
>   #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index fcbc60016beec693f2ce27927a09a2d51494bc38..fae3d113a9dc990f06167ee0b7fb8fab24a0552c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -180,6 +180,7 @@ int iris_open(struct file *filp)
>   	INIT_LIST_HEAD(&inst->buffers[BUF_SCRATCH_2].list);
>   	INIT_LIST_HEAD(&inst->buffers[BUF_VPSS].list);
>   	INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
> +	INIT_LIST_HEAD(&inst->buffers[BUF_ROIMB_DELTAQP].list);
>   	init_completion(&inst->completion);
>   	init_completion(&inst->flush_completion);
> 
> @@ -299,6 +300,7 @@ int iris_close(struct file *filp)
>   	iris_destroy_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>   	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>   	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	iris_destroy_roi_metadata_buffers(inst);
>   	iris_remove_session(inst);
>   	mutex_unlock(&inst->lock);
>   	mutex_destroy(&inst->ctx_q_lock);
> 
> --
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/3] Implement Region of Interest(ROI) support
  2026-08-04 20:05 ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
                     ` (2 preceding siblings ...)
  2026-08-04 20:05   ` [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder Deepa Guthyappa Madivalara
@ 2026-08-05  4:09   ` Bryan O'Donoghue
  2026-08-05 16:39     ` Deepa Guthyappa Madivalara
  2026-08-06  5:24     ` Vikash Garodia
  3 siblings, 2 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2026-08-05  4:09 UTC (permalink / raw)
  To: Deepa Guthyappa Madivalara, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot

On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
> Hi all,
> 
> This patch set implements region of interest(ROI) support for
> video encoder to be configured as a MB based delta_qp for
> the whole frame. Custom controls V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP
> and V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE are implemented to achieve this.
> Corresponding support is added in iris encoder to implement this
> feature.
> 
> This submission and design is based on the feedback received
> from community for the RFC[1] series posted earlier.
> Initial testing is done by adding support to v4l2-ctl[2] to test this
> control setting and its functionality.
> 
> [1]https://lore.kernel.org/linux-media/20260113-iris_enc_roi-v1-0-6c86eba38587@oss.qualcomm.com
> [2]https://lore.kernel.org/linux-media/20260616-enc_roi_enable-v1-1-252792d27894@oss.qualcomm.com
as a sidenote can someone in qcom please, please, please do something 
about Dikshita's email bouncing ?

"Address not found
Your message wasn't delivered to dikshita.agarwal@oss.qualcomm.com 
because the address couldn't be found, or is unable to receive mail.
LEARN MORE"

Understood she is on some kind of sabattical - hope she is enjoying it 
but can you guys either remove her from MAINTAINERS while the email is 
bouncing or fix your email system to stop spamming responders with the 
above response ?

Everybody responding is getting these bounces...

---
bod

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI
  2026-08-05  3:49     ` Bryan O'Donoghue
@ 2026-08-05 16:37       ` Deepa Guthyappa Madivalara
  0 siblings, 0 replies; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-05 16:37 UTC (permalink / raw)
  To: Bryan O'Donoghue, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot


On 8/4/2026 8:49 PM, Bryan O'Donoghue wrote:
> On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
>> Add custom control, V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP for
>> video encoder Region of Interest to allow applications to specify
>> different quality levels for specific regions in video frames. Define
>> V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE to retrieve supported MB size based on
>> codec from the hardware encoder and set the corresponding ROI MB 
>> delta_qp
>> map that adjusts quantization relative to the frame QP's base value.
>>
>> This enables use cases like prioritizing quality for faces in video
>> conferencing or important objects in surveillance footage while reducing
>> bitrate for less critical areas.
>>
>> Signed-off-by: Deepa Guthyappa Madivalara 
>> <deepa.madivalara@oss.qualcomm.com>
>> ---
>>   .../userspace-api/media/v4l/ext-ctrls-codec.rst       | 19 
>> +++++++++++++++++++
>>   .../media/v4l/videodev2.h.rst.exceptions              |  1 +
>>   .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst    |  4 ++++
>>   .../userspace-api/media/v4l/vidioc-queryctrl.rst      |  6 ++++++
>>   include/media/v4l2-ctrls.h                            |  1 +
>>   include/uapi/linux/v4l2-controls.h                    |  2 ++
>>   include/uapi/linux/videodev2.h                        |  3 +++
>>   7 files changed, 36 insertions(+)
>>
>> diff --git 
>> a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst 
>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> index 
>> ab865a1a6ba929f1725ce2ab0fd3aeee4a69329a..0f8179de8f742253a5152231592fb8b4355298f7 
>> 100644
>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>> @@ -1674,6 +1674,25 @@ enum 
>> v4l2_mpeg_video_h264_hierarchical_coding_type -
>>       Codecs need to always use the specified range, rather then a HW 
>> custom range.
>>       Applicable to encoders
>>
>> +``V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP (__s8)``
>> +    This custom control is used to set ROI MB map delta_Qp for whole 
>> frame.
>
> "a whole frame" or "the whole frame"
>
Will update to "the whole frame in v4"
>> +    The frame is divided into a grid of MB Size by MB Size pixels 
>> (MB Size is
>> +    obtained from querying ``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE``). 
>> Each block
>> +    is configured with delta_Qp in raster order. The valid range for 
>> delta_Qp
>> +    is encoder dependent.
>> +    Applicable to encoders.
>> +
>> +``V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE`` (integer)
>> +    A read-only control that reports the MB Size used for the ROI 
>> delta_QP map.
>> +    The supported MB Size depends on the encoder codec.
>> +    Only square macroblocks are supported. The value returned 
>> represents the
>
> macro-blocks ?
I will use MBs to keep it consistent. (in v4)
>
>> +    width and height of each block in pixels. The ROI delta_QP map 
>> provided
>> +    through the ROI control must use the MB Size reported by this 
>> control.
>> +    The number of entries in the ROI delta_QP array is determined by 
>> the
>> +    frame dimensions and the reported MB Size. For example, for a 
>> QCIF frame
>> +    (176x144) if reported MB Size is 16, the delta_QP array must 
>> contain
>> +    99 entries (176 x 144 / 16 x 16).
>> +
>>   .. raw:: latex
>>
>>       \normalsize
>> diff --git 
>> a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions 
>> b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
>> index 
>> 6182b4e2d2ee002c0d3562e1cc3bc2fd9c4286d6..b036ed3ec2f0744175cad1cbc4e73fe828e0c955 
>> 100644
>> --- a/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
>> +++ b/Documentation/userspace-api/media/v4l/videodev2.h.rst.exceptions
>> @@ -141,6 +141,7 @@ replace symbol V4L2_CTRL_TYPE_STRING 
>> :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_U16 :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_U32 :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_U8 :c:type:`V4L.v4l2_ctrl_type`
>> +replace symbol V4L2_CTRL_TYPE_S8 :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_MPEG2_SEQUENCE 
>> :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_MPEG2_PICTURE 
>> :c:type:`V4L.v4l2_ctrl_type`
>>   replace symbol V4L2_CTRL_TYPE_MPEG2_QUANTISATION 
>> :c:type:`V4L.v4l2_ctrl_type`
>> diff --git 
>> a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst 
>> b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
>> index 
>> d78328152b7552f8f0d30a9e2f7c104bfd4b70d3..fe5f8992389c32d8f35f432229ef2f734002644e 
>> 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
>> @@ -188,6 +188,10 @@ still cause this situation.
>>         - ``p_u32``
>>         - A pointer to a matrix control of unsigned 32-bit values. 
>> Valid if
>>       this control is of type ``V4L2_CTRL_TYPE_U32``.
>> +    * - __s8 *
>> +      - ``p_s8``
>> +      - A pointer to a matrix control of signed 8-bit values. Valid if
>> +    this control is of type ``V4L2_CTRL_TYPE_S8``.
>>       * - __s32 *
>>         - ``p_s32``
>>         - A pointer to a matrix control of signed 32-bit values. 
>> Valid if
>> diff --git 
>> a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst 
>> b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> index 
>> 82c8b52e771ce9301491b3f752e501a8fe7e2434..57e1652ae72e0324742142446acf32a16b94941c 
>> 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst
>> @@ -425,6 +425,12 @@ See also the examples in :ref:`control`.
>>         - any
>>         - An unsigned 32-bit valued control ranging from minimum to 
>> maximum
>>       inclusive. The step value indicates the increment between values.
>> +    * - ``V4L2_CTRL_TYPE_S8``
>> +      - any
>> +      - any
>> +      - any
>> +      - A signed 8-bit valued control ranging from minimum to maximum
>> +    inclusive. The step value indicates the increment between values.
>>       * - ``V4L2_CTRL_TYPE_MPEG2_QUANTISATION``
>>         - n/a
>>         - n/a
>> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
>> index 
>> 327976b14d50b73a631e4d2ed95d9f4d146c45ab..a1806ddbc797efa52e83cd3f685ef70d5b5483d2 
>> 100644
>> --- a/include/media/v4l2-ctrls.h
>> +++ b/include/media/v4l2-ctrls.h
>> @@ -64,6 +64,7 @@ union v4l2_ctrl_ptr {
>>       s32 *p_s32;
>>       s64 *p_s64;
>>       u8 *p_u8;
>> +    s8 *p_s8;
>>       u16 *p_u16;
>>       u32 *p_u32;
>>       char *p_char;
>> diff --git a/include/uapi/linux/v4l2-controls.h 
>> b/include/uapi/linux/v4l2-controls.h
>> index 
>> affec0ab4781507be761cb791ff737460acf9cb7..00a2c48630bd198faa9a2b833a9aa346d8357555 
>> 100644
>> --- a/include/uapi/linux/v4l2-controls.h
>> +++ b/include/uapi/linux/v4l2-controls.h
>> @@ -920,6 +920,8 @@ enum v4l2_mpeg_video_av1_level {
>>   };
>>
>>   #define V4L2_CID_MPEG_VIDEO_AVERAGE_QP  (V4L2_CID_CODEC_BASE + 657)
>> +#define V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP (V4L2_CID_CODEC_BASE + 658)
>> +#define V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE  (V4L2_CID_CODEC_BASE + 659)
>>
>>   /*  MPEG-class control IDs specific to the CX2341x driver as 
>> defined by V4L2 */
>>   #define V4L2_CID_CODEC_CX2341X_BASE (V4L2_CTRL_CLASS_CODEC | 0x1000)
>> diff --git a/include/uapi/linux/videodev2.h 
>> b/include/uapi/linux/videodev2.h
>> index 
>> 5373dba640fae3d19247ab1c74ba336e05dc0fe0..3ed343f8c81e0c9240d5a7e5b2dfe365a529fb6c 
>> 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -1884,6 +1884,7 @@ struct v4l2_ext_control {
>>           __s64 value64;
>>           char __user *string;
>>           __u8 __user *p_u8;
>> +        __s8 __user *p_s8;
>>           __u16 __user *p_u16;
>>           __u32 __user *p_u32;
>>           __s32 __user *p_s32;
>> @@ -1961,6 +1962,7 @@ enum v4l2_ctrl_type {
>>       V4L2_CTRL_TYPE_U8         = 0x0100,
>>       V4L2_CTRL_TYPE_U16         = 0x0101,
>>       V4L2_CTRL_TYPE_U32         = 0x0102,
>> +    V4L2_CTRL_TYPE_S8         = 0x0103,
>>       V4L2_CTRL_TYPE_AREA          = 0x0106,
>>       V4L2_CTRL_TYPE_RECT         = 0x0107,
>>
>> @@ -1997,6 +1999,7 @@ enum v4l2_ctrl_type {
>>       V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY = 0x281,
>>       V4L2_CTRL_TYPE_AV1_FRAME        = 0x282,
>>       V4L2_CTRL_TYPE_AV1_FILM_GRAIN        = 0x283,
>> +
>
> stray \n
>
sorry, will fix this in v4
>>   };
>>
>>   /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
>>
>> -- 
>> 2.34.1
>>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control
  2026-08-05  3:51     ` Bryan O'Donoghue
@ 2026-08-05 16:37       ` Deepa Guthyappa Madivalara
  0 siblings, 0 replies; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-05 16:37 UTC (permalink / raw)
  To: Bryan O'Donoghue, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot


On 8/4/2026 8:51 PM, Bryan O'Donoghue wrote:
> On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
>> @@ -1588,6 +1606,7 @@ void cur_to_new(struct v4l2_ctrl *ctrl)
>>           return;
>>       if (ctrl->is_dyn_array)
>>           ctrl->new_elems = ctrl->elems;
>> +
>>       ptr_to_ptr(ctrl, ctrl->p_cur, ctrl->p_new, ctrl->new_elems);
>>   }
>>
>> @@ -2030,6 +2049,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
>> v4l2_ctrl_handler *hdl,
>>       case V4L2_CTRL_TYPE_U32:
>>           elem_size = sizeof(u32);
>>           break;
>> +    case V4L2_CTRL_TYPE_S8:
>> +        elem_size = sizeof(s8);
>> +        break;
>>       case V4L2_CTRL_TYPE_MPEG2_SEQUENCE:
>>           elem_size = sizeof(struct v4l2_ctrl_mpeg2_sequence);
>>           break;
>> @@ -2247,7 +2269,6 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
>> v4l2_ctrl_handler *hdl,
>>
>>       if (flags & V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX) {
>>           void *ptr = ctrl->p_def.p;
>> -
>>           if (p_min.p_const) {
>>               ptr += elem_size;
>>               ctrl->p_min.p = ptr;
>
> You're adding and subtracting newlines here in code you otherwise 
> aren't touching.
>
> ---
> bod
>
ack, will fix this in v4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/3] Implement Region of Interest(ROI) support
  2026-08-05  4:09   ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Bryan O'Donoghue
@ 2026-08-05 16:39     ` Deepa Guthyappa Madivalara
  2026-08-06  5:24     ` Vikash Garodia
  1 sibling, 0 replies; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-05 16:39 UTC (permalink / raw)
  To: Bryan O'Donoghue, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot


On 8/4/2026 9:09 PM, Bryan O'Donoghue wrote:
> On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
>> Hi all,
>>
>> This patch set implements region of interest(ROI) support for
>> video encoder to be configured as a MB based delta_qp for
>> the whole frame. Custom controls V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP
>> and V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE are implemented to achieve this.
>> Corresponding support is added in iris encoder to implement this
>> feature.
>>
>> This submission and design is based on the feedback received
>> from community for the RFC[1] series posted earlier.
>> Initial testing is done by adding support to v4l2-ctl[2] to test this
>> control setting and its functionality.
>>
>> [1]https://lore.kernel.org/linux-media/20260113-iris_enc_roi-v1-0-6c86eba38587@oss.qualcomm.com 
>>
>> [2]https://lore.kernel.org/linux-media/20260616-enc_roi_enable-v1-1-252792d27894@oss.qualcomm.com 
>>
> as a sidenote can someone in qcom please, please, please do something 
> about Dikshita's email bouncing ?
>
> "Address not found
> Your message wasn't delivered to dikshita.agarwal@oss.qualcomm.com 
> because the address couldn't be found, or is unable to receive mail.
> LEARN MORE"
>
> Understood she is on some kind of sabattical - hope she is enjoying it 
> but can you guys either remove her from MAINTAINERS while the email is 
> bouncing or fix your email system to stop spamming responders with the 
> above response ?
>
> Everybody responding is getting these bounces...
>
> ---
> bod
Sure, will check with team internally and do as they suggest.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/3] Implement Region of Interest(ROI) support
  2026-08-05  4:09   ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Bryan O'Donoghue
  2026-08-05 16:39     ` Deepa Guthyappa Madivalara
@ 2026-08-06  5:24     ` Vikash Garodia
  1 sibling, 0 replies; 13+ messages in thread
From: Vikash Garodia @ 2026-08-06  5:24 UTC (permalink / raw)
  To: Bryan O'Donoghue, Deepa Guthyappa Madivalara,
	Mauro Carvalho Chehab, Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot


On 8/5/2026 9:39 AM, Bryan O'Donoghue wrote:
> On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
>> Hi all,
>>
>> This patch set implements region of interest(ROI) support for
>> video encoder to be configured as a MB based delta_qp for
>> the whole frame. Custom controls V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP
>> and V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE are implemented to achieve this.
>> Corresponding support is added in iris encoder to implement this
>> feature.
>>
>> This submission and design is based on the feedback received
>> from community for the RFC[1] series posted earlier.
>> Initial testing is done by adding support to v4l2-ctl[2] to test this
>> control setting and its functionality.
>>
>> [1]https://lore.kernel.org/linux-media/20260113-iris_enc_roi- 
>> v1-0-6c86eba38587@oss.qualcomm.com
>> [2]https://lore.kernel.org/linux-media/20260616-enc_roi_enable- 
>> v1-1-252792d27894@oss.qualcomm.com
> as a sidenote can someone in qcom please, please, please do something 
> about Dikshita's email bouncing ?
> 
> "Address not found
> Your message wasn't delivered to dikshita.agarwal@oss.qualcomm.com 
> because the address couldn't be found, or is unable to receive mail.
> LEARN MORE"
> 
> Understood she is on some kind of sabattical - hope she is enjoying it 
> but can you guys either remove her from MAINTAINERS while the email is 
> bouncing or fix your email system to stop spamming responders with the 
> above response ?

Last time when i got this fixed, it seems the senders from 
oss.qualcomm.com do not get delivery failure message, while others are 
still getting. I wasn't aware of this part, since i was not getting from 
oss* doamin and i assumed it is fixed. Let me followup for the non 
oss.qualcomm* part.

Regards,
Vikash>
> Everybody responding is getting these bounces...
> 
> ---
> bod


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder
  2026-08-05  4:06     ` Bryan O'Donoghue
@ 2026-08-06 18:36       ` Deepa Guthyappa Madivalara
  0 siblings, 0 replies; 13+ messages in thread
From: Deepa Guthyappa Madivalara @ 2026-08-06 18:36 UTC (permalink / raw)
  To: Bryan O'Donoghue, Mauro Carvalho Chehab, Vikash Garodia,
	Dikshita Agarwal, Abhinav Kumar
  Cc: linux-media, linux-kernel, linux-arm-msm, kernel test robot


On 8/4/2026 9:06 PM, Bryan O'Donoghue wrote:
> On 04/08/2026 21:05, Deepa Guthyappa Madivalara wrote:
>> Add ROI support in the iris driver, including control structures
>> and default parameters. Extend support to set ROI parameters
>> using custom control V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP.
>> Implement internal buffer list support for BUF_ROIMB_DELTAQP that holds
>> ROI MB based delta_qp as expected by the firmware. When an input
>> arrives queue the corresponding ROI MB delta_qp buffer to firmware.
>>
>> Signed-off-by: Deepa Guthyappa Madivalara 
>> <deepa.madivalara@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/iris/iris_buffer.c     | 101 
>> ++++++++++++++++++++
>>   drivers/media/platform/qcom/iris/iris_buffer.h     |  22 +++++
>>   drivers/media/platform/qcom/iris/iris_ctrls.c      | 104 
>> ++++++++++++++++++++-
>>   drivers/media/platform/qcom/iris/iris_ctrls.h      |   3 +
>>   drivers/media/platform/qcom/iris/iris_hfi_common.h |   1 +
>>   drivers/media/platform/qcom/iris/iris_hfi_gen2.c   |  14 +++
>>   .../platform/qcom/iris/iris_hfi_gen2_command.c     |  62 ++++++++++++
>>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |   3 +
>>   .../platform/qcom/iris/iris_hfi_gen2_packet.c      |   6 +-
>>   .../platform/qcom/iris/iris_hfi_gen2_packet.h      |   3 +
>>   .../platform/qcom/iris/iris_hfi_gen2_response.c    |  29 ++++++
>>   .../platform/qcom/iris/iris_platform_common.h      |   5 +
>>   drivers/media/platform/qcom/iris/iris_venc.c       |   4 +
>>   drivers/media/platform/qcom/iris/iris_venc.h       |   2 +
>>   drivers/media/platform/qcom/iris/iris_vidc.c       |   2 +
>>   15 files changed, 357 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c 
>> b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index 
>> eb8de60c1177f5e1ab83b90a3c8c80e0b4d1f02e..8186a7d011460af4dfcab0f204da08e19b027b33 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -5,11 +5,13 @@
>>
>>   #include <media/v4l2-event.h>
>>   #include <media/v4l2-mem2mem.h>
>> +#include <linux/slab.h>
>>
>>   #include "iris_buffer.h"
>>   #include "iris_instance.h"
>>   #include "iris_power.h"
>>   #include "iris_vpu_buffer.h"
>> +#include "iris_hfi_gen2_defines.h"
>>
>>   #define PIXELS_4K 4096
>>   #define MAX_WIDTH 4096
>> @@ -705,6 +707,23 @@ int 
>> iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 
>> plane)
>>       return iris_destroy_internal_buffers(inst, plane, false);
>>   }
>>
>> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst)
>> +{
>> +    struct iris_buffer *buf, *next;
>> +    struct iris_buffers *buffers;
>> +    int ret = 0;
>> +
>> +    if (inst->domain == ENCODER) {
>> +        buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
>> +        list_for_each_entry_safe(buf, next, &buffers->list, list) {
>> +            ret = iris_destroy_internal_buffer(inst, buf);
>> +            if (ret)
>> +                return ret;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>>   static int iris_release_internal_buffers(struct iris_inst *inst,
>>                        enum iris_buffer_type buffer_type)
>>   {
>> @@ -928,3 +947,85 @@ int iris_vb2_buffer_done(struct iris_inst *inst, 
>> struct iris_buffer *buf)
>>
>>       return 0;
>>   }
>> +
>> +static int iris_fill_roi_data(struct iris_inst *inst, struct 
>> iris_buffer *buffer)
>> +{
>> +    s8 *p_array = (s8 *)inst->fw_caps[ROI_PARAMS].p_array;
>> +    u32 array_size = inst->fw_caps[ROI_PARAMS].elems;
>> +    struct metabuf_header *mbuf_hdr = buffer->kvaddr;
>> +    struct metapayload_header *mbuf_payload_hdr;
>> +    s16 *p_16;
>> +    u32 payload_offset;
>> +
>> +    memset(mbuf_hdr, 0, sizeof(struct metabuf_header));
>> +    mbuf_hdr->count = 1;
>> +    mbuf_hdr->size = sizeof(struct metabuf_header) +
>> +             sizeof(struct metapayload_header);
>> +    mbuf_hdr->version = 1 << 16;
>
> That's a weird way to set a header version number though isn't it.
>
> Either say mbuf_hdr->version = 0x10000 or just BIT(16)
>
ok, will update in v4
>> +    mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr + 1);
> mbuf_hdr++;
> mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr);
>
> would also work
>
ok, will update in v4
>> +    payload_offset = sizeof(struct metabuf_header) +
>> +             sizeof(struct metapayload_header);
>> +
>> +    memset(mbuf_payload_hdr, 0, sizeof(struct metapayload_header));
>
>> +    mbuf_payload_hdr->type = HFI_PROP_ROI_INFO;
>> +    mbuf_payload_hdr->size = array_size * 2;
>
> Instead of * 2 it should be * sizeof(datatype);
>
ok, will update in v4
> BTW the name "metadata" seems strangely generic.
>
This is the same struct used for different metadata. Keeping it generic 
to reuse this in future.
>> +    mbuf_payload_hdr->version = 1 << 16;
>> +    mbuf_payload_hdr->offset = ALIGN(payload_offset, (u32)256);
>> +    mbuf_payload_hdr->flags = 0;
>> +
>> +    /* Firmware expects 2bytes of delta_Qp, int16_t */
>> +    p_16 = buffer->kvaddr + mbuf_payload_hdr->offset;
>> +    for (int i = 0; i < array_size; i++)
>> +        p_16[i] = p_array[i];
>> +
>> +    return 0;
>> +}
>> +
>> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst 
>> *inst)
>> +{
>> +    struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
>> +    struct iris_core *core = inst->core;
>> +    struct iris_buffer *buffer, *first_buffer, *next;
>> +    bool found = false;
>> +    int ret = 0;
>> +
>> +    if (!buffers->size)
>> +        return 0;
>> +
>> +    list_for_each_entry_safe(buffer, next, &buffers->list, list) {
>> +        if (buffer->attr & BUF_ATTR_DEQUEUED) {
>> +            buffer->attr &= ~BUF_ATTR_DEQUEUED;
>> +            list_move(&buffer->list, &buffers->list);
>> +            found = true;
>> +            break;
>> +        }
>> +    }
>> +    if (!found) {
>> +        buffer = kzalloc_obj(*buffer);
>> +        if (!buffer)
>> +            return -ENOMEM;
>> +
>> +        INIT_LIST_HEAD(&buffer->list);
>> +        buffer->type = BUF_ROIMB_DELTAQP;
>> +        buffer->index++;
>> +        buffer->buffer_size = buffers->size;
>> +        buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE;
>> +
>> +        buffer->kvaddr = dma_alloc_attrs(core->dev, 
>> buffer->buffer_size,
>> +                         &buffer->device_addr, GFP_KERNEL,
>> +                         buffer->dma_attrs);
>> +
>> +        if (!buffer->kvaddr) {
>> +            kfree(buffer);
>> +            return -ENOMEM;
>> +        }
>> +        list_add(&buffer->list, &buffers->list);
>> +    }
>> +
>> +    first_buffer = list_first_entry(&buffers->list, struct 
>> iris_buffer, list);
>> +    ret = iris_fill_roi_data(inst, first_buffer);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return ret;
>> +}
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h 
>> b/drivers/media/platform/qcom/iris/iris_buffer.h
>> index 
>> ab8e5d953101a786ade20540ee3c3ed226160cbe..df9d018493f3ec37fd5e6b0ba049dba0b5ae76e9 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.h
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.h
>> @@ -28,6 +28,7 @@ struct iris_inst;
>>    * @BUF_SCRATCH_2: buffer to store encoding context data for HW
>>    * @BUF_VPSS: buffer to store VPSS context data for HW
>>    * @BUF_PARTIAL: buffer for AV1 IBC data
>> + * @BUF_ROIMB_DELTAQP: metadata buffer for ROI MB DeltaQp
>>    * @BUF_TYPE_MAX: max buffer types
>>    */
>>   enum iris_buffer_type {
>> @@ -44,6 +45,7 @@ enum iris_buffer_type {
>>       BUF_SCRATCH_2,
>>       BUF_VPSS,
>>       BUF_PARTIAL,
>> +    BUF_ROIMB_DELTAQP,
>>       BUF_TYPE_MAX,
>>   };
>>
>> @@ -107,6 +109,24 @@ struct iris_buffers {
>>       u32            size;
>>   };
>>
>> +/* Metadata buffer header */
>> +struct metabuf_header {
>> +    u32 count;
>> +    u32 size;
>> +    u32 version;
>> +    u32 reserved[5];
>> +};
>> +
>> +/* Metadata buffer payload header */
>> +struct metapayload_header {
>> +    u32 type;
>> +    u32 size;
>> +    u32 version;
>> +    u32 offset;
>> +    u32 flags;
>> +    u32 reserved[3];
>> +};
>
> But these are roi metadata buffers not generic metadata buffers right ?
>
> So the name should describe what it is an roi_medatadata_thing not a 
> metadata_thing.
>
> OTOH if these are meant to be generic metadata buffers then the 
> addition of these data-structures and the associated logic to support 
> them should live in their own patch - describing metadata buffers.
>
they are generic, will split the patch in v4
>> +
>>   int iris_get_buffer_size(struct iris_inst *inst, enum 
>> iris_buffer_type buffer_type);
>>   void iris_get_internal_buffers(struct iris_inst *inst, u32 plane);
>>   int iris_create_internal_buffers(struct iris_inst *inst, u32 plane);
>> @@ -121,5 +141,7 @@ int iris_queue_buffer(struct iris_inst *inst, 
>> struct iris_buffer *buf);
>>   int iris_queue_deferred_buffers(struct iris_inst *inst, enum 
>> iris_buffer_type buf_type);
>>   int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer 
>> *buf);
>>   void iris_vb2_queue_error(struct iris_inst *inst);
>> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst 
>> *inst);
>> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst);
>>
>>   #endif
>> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c 
>> b/drivers/media/platform/qcom/iris/iris_ctrls.c
>> index 
>> bf17d310eac081ffd3a4ad4842c2255ad798d4d8..4da24a29567f7824f91b4d68a8310b248ab62176 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
>> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
>> @@ -156,6 +156,10 @@ static enum platform_inst_fw_cap_type 
>> iris_get_cap_id(u32 id)
>>           return LAYER5_BITRATE_HEVC;
>>       case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
>>           return REQUEST_SYNC_FRAME;
>> +    case V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP:
>> +        return ROI_PARAMS;
>> +    case V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE:
>> +        return MB_SIZE;
>>       default:
>>           return INST_FW_CAP_MAX;
>>       }
>> @@ -301,6 +305,10 @@ static u32 iris_get_v4l2_id(enum 
>> platform_inst_fw_cap_type cap_id)
>>           return V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR;
>>       case REQUEST_SYNC_FRAME:
>>           return V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME;
>> +    case ROI_PARAMS:
>> +        return V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP;
>> +    case MB_SIZE:
>> +        return V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE;
>>       default:
>>           return 0;
>>       }
>> @@ -327,6 +335,14 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
>>
>>       inst->fw_caps[cap_id].value = ctrl->val;
>>
>> +    if (inst->fw_caps[cap_id].flags & CAP_FLAG_CUSTOM) {
>> +        if (cap_id == ROI_PARAMS) {
>> +            inst->fw_caps[cap_id].p_array =
>> +                (const void *)ctrl->p_new.p;
>> +            inst->fw_caps[cap_id].elems = ctrl->new_elems;
>> +        }
>> +    }
>> +
>>       if (vb2_is_streaming(q)) {
>>           if (cap[cap_id].set)
>>               cap[cap_id].set(inst, cap_id);
>> @@ -335,8 +351,52 @@ static int iris_op_s_ctrl(struct v4l2_ctrl *ctrl)
>>       return 0;
>>   }
>>
>> +static int iris_get_roi_mb_size(struct iris_inst *inst)
>> +{
>> +    return inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;
>> +}
>> +
>> +static int iris_op_g_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> +    struct iris_inst *inst = container_of(ctrl->handler, struct 
>> iris_inst, ctrl_handler);
>> +    enum platform_inst_fw_cap_type cap_id;
>> +
>> +    cap_id = iris_get_cap_id(ctrl->id);
>> +    if (!iris_valid_cap_id(cap_id))
>> +        return -EINVAL;
>> +
>> +    if (cap_id == MB_SIZE)
>> +        ctrl->val = iris_get_roi_mb_size(inst);
>> +
>> +    return 0;
>> +}
>> +
>>   static const struct v4l2_ctrl_ops iris_ctrl_ops = {
>>       .s_ctrl = iris_op_s_ctrl,
>> +    .g_volatile_ctrl = iris_op_g_ctrl,
>> +};
>> +
>> +const struct v4l2_ctrl_config roi_mbqp_cfg = {
>> +    .ops = &iris_ctrl_ops,
>> +    .id = V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP,
>> +    .name = "Enc Mb ROI Delta QP",
>> +    .type = V4L2_CTRL_TYPE_S8,
>> +    .dims = {139264}, /* Max MBPF = 8192 * 4352 / 256 */
>> +    .min = -31,
>> +    .max = 30,
>> +    .def = 0,
>> +    .step = 1,
>> +};
>> +
>> +static const struct v4l2_ctrl_config roi_mbqp_size = {
>> +    .ops = &iris_ctrl_ops,
>> +    .id = V4L2_CID_MPEG_VIDEO_ROI_MB_SIZE,
>> +    .name = "ROI Mb size",
>> +    .type = V4L2_CTRL_TYPE_U8,
>> +    .min = 16,
>> +    .max = 32,
>> +    .def = 16,
>> +    .step = 1,
>>   };
>>
>>   int iris_ctrls_init(struct iris_inst *inst)
>> @@ -361,7 +421,7 @@ int iris_ctrls_init(struct iris_inst *inst)
>>           return ret;
>>
>>       for (idx = 1; idx < INST_FW_CAP_MAX; idx++) {
>> -        struct v4l2_ctrl *ctrl;
>> +        struct v4l2_ctrl *ctrl = NULL;
>>
>>           v4l2_id = iris_get_v4l2_id(cap[idx].cap_id);
>>           if (!v4l2_id)
>> @@ -379,6 +439,13 @@ int iris_ctrls_init(struct iris_inst *inst)
>>                                 cap[idx].max,
>>                                 ~(cap[idx].step_or_mask),
>>                                 cap[idx].value);
>> +        } else if (cap[idx].flags & CAP_FLAG_CUSTOM) {
>> +            if (cap[idx].cap_id == ROI_PARAMS)
>> +                ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
>> +                                &roi_mbqp_cfg, NULL);
>> +            if (cap[idx].cap_id == MB_SIZE)
>> +                ctrl = v4l2_ctrl_new_custom(&inst->ctrl_handler,
>> +                                &roi_mbqp_size, NULL);
>>           } else {
>>               ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler,
>>                            &iris_ctrl_ops,
>> @@ -1540,3 +1607,38 @@ int iris_set_properties(struct iris_inst 
>> *inst, u32 plane)
>>
>>       return 0;
>>   }
>> +
>> +int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane)
>> +{
>> +    const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
>> +    int ret = 0;
>> +
>> +    /*subscribe to metadata delivery only if ROI is enabled */
>> +    if (!inst->fw_caps[ROI_PARAMS].p_array)
>> +        return ret;
>> +
>> +    ret = hfi_ops->session_subscribe_metadata_delivery(inst, plane);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return ret;
>> +}
>> +
>> +int iris_set_roi_params(struct iris_inst *inst, u32 plane)
>> +{
>> +    struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
>> +    u32 metadata_header_bytes = 256;
>> +    u32 size = 0;
>> +    int ret = 0;
>> +
>> +    if (!inst->fw_caps[ROI_PARAMS].p_array)
>> +        return -EINVAL;
>> +
>> +    size = inst->fw_caps[ROI_PARAMS].elems * 2 + metadata_header_bytes;
>> +    buffers->size = ALIGN(size, 4096);
>> +    iris_hfi_gen2_session_alloc_roi_metadata_buffer(inst);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return 0;
>> +}
>> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.h 
>> b/drivers/media/platform/qcom/iris/iris_ctrls.h
>> index 
>> 5180d53d3c904cad460b2760913b475d0ff1bb55..df8fa957a6127338ae63bdd44871e363deeb48fc 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_ctrls.h
>> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.h
>> @@ -50,5 +50,8 @@ int iris_set_layer_bitrate(struct iris_inst *inst, 
>> enum platform_inst_fw_cap_typ
>>   int iris_set_req_sync_frame(struct iris_inst *inst, enum 
>> platform_inst_fw_cap_type cap_id);
>>   int iris_set_time_delta_based_rc(struct iris_inst *inst, enum 
>> platform_inst_fw_cap_type cap_id);
>>   int iris_set_properties(struct iris_inst *inst, u32 plane);
>> +int iris_set_roi_params(struct iris_inst *inst, u32 plane);
>> +int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane);
>> +int iris_set_roi_mb_size(struct iris_inst *inst);
>>
>>   #endif
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.h 
>> b/drivers/media/platform/qcom/iris/iris_hfi_common.h
>> index 
>> a27447eb2519962cb958b0e330a6d018310c3450..5692375cdd357f9b00760053ffff5af4bd9cce9c 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.h
>> @@ -131,6 +131,7 @@ struct iris_hfi_session_ops {
>>       int (*session_drain)(struct iris_inst *inst, u32 plane);
>>       int (*session_resume_drain)(struct iris_inst *inst, u32 plane);
>>       int (*session_close)(struct iris_inst *inst);
>> +    int (*session_subscribe_metadata_delivery)(struct iris_inst 
>> *inst, u32 plane);
>>   };
>>
>>   struct hfi_subscription_params {
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
>> index 
>> 3b8fbefb8b93576962abd3850215f7b7fb364930..4d40d6733e5877a302ce63fb0b4a61b8f284e6dc 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
>> @@ -1782,6 +1782,20 @@ static const struct platform_inst_fw_cap 
>> inst_fw_cap_sm8550_enc[] = {
>>           .flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_DYNAMIC_ALLOWED,
>>           .set = iris_set_req_sync_frame,
>>       },
>> +    {
>> +        .cap_id = ROI_PARAMS,
>> +        .step_or_mask = 1,
>> +        .p_array = NULL,
>> +        .hfi_id = HFI_PROP_ROI_INFO,
>> +        .flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_CUSTOM |
>> +             CAP_FLAG_DYNAMIC_ALLOWED,
>> +        .set = iris_set_roi_params,
>> +    },
>> +    {
>> +        .cap_id = MB_SIZE,
>> +        .step_or_mask = 1,
>> +        .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_CUSTOM,
>> +    },
>
> The enumeration of the platform and the implementation of the logic 
> should be separated into different patches.
>
ok, will update in v4
>>   };
>>
>>   static const u32 sm8550_vdec_input_config_params_default[] = {
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> index 
>> 388a36ff2b07b7bcd8db21d4345bc900356b4ec3..4581a6ea74a708b0d493bc71d2d1822eda078860 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>> @@ -134,6 +134,7 @@ static u32 
>> iris_hfi_gen2_get_port_from_buf_type(struct iris_inst *inst,
>>           switch (buffer_type) {
>>           case BUF_INPUT:
>>           case BUF_VPSS:
>> +        case BUF_ROIMB_DELTAQP:
>>               return HFI_PORT_RAW;
>>           case BUF_OUTPUT:
>>           case BUF_BIN:
>> @@ -1267,6 +1268,8 @@ static u32 
>> iris_hfi_gen2_buf_type_from_driver(u32 domain, enum iris_buffer_type
>>           return HFI_BUFFER_VPSS;
>>       case BUF_PARTIAL:
>>           return HFI_BUFFER_PARTIAL_DATA;
>> +    case BUF_ROIMB_DELTAQP:
>> +        return HFI_BUFFER_METADATA;
>>       default:
>>           return 0;
>>       }
>> @@ -1307,10 +1310,29 @@ static void iris_hfi_gen2_get_buffer(u32 
>> domain, struct iris_buffer *buffer,
>>       buf->timestamp = buffer->timestamp;
>>   }
>>
>> +static struct iris_buffer *iris_queue_metadata_buffers(struct 
>> iris_inst *inst,
>> +                               enum iris_buffer_type buffer_type, 
>> u32 index)
>> +{
>> +    struct iris_buffers *buffers = &inst->buffers[buffer_type];
>> +    struct iris_buffer *buffer = NULL;
>> +
>> +    if (list_empty(&buffers->list))
>> +        return NULL;
>> +
>> +    buffer = list_first_entry(&buffers->list, typeof(*buffer), list);
>> +    if ((buffer->attr & BUF_ATTR_QUEUED) || (buffer->attr & 
>> BUF_ATTR_DEQUEUED))
>> +        return NULL;
>> +
>> +    buffer->index = index;
>> +
>> +    return buffer;
>> +}
>> +
>>   static int iris_hfi_gen2_session_queue_buffer(struct iris_inst 
>> *inst, struct iris_buffer *buffer)
>>   {
>>       struct iris_inst_hfi_gen2 *inst_hfi_gen2 = 
>> to_iris_inst_hfi_gen2(inst);
>>       struct iris_hfi_buffer hfi_buffer;
>> +    struct iris_hfi_buffer hfi_meta_buffer;
>>       u32 port;
>>       int ret;
>>
>> @@ -1331,6 +1353,25 @@ static int 
>> iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iri
>>                            &hfi_buffer,
>>                            sizeof(hfi_buffer));
>>
>> +    /* check if any metadata buffer is available not queued, queueit */
>> +    if (port == HFI_PORT_RAW) {
>> +        buffer = iris_queue_metadata_buffers(inst, 
>> BUF_ROIMB_DELTAQP, buffer->index);
>> +        if (buffer) {
>> +            iris_hfi_gen2_get_buffer(inst->domain, buffer, 
>> &hfi_meta_buffer);
>> +            port = iris_hfi_gen2_get_port_from_buf_type(inst, 
>> buffer->type);
>> + iris_hfi_gen2_create_packet(inst_hfi_gen2->packet,
>> +                            HFI_CMD_BUFFER,
>> +                            HFI_HOST_FLAGS_INTR_REQUIRED,
>> +                            HFI_PAYLOAD_STRUCTURE,
>> +                            port,
>> +                            inst->core->packet_id++,
>> +                            &hfi_meta_buffer,
>> +                            sizeof(hfi_meta_buffer));
>> +
>> +            buffer->attr |= BUF_ATTR_QUEUED;
>> +            buffer->attr &= ~BUF_ATTR_DEQUEUED;
>> +        }
>> +    }
>>       return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
>>                       inst_hfi_gen2->packet->size);
>>   }
>> @@ -1359,6 +1400,26 @@ static int 
>> iris_hfi_gen2_session_release_buffer(struct iris_inst *inst, struct i
>>                       inst_hfi_gen2->packet->size);
>>   }
>>
>> +static int iris_hfi_gen2_subscribe_metadata_delivery(struct 
>> iris_inst *inst, u32 plane)
>> +{
>> +    struct iris_inst_hfi_gen2 *inst_hfi_gen2 = 
>> to_iris_inst_hfi_gen2(inst);
>> +    u32 port = iris_hfi_gen2_get_port(inst, 
>> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>> +    u32 payload[2] = {HFI_MODE_METADATA, HFI_PROP_ROI_INFO};
>> +
>> +    iris_hfi_gen2_packet_session_command(inst,
>> +                         HFI_CMD_DELIVERY_MODE,
>> +                         (HFI_HOST_FLAGS_RESPONSE_REQUIRED |
>> +                          HFI_HOST_FLAGS_INTR_REQUIRED),
>> +                         port,
>> +                         inst->session_id,
>> +                         HFI_PAYLOAD_U32_ARRAY,
>> +                         &payload,
>> +                         sizeof(u32) * 2);
>> +
>> +    return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
>> +                    inst_hfi_gen2->packet->size);
>> +}
>> +
>>   static const struct iris_hfi_session_ops iris_hfi_gen2_session_ops = {
>>       .session_open = iris_hfi_gen2_session_open,
>>       .session_set_config_params = 
>> iris_hfi_gen2_session_set_config_params,
>> @@ -1372,6 +1433,7 @@ static const struct iris_hfi_session_ops 
>> iris_hfi_gen2_session_ops = {
>>       .session_drain = iris_hfi_gen2_session_drain,
>>       .session_resume_drain = iris_hfi_gen2_session_resume_drain,
>>       .session_close = iris_hfi_gen2_session_close,
>> +    .session_subscribe_metadata_delivery = 
>> iris_hfi_gen2_subscribe_metadata_delivery,
>>   };
>>
>>   static struct iris_inst *iris_hfi_gen2_get_instance(void)
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> index 
>> f43aea10090d8f1d6ae5c20547e5f6321b2ca203..d0661f5fe6a6f39295dad8950edb2f1a85893216 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> @@ -20,6 +20,7 @@
>>   #define HFI_CMD_DRAIN                0x01000007
>>   #define HFI_CMD_RESUME                0x01000008
>>   #define HFI_CMD_BUFFER                0x01000009
>> +#define HFI_CMD_DELIVERY_MODE                0x0100000A
>>   #define HFI_CMD_SUBSCRIBE_MODE            0x0100000B
>>   #define HFI_CMD_SETTINGS_CHANGE            0x0100000C
>>   #define HFI_CMD_PAUSE                0x01000011
>> @@ -133,6 +134,7 @@ enum hfi_flip {
>>   #define HFI_PROP_DEC_START_FROM_RAP_FRAME    0x03000169
>>   #define HFI_PROP_NO_OUTPUT            0x0300016a
>>   #define HFI_PROP_BUFFER_MARK            0x0300016c
>> +#define HFI_PROP_ROI_INFO            0x03000173
>>   #define HFI_PROP_WORST_COMPRESSION_RATIO    0x03000174
>>   #define HFI_PROP_WORST_COMPLEXITY_FACTOR    0x03000175
>>   #define HFI_PROP_RAW_RESOLUTION        0x03000178
>> @@ -174,6 +176,7 @@ enum hfi_flip {
>>   enum hfi_property_mode_type {
>>       HFI_MODE_PORT_SETTINGS_CHANGE        = 0x00000001,
>>       HFI_MODE_PROPERTY            = 0x00000002,
>> +    HFI_MODE_METADATA            = 0x00000004,
>>   };
>>
>>   enum hfi_color_format {
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
>> index 
>> 0d05dd2afc07d830cc8502ab5f28001312991ba8..aeb0426a05694f219f82145cd84a28287ed3075e 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.c
>> @@ -100,9 +100,9 @@ static void iris_hfi_gen2_create_header(struct 
>> iris_hfi_header *hdr,
>>       hdr->num_packets = 0;
>>   }
>>
>> -static void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, 
>> u32 pkt_type,
>> -                    u32 pkt_flags, u32 payload_type, u32 port,
>> -                    u32 packet_id, void *payload, u32 payload_size)
>> +void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 
>> pkt_type,
>> +                 u32 pkt_flags, u32 payload_type, u32 port,
>> +                 u32 packet_id, void *payload, u32 payload_size)
>>   {
>>       struct iris_hfi_packet *pkt = (struct iris_hfi_packet *)((u8 
>> *)hdr + hdr->size);
>>       u32 pkt_size = sizeof(*pkt) + payload_size;
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
>> index 
>> 25b9582349ca1a0ce6efc0b146a3abb798485c45..613eb500609f745daebdcbdf9a25b85cb9465a79 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_packet.h
>> @@ -121,5 +121,8 @@ void iris_hfi_gen2_packet_session_property(struct 
>> iris_inst *inst,
>>   void iris_hfi_gen2_packet_sys_interframe_powercollapse(struct 
>> iris_core *core,
>>                                  struct iris_hfi_header *hdr);
>>   void iris_hfi_gen2_packet_sys_pc_prep(struct iris_core *core, 
>> struct iris_hfi_header *hdr);
>> +void iris_hfi_gen2_create_packet(struct iris_hfi_header *hdr, u32 
>> pkt_type,
>> +                 u32 pkt_flags, u32 payload_type, u32 port,
>> +                 u32 packet_id, void *payload, u32 payload_size);
>>
>>   #endif
>> diff --git 
>> a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c 
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> index 
>> 8c2644c7f6e85983d7ad7584fc0cb570e4813ae4..f63e0a7723e8583da818c2b004e367c1dd3d94f7 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> @@ -58,6 +58,8 @@ static u32 iris_hfi_gen2_buf_type_to_driver(struct 
>> iris_inst *inst,
>>           return BUF_PARTIAL;
>>       case HFI_BUFFER_VPSS:
>>           return BUF_VPSS;
>> +    case HFI_BUFFER_METADATA:
>> +        return BUF_ROIMB_DELTAQP;
>>       default:
>>           return 0;
>>       }
>> @@ -77,6 +79,7 @@ static bool 
>> iris_hfi_gen2_is_valid_hfi_buffer_type(u32 buffer_type)
>>       case HFI_BUFFER_PERSIST:
>>       case HFI_BUFFER_VPSS:
>>       case HFI_BUFFER_PARTIAL_DATA:
>> +    case HFI_BUFFER_METADATA:
>>           return true;
>>       default:
>>           return false;
>> @@ -452,6 +455,30 @@ static int 
>> iris_hfi_gen2_handle_release_internal_buffer(struct iris_inst *inst,
>>       return 0;
>>   }
>>
>> +static int iris_hfi_gen2_handle_output_metadata_buffer(struct 
>> iris_inst *inst,
>> +                               struct iris_hfi_buffer *buffer)
>> +{
>> +    u32 buf_type = iris_hfi_gen2_buf_type_to_driver(inst, 
>> HFI_BUFFER_METADATA);
>> +    struct iris_buffers *buffers = &inst->buffers[buf_type];
>> +    struct iris_buffer *buf, *iter;
>> +    bool found = false;
>> +
>> +    list_for_each_entry(iter, &buffers->list, list) {
>> +        if (iter->device_addr == buffer->base_address) {
>> +            found = true;
>> +            buf = iter;
>> +            break;
>> +        }
>> +    }
>> +    if (!found)
>> +        return -EINVAL;
>> +
>> +    buf->attr &= ~BUF_ATTR_QUEUED;
>> +    buf->attr |= BUF_ATTR_DEQUEUED;
>> +
>> +    return 0;
>> +}
>> +
>>   static int iris_hfi_gen2_handle_session_stop(struct iris_inst *inst,
>>                            struct iris_hfi_packet *pkt)
>>   {
>> @@ -499,6 +526,8 @@ static int 
>> iris_hfi_gen2_handle_session_buffer(struct iris_inst *inst,
>>               return iris_hfi_gen2_handle_input_buffer(inst, buffer);
>>           else if (buffer->type == HFI_BUFFER_BITSTREAM)
>>               return iris_hfi_gen2_handle_output_buffer(inst, buffer);
>> +        else if (buffer->type == HFI_BUFFER_METADATA)
>> +            return iris_hfi_gen2_handle_output_metadata_buffer(inst, 
>> buffer);
>>           else
>>               return 
>> iris_hfi_gen2_handle_release_internal_buffer(inst, buffer);
>>       }
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h 
>> b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index 
>> 9748095091461ba13443c63955a42906fa4f050c..9910eb5514b3ae0a6b5349adb071d0a56375102c 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> @@ -186,6 +186,8 @@ enum platform_inst_fw_cap_type {
>>       LAYER5_BITRATE_HEVC,
>>       REQUEST_SYNC_FRAME,
>>       TIME_DELTA_BASED_RC,
>> +    ROI_PARAMS,
>> +    MB_SIZE,
>>       INST_FW_CAP_MAX,
>>   };
>>
>> @@ -197,6 +199,7 @@ enum platform_inst_fw_cap_flags {
>>       CAP_FLAG_CLIENT_SET        = BIT(4),
>>       CAP_FLAG_BITMASK        = BIT(5),
>>       CAP_FLAG_VOLATILE        = BIT(6),
>> +    CAP_FLAG_CUSTOM            = BIT(7),
>>   };
>>
>>   struct platform_inst_fw_cap {
>> @@ -206,6 +209,8 @@ struct platform_inst_fw_cap {
>>       s64 step_or_mask;
>>       s64 value;
>>       u32 hfi_id;
>> +    const void *p_array;
>> +    u32 elems;
>>       enum platform_inst_fw_cap_flags flags;
>>       int (*set)(struct iris_inst *inst,
>>              enum platform_inst_fw_cap_type cap_id);
>> diff --git a/drivers/media/platform/qcom/iris/iris_venc.c 
>> b/drivers/media/platform/qcom/iris/iris_venc.c
>> index 
>> a945992f63aa8a0c40b8d6bc473d0bc90270e6cd..21ba9639d17b2c64e8fa33b4aacbd1946fc4dae2 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_venc.c
>> +++ b/drivers/media/platform/qcom/iris/iris_venc.c
>> @@ -498,6 +498,10 @@ int iris_venc_streamon_output(struct iris_inst 
>> *inst)
>>       if (ret)
>>           goto error;
>>
>> +    ret = iris_set_metadata_delivery(inst, 
>> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>> +    if (ret)
>> +        goto error;
>> +
>>       ret = iris_alloc_and_queue_persist_bufs(inst, BUF_ARP);
>>       if (ret)
>>           return ret;
>> diff --git a/drivers/media/platform/qcom/iris/iris_venc.h 
>> b/drivers/media/platform/qcom/iris/iris_venc.h
>> index 
>> 00c1716b2747c7e840c2a3317800d83663744bf0..df97011636740be6cdfe5867f273c5f93408f570 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_venc.h
>> +++ b/drivers/media/platform/qcom/iris/iris_venc.h
>> @@ -22,5 +22,7 @@ int iris_venc_streamon_output(struct iris_inst *inst);
>>   int iris_venc_qbuf(struct iris_inst *inst, struct vb2_v4l2_buffer 
>> *vbuf);
>>   int iris_venc_start_cmd(struct iris_inst *inst);
>>   int iris_venc_stop_cmd(struct iris_inst *inst);
>> +struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
>> +                        enum iris_buffer_type buffer_type, u32 index);
>>
>>   #endif
>> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c 
>> b/drivers/media/platform/qcom/iris/iris_vidc.c
>> index 
>> fcbc60016beec693f2ce27927a09a2d51494bc38..fae3d113a9dc990f06167ee0b7fb8fab24a0552c 
>> 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
>> @@ -180,6 +180,7 @@ int iris_open(struct file *filp)
>>       INIT_LIST_HEAD(&inst->buffers[BUF_SCRATCH_2].list);
>>       INIT_LIST_HEAD(&inst->buffers[BUF_VPSS].list);
>>       INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
>> + INIT_LIST_HEAD(&inst->buffers[BUF_ROIMB_DELTAQP].list);
>>       init_completion(&inst->completion);
>>       init_completion(&inst->flush_completion);
>>
>> @@ -299,6 +300,7 @@ int iris_close(struct file *filp)
>>       iris_destroy_all_internal_buffers(inst, 
>> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>>       iris_check_num_queued_internal_buffers(inst, 
>> V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>>       iris_check_num_queued_internal_buffers(inst, 
>> V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>> +    iris_destroy_roi_metadata_buffers(inst);
>>       iris_remove_session(inst);
>>       mutex_unlock(&inst->lock);
>>       mutex_destroy(&inst->ctx_q_lock);
>>
>> -- 
>> 2.34.1
>>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-08-06 18:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <_l91CrDl1TZCUzjX1JcRr1cgRM6CNFGa4eRe52VgXTUOOcx8yjj9YCeBaulfAgFkrnN6fCpZfpCIilkjdOV_JA==@protonmail.internalid>
2026-08-04 20:05 ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
2026-08-04 20:05   ` [PATCH v3 1/3] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
2026-08-05  3:49     ` Bryan O'Donoghue
2026-08-05 16:37       ` Deepa Guthyappa Madivalara
2026-08-04 20:05   ` [PATCH v3 2/3] media: v4l2-core: Add support for video encoder ROI control Deepa Guthyappa Madivalara
2026-08-05  3:51     ` Bryan O'Donoghue
2026-08-05 16:37       ` Deepa Guthyappa Madivalara
2026-08-04 20:05   ` [PATCH v3 3/3] media: iris: Add ROI support framework for iris video encoder Deepa Guthyappa Madivalara
2026-08-05  4:06     ` Bryan O'Donoghue
2026-08-06 18:36       ` Deepa Guthyappa Madivalara
2026-08-05  4:09   ` [PATCH v3 0/3] Implement Region of Interest(ROI) support Bryan O'Donoghue
2026-08-05 16:39     ` Deepa Guthyappa Madivalara
2026-08-06  5:24     ` Vikash Garodia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox