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