From: Tomasz Figa <tfiga@chromium.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Hsia-Jun Li <Randy.Li@synaptics.com>,
Nicolas Dufresne <nicolas@ndufresne.ca>,
mchehab@kernel.org, hans.verkuil@cisco.com, sakari.ailus@iki.fi,
boris.brezillon@collabora.com, hiroh@chromium.org,
Brian.Starkey@arm.com, kernel@collabora.com,
narmstrong@baylibre.com, linux-kernel@vger.kernel.org,
frkoenig@chromium.org, stanimir.varbanov@linaro.org,
Hans Verkuil <hverkuil@xs4all.nl>,
linux-media@vger.kernel.org
Subject: Re: [RFC PATCH v6 02/11] media: v4l2: Extend pixel formats to unify single/multi-planar handling (and more)
Date: Fri, 11 Nov 2022 17:54:01 +0900 [thread overview]
Message-ID: <CAAFQd5DZG9QDp-+e8Xs7gKL=ZLkm3hA1-=eJZe8hjoB42xE5qw@mail.gmail.com> (raw)
In-Reply-To: <Y24LBrkveiXlmCMy@pendragon.ideasonboard.com>
On Fri, Nov 11, 2022 at 5:43 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Tomasz,
>
> On Fri, Nov 11, 2022 at 02:48:48PM +0900, Tomasz Figa wrote:
> > On Fri, Nov 11, 2022 at 12:04 PM Hsia-Jun Li wrote:
> > > On 11/11/22 01:06, Nicolas Dufresne wrote:
> > > > Le samedi 05 novembre 2022 à 23:19 +0800, Hsia-Jun Li a écrit :
> > > >>>> VIDIOC_ENUM_EXT_PIX_FMT would report NV12 and NV12M, while
> > > >>>> VIDIOC_ENUM_FMT
> > > >>>> would just report NV12M.
> > > >>>
> > > >>> If NV12 and NV12M are equivalent in Ext API, I don't see why we would
> > > >>> report both (unless I'm missing something, which is probably the case).
> > > >>>
> > > >>> The idea was to deprecate the M-variants one day.
> > > >> I was thinking the way in DRM API is better, always assuming it would
> > > >> always in a multiple planes. The only problem is we don't have a way to
> > > >> let the allocator that allocate contiguous memory for planes when we
> > > >> need to do that.
> > > >
> > > > Its not too late to allow this to be negotiated, but I would move this out of
> > > > the pixel format definition to stop the explosion of duplicate pixel formats,
> > > > which is a nightmare to deal with.
> > >
> > > I wonder whether we need to keep the pixel formats in videodev2.h
> > > anymore. If we would like to use the modifiers from drm_fourcc.h, why
> > > don't we use their pixel formats, they should be the same values of
> > > non-M variant pixel formats of v4l2.
> > >
> > > Let videodev2.h only maintain the those codecs or motion based
> > > compressed (pixel) formats.
> > >
> > > If I simplify the discussion, we want to
> > >
> > > > negotiate contiguity with the driver. The new FMT structure should have a
> > > > CONTIGUOUS flag. So if userpace sets:
> > > >
> > > > S_FMT(NV12, CONTIGUOUS)
> > >
> > > I wonder whether we would allow some planes being contiguous while some
> > > would not. For example, the graphics planes could be in a contiguous
> > > memory address while its compression metadata are not.
> > > Although that is not the case of our platform. I believe it sounds like
> > > reasonable case for improving the performance, two meta planes could
> > > resident in a different memory bank.
> >
> > I feel like this would be only useful in the MMAP mode. Looking at how
> > the other UAPIs are evolving, things are going towards
> > userspace-managed allocations, using, for example, DMA-buf heaps. I
> > think we should follow the trend and keep the MMAP mode just at the
> > same level of functionality as is today and focus on improvements and
> > new functionality for the DMABUF mode.
>
> I agree, but we will need an API to expose the memory constraints of the
> device, or userspace won't be able to allocate memory compatible with
> the hardware or driver requirements.
Yes, I fully agree and that's why I think we should rather focus our
efforts in that direction rather than expanding the existing MMAP
capabilities.
>
> > > That lead to another question which I forgot whether I mention it before.
> > >
> > > There are four modifiers in DRM while we would only one in these patches.
> > > From the EGL
> > > https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_image_dma_buf_import_modifiers.txt
> > >
> > > The modifier for echo plane could be different. I wish it would be
> > > better to create a framebuffer being aware of which planes are graphics
> > > or metadata.
> >
> > What's an echo plane?
> >
> > That said, it indeed looks like we may want to be consistent with DRM
> > here and allow per-plane modifiers.
> >
> > > I wonder whether it would be better that convincing the DRM maintainer
> > > adding a non vendor flag for contiguous memory allocation here(DRM
> > > itself don't need it).
> > > While whether the memory could be contiguous for these vendor pixel
> > > formats, it is complex vendor defined.
> >
> > Memory allocation doesn't sound to me like it is related to formats or
> > modifiers in any way. I agree with Nicolas that if we want to allow
> > the userspace to specify if the memory should be contiguous or not,
> > that should be a separate flag and actually I'd probably see it in
> > REQBUF_EXT and CREATE_BUFS_EXT, rather than as a part of the format.
>
> I like how DRM decouples allocation of buffer objects and creation of
> frame buffers.
Exactly why I proposed so rather than coupling it with S_FMT. (But
then it's moot if we decide to focus on DMABUF mode.)
>
> > > > The driver can accepts, and return the unmodified structure, or may drop the
> > > > CONTIGUOUS flag, which would mean its not supported. Could be the other way
> > > > around too. As for allocation, if you have CONTIGUOUS flag set, userspace does
> > > > not have to export or map memory for each planes, as they are the same. We
> > > > simply need to define the offset as relative to their allocation, which I think
> > > > is the most sensible thing.
>
> --
> Regards,
>
> Laurent Pinchart
next prev parent reply other threads:[~2022-11-11 8:54 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-14 18:07 [RFC PATCH v6 00/11] media: v4l2: Add extended fmt and buffer ioctls Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 01/11] media: v4l2-common: add normalized pixelformat field to struct v4l2_format_info Helen Koike
2021-02-10 12:37 ` Dafna Hirschfeld
2021-01-14 18:07 ` [RFC PATCH v6 02/11] media: v4l2: Extend pixel formats to unify single/multi-planar handling (and more) Helen Koike
2021-02-10 15:02 ` Dafna Hirschfeld
2021-02-23 12:35 ` Hans Verkuil
2021-02-24 15:12 ` Helen Koike
2022-11-05 15:19 ` Hsia-Jun Li
2022-11-06 19:24 ` Laurent Pinchart
2022-11-07 1:54 ` Hsia-Jun Li
2022-11-07 8:28 ` Laurent Pinchart
2022-11-07 8:49 ` Hsia-Jun Li
2022-11-06 22:11 ` Dmitry Osipenko
2022-11-07 2:04 ` Hsia-Jun Li
2022-11-07 8:30 ` Laurent Pinchart
2022-11-08 14:58 ` Dmitry Osipenko
2022-11-07 16:50 ` Fritz Koenig
2022-11-07 8:42 ` Hans Verkuil
2022-11-10 17:06 ` Nicolas Dufresne
2022-11-11 3:03 ` Hsia-Jun Li
2022-11-11 5:48 ` Tomasz Figa
2022-11-11 6:30 ` Hsia-Jun Li
2022-11-11 8:52 ` Tomasz Figa
2022-11-11 9:13 ` Hsia-Jun Li
2022-11-15 16:03 ` Nicolas Dufresne
2022-11-16 12:38 ` ayaka
2022-11-11 8:42 ` Laurent Pinchart
2022-11-11 8:54 ` Tomasz Figa [this message]
2022-11-15 16:19 ` Nicolas Dufresne
2022-11-15 15:57 ` Nicolas Dufresne
2021-01-14 18:07 ` [RFC PATCH v6 03/11] media: v4l2: Add extended buffer (de)queue operations for video types Helen Koike
2021-02-23 12:58 ` Hans Verkuil
2023-01-26 7:07 ` ayaka
[not found] ` <20230125200026.16643-1-ayaka@soulik.info>
2023-01-26 8:57 ` Hans Verkuil
2023-01-26 11:02 ` Laurent Pinchart
2023-01-26 18:36 ` ayaka
2023-01-27 8:11 ` Hans Verkuil
2023-01-30 10:07 ` Hsia-Jun Li
2023-01-30 12:17 ` Hans Verkuil
2021-01-14 18:07 ` [RFC PATCH v6 04/11] media: videobuf2-v4l2: reorganize flags handling Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 05/11] media: videobuf2: Expose helpers for Ext qbuf/dqbuf Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 06/11] media: vivid: use vb2_ioctls_ext_{d}qbuf hooks Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 07/11] media: vimc: " Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 08/11] media: mediabus: Add helpers to convert a ext_pix format to/from a mbus_fmt Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 09/11] media: vivid: Convert to v4l2_ext_pix_format Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 10/11] media: vimc: " Helen Koike
2021-01-14 18:07 ` [RFC PATCH v6 11/11] media: docs: add documentation for the Extended API Helen Koike
2021-02-05 18:39 ` [RFC PATCH v6 00/11] media: v4l2: Add extended fmt and buffer ioctls Helen Koike
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAFQd5DZG9QDp-+e8Xs7gKL=ZLkm3hA1-=eJZe8hjoB42xE5qw@mail.gmail.com' \
--to=tfiga@chromium.org \
--cc=Brian.Starkey@arm.com \
--cc=Randy.Li@synaptics.com \
--cc=boris.brezillon@collabora.com \
--cc=frkoenig@chromium.org \
--cc=hans.verkuil@cisco.com \
--cc=hiroh@chromium.org \
--cc=hverkuil@xs4all.nl \
--cc=kernel@collabora.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=narmstrong@baylibre.com \
--cc=nicolas@ndufresne.ca \
--cc=sakari.ailus@iki.fi \
--cc=stanimir.varbanov@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).