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

[Dmitry Baryshkov]

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@xxxxxxxxxxxxxxxx>
> ---
> 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.

> - Link to v1: https://lore.kernel.org/linux-media/20260213-b4-add_sc7280_mbr-v1-0-e8d95b4e4809@xxxxxxxxxxxxxxxx/
> ---
>  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?

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

-- 
With best wishes
Dmitry