Re: [RFC PATCH v6 02/11] media: v4l2: Extend pixel formats to unify single/multi-planar handling (and more)

[Hsia-Jun Li]

On 11/11/22 01:06, Nicolas Dufresne wrote:
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> 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.

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.

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.

> 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.
>
> Nicolas

--
Hsia-Jun(Randy) Li