On 11/11/22 13:48, Tomasz Figa wrote:
CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
On Fri, Nov 11, 2022 at 12:04 PM Hsia-Jun Li <Randy.Li@xxxxxxxxxxxxx> wrote:
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.
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 know there are still some devices(encoder) which only have one
register for storing the address of a graphics buffer.
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://urldefense.proofpoint.com/v2/url?u=https-3A__registry.khronos.org_EGL_extensions_EXT_EGL-5FEXT-5Fimage-5Fdma-5Fbuf-5Fimport-5Fmodifiers.txt&d=DwIFaQ&c=7dfBJ8cXbWjhc0BhImu8wVIoUFmBzj1s88r8EGyM0UY&r=P4xb2_7biqBxD4LGGPrSV6j-jf3C3xlR7PXU-mLTeZE&m=mCebYOAiZK6pbpH1MrZGq-ZkDW-OqORCSwsCEX9ScgdXk_yfWZFJPC5aC93CUg5F&s=rtmW_t2LYoJ6g3Y5wgyICmABu-2Npw3JCOlvUVIYH2o&e=
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?
They could be
DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED
DRM_FORMAT_MOD_SYNA_V4H1_128L128_COMPRESSED
DRM_FORMAT_MOD_SYNA_MTR
DRM_FORMAT_MOD_SYNA_MTR
Or
DRM_FORMAT_MOD_SYNA_V4H3P8_64L4
DRM_FORMAT_MOD_SYNA_V4H3P8_64L4
in our platform. It could give a better idea on what is stored in a 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 agree with that. But here is a problem, if there was a display
device(DRM) that only supports contiguous planes in a frame buffer.
How do we be aware of that?
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
--
Hsia-Jun(Randy) Li
