Received-SPF: pass (google.com: domain of linux-kernel+bounces-30049-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240115170826.214519-1-benjamin.gaignard@collabora.com>
 <20240115170826.214519-2-benjamin.gaignard@collabora.com> <c832da45-c818-420d-aaf8-05c15c1e5426@xs4all.nl>
 <521a2a44-9ec1-4898-aca7-721d07e12643@collabora.com>
In-Reply-To: <521a2a44-9ec1-4898-aca7-721d07e12643@collabora.com>
From: Tomasz Figa <tfiga@chromium.org>
Date: Thu, 18 Jan 2024 20:22:53 +0900
Message-ID: <CAAFQd5DWOZwm2E8BwfONtjrcg289wPHJEe3TMZKsBvDcn3OfNw@mail.gmail.com>
Subject: Re: [PATCH] media: media videobuf2: Stop direct calls to queue
 num_buffers field
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>, m.szyprowski@samsung.com, mchehab@kernel.org, 
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, 
	kernel@collabora.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 16, 2024 at 6:32=E2=80=AFPM Benjamin Gaignard
<benjamin.gaignard@collabora.com> wrote:
>
>
> Le 16/01/2024 =C3=A0 10:21, Hans Verkuil a =C3=A9crit :
> > On 15/01/2024 18:08, Benjamin Gaignard wrote:
> >> Use vb2_get_num_buffers() to avoid using queue num_buffers field direc=
tly.
> >> This allows us to change how the number of buffers is computed in the
> >> future.
> >>
> >> Fixes: c838530d230b ("media: media videobuf2: Be more flexible on the =
number of queue stored buffers")
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> >> ---
> >>   drivers/media/common/videobuf2/videobuf2-core.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers=
/media/common/videobuf2/videobuf2-core.c
> >> index 41a832dd1426..b6bf8f232f48 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> >> @@ -989,7 +989,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum=
 vb2_memory memory,
> >>      bool no_previous_buffers =3D !q_num_bufs;
> >>      int ret =3D 0;
> >>
> >> -    if (q->num_buffers =3D=3D q->max_num_buffers) {
> >> +    if (q_num_bufs =3D=3D q->max_num_buffers) {
> >>              dprintk(q, 1, "maximum number of buffers already allocate=
d\n");
> >>              return -ENOBUFS;
> >>      }
> > There is another case in vb2_ioctl_create_bufs() where num_buffers is a=
ccessed directly.
> > Can you fix that one as well?
>
> It is removed by the patch I have send just after this one:
> "media: core: Refactor vb2_ioctl_create_bufs() to always set capabilities=
 flags"

I'd prefer that to be also included in this fix, so that it's all
logically grouped together. Actually Hans also ended up including that
change in his patch, without the commit message mentioning it.

Best regards,
Tomasz