Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <0e64b778-e4ff-4b2f-9864-b5d58f642e0e@collabora.com>
Date:   Thu, 14 Dec 2023 16:41:51 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 1/3] videobuf2: core: Rename min_buffers_needed field
 to vb2_queue
To:     Hans Verkuil <hverkuil@xs4all.nl>, mchehab@kernel.org,
        tfiga@chromium.org
Cc:     linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
        linux-staging@lists.linux.dev, kernel@collabora.com
References: <20231211133251.150999-1-benjamin.gaignard@collabora.com>
 <20231211133251.150999-2-benjamin.gaignard@collabora.com>
 <839535af-acba-4240-b9c6-d592d898dc4c@xs4all.nl>
Content-Language: en-US
From:   Benjamin Gaignard <benjamin.gaignard@collabora.com>
In-Reply-To: <839535af-acba-4240-b9c6-d592d898dc4c@xs4all.nl>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


Le 13/12/2023 à 17:39, Hans Verkuil a écrit :
> Hi Benjamin,
>
> On 11/12/2023 14:32, Benjamin Gaignard wrote:
>> Rename min_buffers_needed into min_queued_buffers and update
>> the documentation about it.
> I merged this patch, but not the others. I also dropped one functional
> change:
>
> <snip>
>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
>> index 40d89f29fa33..8912dff5bde3 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-core.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
>> @@ -865,7 +865,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
>>   	/*
>>   	 * Make sure the requested values and current defaults are sane.
>>   	 */
>> -	num_buffers = max_t(unsigned int, *count, q->min_buffers_needed);
>> +	num_buffers = max_t(unsigned int, *count, q->min_queued_buffers + 1);
>>   	num_buffers = min_t(unsigned int, num_buffers, q->max_num_buffers);
>>   	memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
>>   	/*
> That "+ 1" didn't really belong here, since everything else was just renaming a
> field. Such a patch shouldn't make any other changes.
>
> There were also three more occurrences of min_buffers_needed (one in a comment,
> two in a vivid function argument), and I renamed those as well.
>
> 'git grep min_buffers_needed' now no longer shows any hits.
>
> I decided not to take the other patches, I think it is best if you rebase
> and repost the series on top of staging and in the new year we'll continue with
> it. I did not feel that I had enough time to really review the remaining patches.

Do you want me to re-post only the two missing patches or should I add the patches for
delete buffers feature since it is the ultimate goal of this ?

Regards,
Benjamin

>
> However, it is nice to have this large rename patch merged. It touches on a lot
> of files, so it is annoying to have to carry that around. And now was a good
> moment to merge it.
>
> Regards,
>
> 	Hans
>