Re: [PATCH v0 1/3] media: chips-media: wave5: Add Support for Background Detection
From: Nicolas Dufresne
Date: Thu Mar 19 2026 - 17:18:13 EST
Le jeudi 19 mars 2026 à 14:32 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
>
> Wave5 encoder can be configured to detect a background region in a frame.
> If a background region is detected, it will use less bits or skip mode to
> encode that region. This can assist in lowering the video bitrate for a
> given stream.
Since you are adding a generic control (and I think the feature is pretty clear
an generic), you should make this clear in the text. So start saying you are
adding a generic control for the background detection feature and then quote
that this will be implemented by wave5. Its just a nit on wording, I have no
doubt this option would also exist on other IP and be compatible through the
provided description.
>
> Signed-off-by: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
> ---
> Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 5 +++++
> drivers/media/platform/chips-media/wave5/wave5-hw.c | 4 +++-
> drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 7 +++++++
> drivers/media/platform/chips-media/wave5/wave5-vpuapi.h | 1 +
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 2 ++
> include/uapi/linux/v4l2-controls.h | 2 ++
> 6 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> index c8890cb5e00a..b992a0d5c7bf 100644
> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
> @@ -605,6 +605,11 @@ enum v4l2_mpeg_video_frame_skip_mode -
> chosen data limit then the frame will be skipped. Possible values
> are:
>
> +``V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION (boolean)``
> + If enabled then, the encoder detect a background region in frame and
drop then
> + use low bits or skip mode to encode the background region.
> + If a lot of scenes are stationary or background, It may help to
> + reduce the video bitrate. Applicable to the encoder.
>
> .. tabularcolumns:: |p{8.2cm}|p{9.3cm}|
>
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> index 687ce6ccf3ae..c516d125f553 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
Please, split the API addition from the driver changes.
> @@ -49,6 +49,7 @@
>
> #define FASTIO_ADDRESS_MASK GENMASK(15, 0)
> #define SEQ_PARAM_PROFILE_MASK GENMASK(30, 24)
> +#define SEQ_BG_PARAM_REG_DATA 0x3800410
>
> static void _wave5_print_reg_err(struct vpu_device *vpu_dev, u32
> reg_fail_reason,
> const char *func);
> @@ -1838,7 +1839,8 @@ int wave5_vpu_enc_init_seq(struct vpu_instance *inst)
> vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_RC_BIT_RATIO_LAYER_4_7, 0);
> vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_ROT_PARAM, rot_mir_mode);
>
> - vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_BG_PARAM, 0);
> + vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_BG_PARAM,
> + SEQ_BG_PARAM_REG_DATA | p_param->bg_detection);
> vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_CUSTOM_LAMBDA_ADDR, 0);
> vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_CONF_WIN_TOP_BOT,
> p_param->conf_win_bot << 16 | p_param->conf_win_top);
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> index 7613fcdbafed..6fe01217233f 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -780,6 +780,9 @@ static int wave5_vpu_enc_s_ctrl(struct v4l2_ctrl *ctrl)
> case V4L2_CID_MPEG_VIDEO_BITRATE:
> inst->bit_rate = ctrl->val;
> break;
> + case V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION:
> + inst->enc_param.bg_detection = ctrl->val;
> + break;
> case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
> inst->enc_param.avc_idr_period = ctrl->val;
> break;
> @@ -1205,6 +1208,7 @@ static int wave5_set_enc_openparam(struct enc_open_param
> *open_param,
> open_param->wave_param.beta_offset_div2 = input.beta_offset_div2;
> open_param->wave_param.decoding_refresh_type =
> input.decoding_refresh_type;
> open_param->wave_param.intra_period = input.intra_period;
> + open_param->wave_param.bg_detection = input.bg_detection;
> if (inst->std == W_HEVC_ENC) {
> if (input.intra_period == 0) {
> open_param->wave_param.decoding_refresh_type =
> DEC_REFRESH_TYPE_IDR;
> @@ -1700,6 +1704,9 @@ static int wave5_vpu_open_enc(struct file *filp)
> v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> V4L2_CID_MPEG_VIDEO_AU_DELIMITER,
> 0, 1, 1, 1);
> + v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> + V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION,
> + 0, 1, 1, 0);
The hard question, last parameter here is def, so I read this is disabled by
default. What's the side effect of having this enabled by default ? Did you
already considered that option ?
regards,
Nicolas
> v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> V4L2_CID_HFLIP,
> 0, 1, 1, 0);
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> index c64135769869..dc31689e0d27 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> @@ -570,6 +570,7 @@ struct enc_wave_param {
> u32 transform8x8_enable: 1; /* enable 8x8 intra prediction and 8x8
> transform */
> u32 mb_level_rc_enable: 1; /* enable MB-level rate control */
> u32 forced_idr_header_enable: 1; /* enable header encoding before IDR
> frame */
> + u32 bg_detection: 1; /* enable background detection */
> };
>
> struct enc_open_param {
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-
> core/v4l2-ctrls-defs.c
> index 551426c4cd01..e062f2088490 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -889,6 +889,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> case V4L2_CID_MPEG_VIDEO_DEC_DISPLAY_DELAY: return
> "Display Delay";
> case V4L2_CID_MPEG_VIDEO_DEC_DISPLAY_DELAY_ENABLE: return
> "Display Delay Enable";
> case V4L2_CID_MPEG_VIDEO_AU_DELIMITER: return
> "Generate Access Unit Delimiters";
> + case V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION: return
> "Background Detection";
> case V4L2_CID_MPEG_VIDEO_H263_I_FRAME_QP: return "H263
> I-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_P_FRAME_QP: return "H263
> P-Frame QP Value";
> case V4L2_CID_MPEG_VIDEO_H263_B_FRAME_QP: return "H263
> B-Frame QP Value";
> @@ -1296,6 +1297,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type,
> case V4L2_CID_MPEG_VIDEO_MPEG4_QPEL:
> case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:
> case V4L2_CID_MPEG_VIDEO_AU_DELIMITER:
> + case V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION:
> case V4L2_CID_WIDE_DYNAMIC_RANGE:
> case V4L2_CID_IMAGE_STABILIZATION:
> case V4L2_CID_RDS_RECEPTION:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-
> controls.h
> index 68dd0c4e47b2..939f796261c0 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -464,6 +464,8 @@ enum v4l2_mpeg_video_intra_refresh_period_type {
> V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD_TYPE_CYCLIC = 1,
> };
>
> +#define
> V4L2_CID_MPEG_VIDEO_BACKGROUND_DETECTION (V4L2_CID_CODEC_BASE+238)
> +
> /* CIDs for the MPEG-2 Part 2 (H.262) codec */
> #define
> V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_CODEC_BASE+270)
> enum v4l2_mpeg_video_mpeg2_level {
Attachment:
signature.asc
Description: This is a digitally signed message part