RE: [PATCH v0 2/3] media: chips-media: wave5: Support CBP profile
From: jackson . lee
Date: Thu Mar 19 2026 - 21:23:23 EST
Hi Nicolas
> -----Original Message-----
> From: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> Sent: Thursday, March 19, 2026 9:33 PM
> To: jackson.lee <jackson.lee@xxxxxxxxxxxxxxx>; mchehab@xxxxxxxxxx;
> hverkuil-cisco@xxxxxxxxx; bob.beckett@xxxxxxxxxxxxx
> Cc: linux-media@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; lafley.kim
> <lafley.kim@xxxxxxxxxxxxxxx>; b-brnich@xxxxxx; hverkuil@xxxxxxxxx; Nas
> Chung <nas.chung@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v0 2/3] media: chips-media: wave5: Support CBP profile
>
> Hi Jackson,
>
> Le jeudi 19 mars 2026 à 14:32 +0900, Jackson.lee a écrit :
> > From: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> >
> > Constrained Baseline Profile (CBP) and Baseline Profile (BP) have been
> > treated as the same.
> > Introduce the ability to differentiate between the two.
> >
> > Signed-off-by: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> > Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
>
> Should we consider this one has a bug fix ? I suspect previously
> constraint_set1_flag was never set in the bitstream, and now it is set
> accordingly to the selected profile, which fixes a miss-match of user
> expectation vs bitstream values. If you agree with this, a Fixes: tag
> would be nice.
>
I will add the "Fixes" tag.
Thanks for your review.
Jackson
> > ---
> > drivers/media/platform/chips-media/wave5/wave5-hw.c | 3 +++
> > .../media/platform/chips-media/wave5/wave5-vpu-enc.c | 10
> > +++++++---
> > .../media/platform/chips-media/wave5/wave5-vpuapi.h | 1 +
> > 3 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > index c516d125f553..2392bce8d840 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > @@ -1763,6 +1763,9 @@ int wave5_vpu_enc_init_seq(struct vpu_instance
> *inst)
> > (p_param->skip_intra_trans << 25) |
> > (p_param->strong_intra_smooth_enable << 27) |
> > (p_param->en_still_picture << 30);
> > + else if (inst->std == W_AVC_ENC)
> > + reg_val |= (p_param->constraint_set1_flag << 29);
> > +
> > vpu_write_reg(inst->dev, W5_CMD_ENC_SEQ_SPS_PARAM, reg_val);
> >
> > reg_val = (p_param->lossless_enable) | 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 6fe01217233f..f315ed7243a7 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > @@ -939,6 +939,8 @@ static int wave5_vpu_enc_s_ctrl(struct v4l2_ctrl
> *ctrl)
> > case V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE:
> > inst->enc_param.profile = H264_PROFILE_BP;
> > inst->bit_depth = 8;
> > + if (ctrl->val ==
> V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE)
> > + inst->enc_param.constraint_set1_flag = 1;
> > break;
> > case V4L2_MPEG_VIDEO_H264_PROFILE_MAIN:
> > inst->enc_param.profile = H264_PROFILE_MP; @@ -1214,9
> +1216,11 @@
> > static int wave5_set_enc_openparam(struct enc_open_param *open_param,
> > open_param->wave_param.decoding_refresh_type =
> DEC_REFRESH_TYPE_IDR;
> > open_param->wave_param.intra_period =
> input.avc_idr_period;
> > }
> > - } else {
> > + } else if (inst->std == W_AVC_ENC)
> > + open_param->wave_param.constraint_set1_flag =
> input.constraint_set1_flag;
> > + else
> > open_param->wave_param.avc_idr_period = input.avc_idr_period;
> > - }
>
> nit: Just keep the bracket, so that all branches have brackets.
>
> cheers,
> Nicolas
>
> > +
> > open_param->wave_param.entropy_coding_mode =
> input.entropy_coding_mode;
> > open_param->wave_param.lossless_enable = input.lossless_enable;
> > open_param->wave_param.const_intra_pred_flag =
> > input.const_intra_pred_flag; @@ -1687,7 +1691,7 @@ static int
> wave5_vpu_open_enc(struct file *filp)
> > -6, 6, 1, 0);
> > v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> > V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM,
> > - 0, 1, 1, 1);
> > + 0, 1, 1, 0);
> > v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> >
> V4L2_CID_MPEG_VIDEO_H264_CONSTRAINED_INTRA_PREDICTION,
> > 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 dc31689e0d27..7b08fef58217 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 constraint_set1_flag: 1; /* enable CBP */
> > u32 bg_detection: 1; /* enable background detection */
> > };
> >