Received-SPF: pass (google.com: domain of linux-kernel+bounces-30435-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <04a364e8-534c-40a4-a031-b9f9d2304c39@linaro.org>
Date: Thu, 18 Jan 2024 18:44:56 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] media: venus: add new rate control type MBR for
 encoder
Content-Language: en-US
To: Sachin Kumar Garg <quic_sachinku@quicinc.com>, hverkuil-cisco@xs4all.nl,
 Mauro Carvalho Chehab <mchehab@kernel.org>,
 Stanimir Varbanov <stanimir.k.varbanov@gmail.com>,
 Vikash Garodia <quic_vgarodia@quicinc.com>, Andy Gross <agross@kernel.org>,
 Bjorn Andersson <andersson@kernel.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
 linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-arm-msm@vger.kernel.org
References: <20240118105934.137919-1-quic_sachinku@quicinc.com>
 <20240118105934.137919-3-quic_sachinku@quicinc.com>
From: Konrad Dybcio <konrad.dybcio@linaro.org>
In-Reply-To: <20240118105934.137919-3-quic_sachinku@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


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@quicinc.com>
> ---
>   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)?

[...]

> --- 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?

Konrad