On 1/18/2024 11:14 PM, Konrad Dybcio wrote: > > > On 1/18/24 11:59, Sachin Kumar Garg wrote: >> There is no limit on the maximum level of the bit rate with >> the existing VBR rate control. >> V4L2_MPEG_VIDEO_BITRATE_MODE_MBR rate control will limit the >> frame maximum bit rate range to the +/- 10% of the configured >> bit-rate value. Encoder will choose appropriate quantization >> parameter and do the smart bit allocation to set the frame >> maximum bitrate level. >> >> Signed-off-by: Sachin Kumar Garg <quic_sachinku@xxxxxxxxxxx> >> --- >> drivers/media/platform/qcom/venus/hfi_cmds.c | 38 +++++++++++++------ >> .../media/platform/qcom/venus/hfi_helper.h | 1 + >> drivers/media/platform/qcom/venus/venc.c | 2 + >> .../media/platform/qcom/venus/venc_ctrls.c | 5 ++- >> 4 files changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c >> b/drivers/media/platform/qcom/venus/hfi_cmds.c >> index 3418d2dd9371..95fc27e0dc7d 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c >> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c >> @@ -645,17 +645,33 @@ static int pkt_session_set_property_1x(struct >> hfi_session_set_property_pkt *pkt, >> case HFI_PROPERTY_PARAM_VENC_RATE_CONTROL: { >> u32 *in = pdata; >> - switch (*in) { >> - case HFI_RATE_CONTROL_OFF: >> - case HFI_RATE_CONTROL_CBR_CFR: >> - case HFI_RATE_CONTROL_CBR_VFR: >> - case HFI_RATE_CONTROL_VBR_CFR: >> - case HFI_RATE_CONTROL_VBR_VFR: >> - case HFI_RATE_CONTROL_CQ: >> - break; >> - default: >> - ret = -EINVAL; >> - break; >> + if (hfi_ver == HFI_VERSION_4XX) { > > So, only sdm845/sc7180 and friends support it, but the newer > SoCs (like 8250 don't)? Thats correct. Supported only in AR50 generations. Not available in 8250. > > [...] > >> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c >> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c >> @@ -387,10 +387,11 @@ int venc_ctrl_init(struct venus_inst *inst) >> v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_MPEG_VIDEO_BITRATE_MODE, >> - V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, >> + V4L2_MPEG_VIDEO_BITRATE_MODE_MBR, > > Is this okay, since you're claiming only v4 supports it? This looks okay to extend the support for new RC mode. I see an issue in handling this new RC for non supported SOCs. This needs to be fixed in hfi_cmds.c while preparing the packet. MBR for unsupported SOC should be treated as -ENOTSUPP instead of -EINVAL which would terminate the session. This need to be fixed. Regards, Vikash