Received-SPF: pass (google.com: domain of linux-kernel+bounces-6584-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <d3dfefbe-0aee-4b4a-ac5c-32db29d691c9@quicinc.com>
Date: Wed, 20 Dec 2023 14:07:37 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v2 17/34] media: iris: implement vb2_ops queue setup
Content-Language: en-US
To: Konrad Dybcio <konrad.dybcio@linaro.org>, <linux-media@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <stanimir.k.varbanov@gmail.com>,
        <quic_vgarodia@quicinc.com>, <agross@kernel.org>,
        <andersson@kernel.org>, <mchehab@kernel.org>,
        <bryan.odonoghue@linaro.org>
CC: <linux-arm-msm@vger.kernel.org>, <quic_abhinavk@quicinc.com>
References: <1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com>
 <1702899149-21321-18-git-send-email-quic_dikshita@quicinc.com>
 <cb2a921c-5c8e-44c8-af1d-78d877977b62@linaro.org>
From: Dikshita Agarwal <quic_dikshita@quicinc.com>
In-Reply-To: <cb2a921c-5c8e-44c8-af1d-78d877977b62@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit


On 12/19/2023 5:26 PM, Konrad Dybcio wrote:
> On 18.12.2023 12:32, Dikshita Agarwal wrote:
>> Implement queue_setup vb2_ops.
>> Calculate the buffer count and buffer size as par video
>> hardware requirement and updates to client.
>> Also, allocate the video driver buffers for output and
>> capture plane.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
> [...]
> 
>> +static int input_min_count(struct iris_inst *inst)
>> +{
>> +	return MIN_BUFFERS;
>> +}
> Why is this a function?
> 
Some extra checks will be performed to calculate the input buffers for
encoder, hence it was made a function. May be I can directly use the macro
at this point and introduce the function later in the patch with encoder
patches.
>> +
>> +static int output_min_count(struct iris_inst *inst)
>> +{
>> +	int output_min_count;
>> +
>> +	switch (inst->codec) {
>> +	case H264:
>> +	case HEVC:
>> +		output_min_count = 4;
>> +		break;
>> +	case VP9:
>> +		output_min_count = 9;
>> +		break;
>> +	default:
>> +		output_min_count = 4;
>> +		break;
>> +	}
> 
> switch (inst->codec) {
> case VP9:
> 	return 9;
> case H264:
> case HEVC:
> default:
> 	return 4;
> }
> 
Right, this is better, will make the change.
>> +
>> +	return output_min_count;
>> +}
>> +
>> +int iris_get_buf_min_count(struct iris_inst *inst,
>> +			   enum iris_buffer_type buffer_type)
>> +{
>> +	switch (buffer_type) {
>> +	case BUF_INPUT:
>> +		return input_min_count(inst);
>> +	case BUF_OUTPUT:
>> +		return output_min_count(inst);
>> +	default:
>> +		return 0;
>> +	}
>> +}
>> +
>> +static u32 input_buffer_size(struct iris_inst *inst)
>> +{
>> +	u32 base_res_mbs = NUM_MBS_4k;
>> +	u32 frame_size, num_mbs;
>> +	struct v4l2_format *f;
>> +	u32 div_factor = 1;
>> +	u32 codec;
>> +
>> +	f = inst->fmt_src;
>> +	codec = f->fmt.pix_mp.pixelformat;
>> +
>> +	num_mbs = get_mbpf(inst);
>> +	if (num_mbs > NUM_MBS_4k) {
>> +		div_factor = 4;
>> +		base_res_mbs = inst->cap[MBPF].value;
>> +	} else {
>> +		base_res_mbs = NUM_MBS_4k;
>> +		if (codec == V4L2_PIX_FMT_VP9)
>> +			div_factor = 1;
>> +		else
>> +			div_factor = 2;
>> +	}
>> +
>> +	frame_size = base_res_mbs * MB_IN_PIXEL * 3 / 2 / div_factor;
> that's a bit magic..
> 
This is the formula to calculate video frame size, no magic here :)
>> +
>> +	 /* multiply by 10/8 (1.25) to get size for 10 bit case */
> misaligned
> 
Oh! wasn't caught by check-patch.
Thanks for pointing this, Will fix.
> 
> Konrad