Re: [PATCH v7 3/6] media: qcom: iris: Add B frames support for encoder

[Dmitry Baryshkov]

On Tue, May 12, 2026 at 04:55:12PM +0800, Wangao Wang wrote:
> Add support for B-frame configuration on both gen1 and gen2 encoders by
> enabling V4L2_CID_MPEG_VIDEO_B_FRAMES control.
> 
> Reviewed-by: Dikshita Agarwal <dikshita.agarwal@xxxxxxxxxxxxxxxx>
> Tested-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> # on SM8650-HDK
> Signed-off-by: Wangao Wang <wangao.wang@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/iris/iris_ctrls.c      | 30 ++++++++++++++++++++++
>  drivers/media/platform/qcom/iris/iris_ctrls.h      |  1 +
>  drivers/media/platform/qcom/iris/iris_hfi_gen1.c   | 18 +++++++++++++
>  .../platform/qcom/iris/iris_hfi_gen1_command.c     |  8 ++++++
>  .../platform/qcom/iris/iris_hfi_gen1_defines.h     | 10 ++++++++
>  drivers/media/platform/qcom/iris/iris_hfi_gen2.c   | 10 ++++++++
>  .../platform/qcom/iris/iris_platform_common.h      |  2 ++
>  drivers/media/platform/qcom/iris/iris_vpu_buffer.c |  6 ++++-
>  8 files changed, 84 insertions(+), 1 deletion(-)
> 
> +int iris_set_intra_period(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id)
> +{
> +	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;
> +	u32 gop_size = inst->fw_caps[GOP_SIZE].value;
> +	u32 b_frame = inst->fw_caps[B_FRAME].value;
> +	u32 hfi_id = inst->fw_caps[cap_id].hfi_id;
> +	struct hfi_intra_period intra_period;
> +
> +	if (!gop_size || b_frame >= gop_size)
> +		return -EINVAL;

The same comment. Maybe I misunderstand something, please correct me if
I'm wrong. The definition of the GOP_SIZE capability allows 0 as a valid
value. Here you are declining it. Why?

> +
> +	/*
> +	 * intra_period represents the length of a GOP, which includes both P-frames
> +	 * and B-frames. The counts of P-frames and B-frames within a GOP must be
> +	 * communicated to the firmware.
> +	 */
> +	intra_period.pframes = (gop_size - 1) / (b_frame + 1);
> +	intra_period.bframes = b_frame;
> +
> +	return hfi_ops->session_set_property(inst, hfi_id,
> +					     HFI_HOST_FLAGS_NONE,
> +					     iris_get_port_info(inst, cap_id),
> +					     HFI_PAYLOAD_STRUCTURE,
> +					     &intra_period, sizeof(intra_period));
> +}
> +
>  int iris_set_properties(struct iris_inst *inst, u32 plane)
>  {
>  	const struct iris_hfi_session_ops *hfi_ops = inst->hfi_session_ops;

-- 
With best wishes
Dmitry