Re: [PATCH v2 12/24] media: iris: Initialize and deinitialize encoder instance structure

[Dikshita Agarwal <quic_dikshita@quicinc.com>]

On 8/14/2025 7:55 PM, Vikash Garodia wrote:
> 
> On 8/13/2025 3:08 PM, Dikshita Agarwal wrote:
>> Introduce initialization and deinitialization for internal encoder
>> instance structure with necessary hooks.
>>
>> Tested-by: Vikash Garodia <quic_vgarodia@quicinc.com> # X1E80100
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
>>  drivers/media/platform/qcom/iris/Makefile          |  1 +
>>  drivers/media/platform/qcom/iris/iris_buffer.c     | 59 ++++++++++++++++----
>>  .../platform/qcom/iris/iris_hfi_gen1_command.c     |  7 ++-
>>  .../platform/qcom/iris/iris_hfi_gen1_defines.h     |  1 +
>>  drivers/media/platform/qcom/iris/iris_instance.h   |  7 +++
>>  drivers/media/platform/qcom/iris/iris_vdec.c       |  2 -
>>  drivers/media/platform/qcom/iris/iris_venc.c       | 65 ++++++++++++++++++++++
>>  drivers/media/platform/qcom/iris/iris_venc.h       | 14 +++++
>>  drivers/media/platform/qcom/iris/iris_vidc.c       | 27 ++++++++-
>>  drivers/media/platform/qcom/iris/iris_vpu_buffer.c |  5 +-
>>  10 files changed, 170 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
>> index e86d00ee6f15dda8bae2f25f726feb0d427b7684..ec32145e081b1fc3538dfa7d5113162a76a6068c 100644
>> --- a/drivers/media/platform/qcom/iris/Makefile
>> +++ b/drivers/media/platform/qcom/iris/Makefile
>> @@ -19,6 +19,7 @@ qcom-iris-objs += \
>>               iris_vidc.o \
>>               iris_vb2.o \
>>               iris_vdec.o \
>> +             iris_venc.o \
>>               iris_vpu2.o \
>>               iris_vpu3x.o \
>>               iris_vpu_buffer.o \
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index 38548ee4749ea7dd1addf2c9d0677cf5217e3546..6bf9b0b35d206d51b927c824d5a5b327596251c6 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -63,7 +63,12 @@
>>  static u32 iris_yuv_buffer_size_nv12(struct iris_inst *inst)
>>  {
>>  	u32 y_plane, uv_plane, y_stride, uv_stride, y_scanlines, uv_scanlines;
>> -	struct v4l2_format *f = inst->fmt_dst;
>> +	struct v4l2_format *f;
>> +
>> +	if (inst->domain == DECODER)
>> +		f = inst->fmt_dst;
>> +	else
>> +		f = inst->fmt_src;
>>  
>>  	y_stride = ALIGN(f->fmt.pix_mp.width, Y_STRIDE_ALIGN);
>>  	uv_stride = ALIGN(f->fmt.pix_mp.width, UV_STRIDE_ALIGN);
>> @@ -194,7 +199,7 @@ static u32 iris_yuv_buffer_size_qc08c(struct iris_inst *inst)
>>  	return ALIGN(y_meta_plane + y_plane + uv_meta_plane + uv_plane, PIXELS_4K);
>>  }
>>  
>> -static u32 iris_bitstream_buffer_size(struct iris_inst *inst)
>> +static u32 iris_dec_bitstream_buffer_size(struct iris_inst *inst)
>>  {
>>  	struct platform_inst_caps *caps = inst->core->iris_platform_data->inst_caps;
>>  	u32 base_res_mbs = NUM_MBS_4K;
>> @@ -219,18 +224,50 @@ static u32 iris_bitstream_buffer_size(struct iris_inst *inst)
>>  	return ALIGN(frame_size, PIXELS_4K);
>>  }
>>  
>> +static u32 iris_enc_bitstream_buffer_size(struct iris_inst *inst)
>> +{
>> +	u32 aligned_width, aligned_height, bitstream_size, yuv_size;
>> +	struct v4l2_format *f;
>> +
>> +	f = inst->fmt_dst;
>> +
>> +	aligned_width = ALIGN(f->fmt.pix_mp.width, 32);
>> +	aligned_height = ALIGN(f->fmt.pix_mp.height, 32);
>> +	bitstream_size = aligned_width * aligned_height * 3;
>> +	yuv_size = (aligned_width * aligned_height * 3) >> 1;
>> +	if (aligned_width * aligned_height > (4096 * 2176))
>> +		/* bitstream_size = 0.25 * yuv_size; */
>> +		bitstream_size = (bitstream_size >> 3);
>> +	else if (aligned_width * aligned_height > (1280 * 720))
>> +		/* bitstream_size = 0.5 * yuv_size; */
>> +		bitstream_size = (bitstream_size >> 2);
>> +
>> +	return ALIGN(bitstream_size, 4096);
>> +}
>> +
>>  int iris_get_buffer_size(struct iris_inst *inst,
>>  			 enum iris_buffer_type buffer_type)
>>  {
>> -	switch (buffer_type) {
>> -	case BUF_INPUT:
>> -		return iris_bitstream_buffer_size(inst);
>> -	case BUF_OUTPUT:
>> -		return iris_yuv_buffer_size_nv12(inst);
>> -	case BUF_DPB:
>> -		return iris_yuv_buffer_size_qc08c(inst);
>> -	default:
>> -		return 0;
>> +	if (inst->domain == DECODER) {
>> +		switch (buffer_type) {
>> +		case BUF_INPUT:
>> +			return iris_dec_bitstream_buffer_size(inst);
>> +		case BUF_OUTPUT:
>> +			return iris_yuv_buffer_size_nv12(inst);
>> +		case BUF_DPB:
>> +			return iris_yuv_buffer_size_qc08c(inst);
>> +		default:
>> +			return 0;
>> +		}
>> +	} else {
>> +		switch (buffer_type) {
>> +		case BUF_INPUT:
>> +			return iris_yuv_buffer_size_nv12(inst);
>> +		case BUF_OUTPUT:
>> +			return iris_enc_bitstream_buffer_size(inst);
>> +		default:
>> +			return 0;
>> +		}
>>  	}
>>  }
>>  
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> index 5097680ee14ebba3a126213c0584161627ca47d7..eae3dc5c596d1eb6090126ac391b8e0e2c9f09eb 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
>> @@ -109,7 +109,12 @@ static int iris_hfi_gen1_session_open(struct iris_inst *inst)
>>  	packet.shdr.hdr.size = sizeof(struct hfi_session_open_pkt);
>>  	packet.shdr.hdr.pkt_type = HFI_CMD_SYS_SESSION_INIT;
>>  	packet.shdr.session_id = inst->session_id;
>> -	packet.session_domain = HFI_SESSION_TYPE_DEC;
>> +
>> +	if (inst->domain == DECODER)
>> +		packet.session_domain = HFI_SESSION_TYPE_DEC;
>> +	else
>> +		packet.session_domain = HFI_SESSION_TYPE_ENC;
>> +
>>  	packet.session_codec = codec;
>>  
>>  	reinit_completion(&inst->completion);
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
>> index d4d119ca98b0cb313db351f3794bf278216bd539..5b7c641b727a16c3aa7196a6d49786133653279f 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h
>> @@ -10,6 +10,7 @@
>>  
>>  #define HFI_VIDEO_ARCH_OX				0x1
>>  
>> +#define HFI_SESSION_TYPE_ENC				1
>>  #define HFI_SESSION_TYPE_DEC				2
>>  
>>  #define HFI_VIDEO_CODEC_H264				0x00000002
>> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
>> index 0e1f5799b72d993b25820608969e0011eabdb6bc..ff90f010f1d36690cbadeff0787b1fb7458d7f75 100644
>> --- a/drivers/media/platform/qcom/iris/iris_instance.h
>> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
>> @@ -12,6 +12,9 @@
>>  #include "iris_core.h"
>>  #include "iris_utils.h"
>>  
>> +#define DEFAULT_WIDTH 320
>> +#define DEFAULT_HEIGHT 240
>> +
>>  /**
>>   * struct iris_inst - holds per video instance parameters
>>   *
>> @@ -24,7 +27,9 @@
>>   * @fmt_src: structure of v4l2_format for source
>>   * @fmt_dst: structure of v4l2_format for destination
>>   * @ctrl_handler: reference of v4l2 ctrl handler
>> + * @domain: domain type: encoder or decoder
>>   * @crop: structure of crop info
>> + * @compose: structure of compose info
>>   * @completion: structure of signal completions
>>   * @flush_completion: structure of signal completions for flush cmd
>>   * @flush_responses_pending: counter to track number of pending flush responses
>> @@ -57,7 +62,9 @@ struct iris_inst {
>>  	struct v4l2_format		*fmt_src;
>>  	struct v4l2_format		*fmt_dst;
>>  	struct v4l2_ctrl_handler	ctrl_handler;
>> +	enum domain_type		domain;
>>  	struct iris_hfi_rect_desc	crop;
>> +	struct iris_hfi_rect_desc	compose;
>>  	struct completion		completion;
>>  	struct completion		flush_completion;
>>  	u32				flush_responses_pending;
>> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
>> index d670b51c5839d1fad54d34f373cf71d5f3973a96..3482ff3f18d2bcd592b7eb7d803bf98e29276ebb 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
>> @@ -13,8 +13,6 @@
>>  #include "iris_vdec.h"
>>  #include "iris_vpu_buffer.h"
>>  
>> -#define DEFAULT_WIDTH 320
>> -#define DEFAULT_HEIGHT 240
>>  #define DEFAULT_CODEC_ALIGNMENT 16
>>  
>>  int iris_vdec_inst_init(struct iris_inst *inst)
>> diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..e418d347ac111c1bc48304adafa259d697e49fed
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/iris/iris_venc.c
>> @@ -0,0 +1,65 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#include "iris_buffer.h"
>> +#include "iris_instance.h"
>> +#include "iris_venc.h"
>> +#include "iris_vpu_buffer.h"
>> +
>> +int iris_venc_inst_init(struct iris_inst *inst)
>> +{
>> +	struct v4l2_format *f;
>> +
>> +	inst->fmt_src = kzalloc(sizeof(*inst->fmt_src), GFP_KERNEL);
>> +	inst->fmt_dst  = kzalloc(sizeof(*inst->fmt_dst), GFP_KERNEL);
>> +	if (!inst->fmt_src || !inst->fmt_dst)
>> +		return -ENOMEM;
> 
> What if inst->fmt_src succeeds while inst->fmt_dst fails, does it frees up
> inst->fmt_src ?
> 

would you prefer below?

if (!inst->fmt_src || !inst->fmt_dst) {
    kfree(inst->fmt_src);
    kfree(inst->fmt_dst);
    return -ENOMEM;
}

Thanks,
Dikshita
>> +
>> +	f = inst->fmt_dst;
>> +	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>> +	f->fmt.pix_mp.width = DEFAULT_WIDTH;
>> +	f->fmt.pix_mp.height = DEFAULT_HEIGHT;
>> +	f->fmt.pix_mp.pixelformat = V4L2_PIX_FMT_H264;
>> +	inst->codec = f->fmt.pix_mp.pixelformat;
>> +	f->fmt.pix_mp.num_planes = 1;
>> +	f->fmt.pix_mp.plane_fmt[0].bytesperline = 0;
>> +	f->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_OUTPUT);
>> +	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
>> +	f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +	f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +	f->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +	f->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +	inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT);
>> +	inst->buffers[BUF_OUTPUT].size = f->fmt.pix_mp.plane_fmt[0].sizeimage;
>> +
>> +	f = inst->fmt_src;
>> +	f->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>> +	f->fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12;
>> +	f->fmt.pix_mp.width = ALIGN(DEFAULT_WIDTH, 128);
>> +	f->fmt.pix_mp.height = ALIGN(DEFAULT_HEIGHT, 32);
>> +	f->fmt.pix_mp.num_planes = 1;
>> +	f->fmt.pix_mp.plane_fmt[0].bytesperline = ALIGN(DEFAULT_WIDTH, 128);
>> +	f->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_INPUT);
>> +	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
>> +	f->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT;
>> +	f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +	f->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> +	f->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT;
>> +	inst->buffers[BUF_INPUT].min_count = iris_vpu_buf_count(inst, BUF_INPUT);
>> +	inst->buffers[BUF_INPUT].size = f->fmt.pix_mp.plane_fmt[0].sizeimage;
>> +
>> +	inst->crop.left = 0;
>> +	inst->crop.top = 0;
>> +	inst->crop.width = f->fmt.pix_mp.width;
>> +	inst->crop.height = f->fmt.pix_mp.height;
>> +
>> +	return 0;
>> +}
>> +