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

Hsia-Jun Li
2022-11-11T03:04:23Z

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