Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6f2979d33526e5ccdc32cf096415d8309fc91d3d.camel@ndufresne.ca>
Subject: Re: [RFC 2/4] media: videobuf2: Replace bufs array by a list
From:   Nicolas Dufresne <nicolas@ndufresne.ca>
To:     Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     David Laight <David.Laight@ACULAB.COM>,
        Benjamin Gaignard <benjamin.gaignard@collabora.com>,
        "tfiga@chromium.org" <tfiga@chromium.org>,
        "m.szyprowski@samsung.com" <m.szyprowski@samsung.com>,
        "mchehab@kernel.org" <mchehab@kernel.org>,
        "ming.qian@nxp.com" <ming.qian@nxp.com>,
        "shijie.qin@nxp.com" <shijie.qin@nxp.com>,
        "eagle.zhou@nxp.com" <eagle.zhou@nxp.com>,
        "bin.liu@mediatek.com" <bin.liu@mediatek.com>,
        "matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
        "angelogioacchino.delregno@collabora.com" 
        <angelogioacchino.delregno@collabora.com>,
        "tiffany.lin@mediatek.com" <tiffany.lin@mediatek.com>,
        "andrew-ct.chen@mediatek.com" <andrew-ct.chen@mediatek.com>,
        "yunfei.dong@mediatek.com" <yunfei.dong@mediatek.com>,
        "stanimir.k.varbanov@gmail.com" <stanimir.k.varbanov@gmail.com>,
        "quic_vgarodia@quicinc.com" <quic_vgarodia@quicinc.com>,
        "agross@kernel.org" <agross@kernel.org>,
        "andersson@kernel.org" <andersson@kernel.org>,
        "konrad.dybcio@linaro.org" <konrad.dybcio@linaro.org>,
        "ezequiel@vanguardiasur.com.ar" <ezequiel@vanguardiasur.com.ar>,
        "p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
        "daniel.almeida@collabora.com" <daniel.almeida@collabora.com>,
        "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-mediatek@lists.infradead.org" 
        <linux-mediatek@lists.infradead.org>,
        "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
        "linux-rockchip@lists.infradead.org" 
        <linux-rockchip@lists.infradead.org>,
        "kernel@collabora.com" <kernel@collabora.com>
Date:   Fri, 24 Mar 2023 11:34:48 -0400
In-Reply-To: <68ba7b3f-57f5-3969-5036-2c8d08273548@xs4all.nl>
References: <20230313135916.862852-1-benjamin.gaignard@collabora.com>
         <20230313135916.862852-3-benjamin.gaignard@collabora.com>
         <20230313181155.GC22646@pendragon.ideasonboard.com>
         <86df05244d974416903e919d387a0a0b@AcuMS.aculab.com>
         <e704b505-86d8-c6f2-8546-adccdab72622@xs4all.nl>
         <dc04d48e34ed40e58f43badd001a81d0@AcuMS.aculab.com>
         <cbf34cf1-e065-8136-8344-89ca1864f637@xs4all.nl>
         <20230319233358.GD20234@pendragon.ideasonboard.com>
         <f085aa9225c573df906bdc7ff032a8fd591b18b3.camel@ndufresne.ca>
         <20230322150153.GO20234@pendragon.ideasonboard.com>
         <2d6480e36ce061a63440d1e11d52b02e57ba746d.camel@ndufresne.ca>
         <68ba7b3f-57f5-3969-5036-2c8d08273548@xs4all.nl>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

Le vendredi 24 mars 2023 =C3=A0 16:18 +0100, Hans Verkuil a =C3=A9crit=C2=
=A0:
> On 24/03/2023 16:14, Nicolas Dufresne wrote:
> > Le mercredi 22 mars 2023 =C3=A0 17:01 +0200, Laurent Pinchart a =C3=A9c=
rit=C2=A0:
> > > On Wed, Mar 22, 2023 at 10:50:52AM -0400, Nicolas Dufresne wrote:
> > > > Hi Laurent,
> > > >=20
> > > > Le lundi 20 mars 2023 =C3=A0 01:33 +0200, Laurent Pinchart a =C3=A9=
crit=C2=A0:
> > > > > > The typical usage is that applications allocate N buffers with =
the
> > > > > > VIDIOC_REQBUFS ioctl, and in most cases that's all they use.
> > > > >=20
> > > > > Note that once we get DELETE_BUF (or DELETE_BUFS) support I'd lik=
e to
> > > > > encourage applications to use the new API, and deprecate REQBUFS
> > > > > (dropping it isn't on my radar, as it would take forever before n=
o
> > > > > userspace uses it anymore).
> > > >=20
> > > > I was wondering if you can extend on this. I'm worried the count se=
mantic might
> > > > prevent emulating it over create_bufs() ops, but if that works, did=
 you meant to
> > > > emulate it so driver no longer have to implement reqbufs() if they =
have
> > > > create_bufs() ?
> > >=20
> > > For drivers it should be fairly simply, as the reqbufs and create_buf=
s
> > > ioctl handlers should just point to the corresponding videobuf2 helpe=
rs.
> > >=20
> > > What I meant is that I'd like to encourage userspace to use the
> > > VIDIOC_CREATE_BUFS ioctl instead of VIDIOC_REQBUFS.
> > >=20
> >=20
> > I'm not sure what rationale I can give implementer to "encourage" them =
to use a
> > more complex API that needs to copy over the FMT (which has just been s=
et),
> > specially in the initial pre-allocation case. For most, CREATE_BUFS aft=
er SMT
> > will look like a very redundant and counter intuitive thing. Maybe you =
have a
> > more optimistic view on the matter ? Or you have a better idea how we c=
ould give
> > a meaning to having a fmt there on the initial case where the allocatio=
n matches
> > the queue FMT ?
>=20
> I wouldn't mind if we can make a much nicer CREATE_BUFS variant with just=
 the
> size instead of a format. That was in hindsight a really bad idea, terrib=
le
> over-engineering.

Note that all DRM allocators also includes width/height and some format rel=
ated
info (or the full info). This is because the driver deals with the alignmen=
t
requirements. In some use cases (I have inter frame dynamic control in mind
here) the fmt could be a mean to feedback the alignment (like bytesperline)=
 back
to the application where the stream is no longer homogeneous on the FMT.

That being said, If we move toward a size base allocator API, we could also=
 just
point back to an existing HEAP (or export an new heap if none are valid). A=
nd
define the sizeimage(s) is now that information you need from the FMT to
allocate anything + which heap needs to be used for the current setup.

Nicolas

>=20
> Regards,
>=20
> 	Hans