Re: [PATCH v2] media: iris: add new rate control type MBR for encoder

[Sachin Kumar Garg <sachin.garg@oss.qualcomm.com>]

On 7/21/2026 2:44 AM, Dmitry Baryshkov wrote:
> On Mon, Jul 20, 2026 at 10:44:19PM +0530, Sachin Kumar Garg wrote:
>> Add support for the MBR rate-control mode on the encoder.
>> MBR has no corresponding V4L2-spec definition, it is exposed as a
>> Qualcomm IRIS vendor-specific control rather than a generic
>> bitrate-mode value.
>>
>> Reserve V4L2_CID_USER_QCOM_IRIS_BASE for Qualcomm IRIS driver
>> specific controls and add a new boolean control,
>> V4L2_CID_QCOM_MBR_RATE_CONTROL, to enable it. When set, the driver
>> programs the firmware with HFI_RATE_CONTROL_MBR, taking priority
>> over V4L2_CID_MPEG_VIDEO_BITRATE_MODE when both are client-set.
>>
>> MBR is currently supported only on sc7280. Rather than duplicating
>> the encoder capability table per platform, add a mbr_rc_supported
>> flag to struct iris_platform_data, and merge in a small
>> platform-specific capability table containing only the MBR cap for
>> platforms that support it.
>>
>> Signed-off-by: Sachin Kumar Garg <sachin.garg@oss.qualcomm.com>
>> ---
>> This patch adds support for the MBR rate-control mode in the Iris driver.
>>
>> Changes in v2:
>> - Per upstream review feedback, convert MBR enablement from the
>>    generic V4L2_MPEG_VIDEO_BITRATE_MODE_MBR menu value to a new
>>    Qualcomm IRIS vendor-specific boolean control,
>>    V4L2_CID_QCOM_MBR_RATE_CONTROL.
>> - Drop the now-unneeded generic V4L2_MPEG_VIDEO_BITRATE_MODE_MBR
>>    enum, its "Maximum Bitrate" menu string, and its documentation
>>    (previously sent as a separate patch 1/2).
>> - Drop the separate inst_fw_cap_sc7280_enc[] table (a near-complete
>>    duplicate of inst_fw_cap_sm8250_enc[]); sc7280 now reuses the
>>    shared encoder cap table and layers in only the MBR-specific cap
>>    entry via the new mbr_rc_supported flag.
> 
> No. If we have capabiltiy arrays, please extend them. Does SM8250
> firmware support MBR mode? Is it supported by AR50LT firmware?
> 
> Another option is to rework other entries in the caps array. Describe
> logical platform flags (describing possible modes, profiles, etc.,
> etc.), generating V4L2 controls on the fly.
> 

SM8250 or AR50LT firmware does not implement HFI_RATE_CONTROL_MBR. This 
is sc7280-only enabled feature. sc7280 and sm8250 currently share same 
array (inst_fw_cap_sm8250_enc) via iris_hfi_gen1_data. Adding 
MBR_RATE_CONTROL there would register the control on sm8250 as well, 
which is wrong on its own. Also, every sm8250 stream-on would also 
invoke iris_set_mbr_rate_control() via .set() function and attempt to 
program a rate-control mode that firmware doesn't support.  That means 
some form of platform gating is required.
In the current change: mbr_rc_supported + a small overlay table keeps 
that gate in exactly one place (iris_session_init_caps()), rather than 
adding a platform check inside iris_set_properties()/iris_ctrls_init()'s 
generic loops. It's the same idea as other platform capability bits 
already in iris_platform_data (e.g. no_aon), applied to a cap-table 
entry instead of a runtime branch. Hence idea is not to duplicate the 
other ~25 shared gen1 encoder cap entries into per-SoC tables just to 
append one MBR entry to sc7280's copy.

>> - Link to v1: https://lore.kernel.org/linux-media/20260213-b4-add_sc7280_mbr-v1-0-e8d95b4e4809@oss.qualcomm.com/
>> ---
>>   drivers/media/platform/qcom/iris/iris_ctrls.c      | 101 +++++++++++++++++----
>>   drivers/media/platform/qcom/iris/iris_ctrls.h      |   4 +
>>   .../platform/qcom/iris/iris_hfi_gen1_defines.h     |   1 +
>>   .../platform/qcom/iris/iris_platform_common.h      |   2 +
>>   .../media/platform/qcom/iris/iris_platform_vpu2.c  |   1 +
>>   include/uapi/linux/v4l2-controls.h                 |   6 ++
>>   6 files changed, 98 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
>> index 10e33b8a73f6..0a979649663b 100644
>> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
>> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
>> @@ -154,6 +154,8 @@ static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id)
>>   		return LAYER4_BITRATE_HEVC;
>>   	case V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR:
>>   		return LAYER5_BITRATE_HEVC;
>> +	case V4L2_CID_QCOM_MBR_RATE_CONTROL:
>> +		return MBR_RATE_CONTROL;
>>   	default:
>>   		return INST_FW_CAP_MAX;
>>   	}
>> @@ -297,6 +299,8 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id)
>>   		return V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_BR;
>>   	case LAYER5_BITRATE_HEVC:
>>   		return V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_BR;
>> +	case MBR_RATE_CONTROL:
>> +		return V4L2_CID_QCOM_MBR_RATE_CONTROL;
>>   	default:
>>   		return 0;
>>   	}
>> @@ -335,6 +339,17 @@ static const struct v4l2_ctrl_ops iris_ctrl_ops = {
>>   	.s_ctrl = iris_op_s_ctrl,
>>   };
>>   
>> +static const struct v4l2_ctrl_config iris_mbr_rate_control_ctrl_config = {
>> +	.ops = &iris_ctrl_ops,
>> +	.id = V4L2_CID_QCOM_MBR_RATE_CONTROL,
>> +	.name = "MBR Rate Control",
>> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
>> +	.min = 0,
>> +	.max = 1,
>> +	.step = 1,
>> +	.def = 0,
> 
> How does this play with other rate control mechanisms? Will it allow
> user to select multiple?
> 

Both V4L2_CID_QCOM_MBR_RATE_CONTROL and V4L2_CID_MPEG_VIDEO_BITRATE_MODE 
can be set by the client — the control framework doesn't prevent it. At 
apply time, iris_set_bitrate_mode_gen1() checks whether MBR_RATE_CONTROL 
is client-set and enabled; if so, it takes priority and programs 
HFI_RATE_CONTROL_MBR, and BITRATE_MODE's value has no effect for that 
session. This is intentional and mentioned in the commit message.

>> +};
>> +
>>   int iris_ctrls_init(struct iris_inst *inst)
>>   {
>>   	struct platform_inst_fw_cap *cap = &inst->fw_caps[0];
>