LinuxLists.cc - [PATCH v2 00/12] RkVDEC HEVC driver

2023-01-12 22:25:05

by Nicolas Dufresne

[permalink] [raw]

Subject: Re: [PATCH v2 11/12] staging: media: rkvdec: Enable S_CTRL IOCTL

Le jeudi 12 janvier 2023 à 12:09 -0300, Ezequiel Garcia a écrit :
> Hi Sebastian,
>
> On Thu, Jan 12, 2023 at 9:57 AM Sebastian Fricke
> <[email protected]> wrote:
> >
> > Enable user-space to set the SPS of the current byte-stream on the
> > decoder. This action will enable the decoder to pick the optimal
> > pixel-format for the capture queue, whenever it is required.
> >
> > Signed-off-by: Sebastian Fricke <[email protected]>
> > Signed-off-by: Jonas Karlman <[email protected]>
> > ---
> > drivers/staging/media/rkvdec/rkvdec.c | 81 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 81 insertions(+)
> >
> > diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> > index b303c6e0286d..3d413c5ad1d2 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec.c
> > @@ -93,6 +93,79 @@ static int rkvdec_get_sps_attributes(struct rkvdec_ctx *ctx, void *sps,
> > return 0;
> > }
> >
> > +static int rkvdec_set_sps(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl)
> > +{
> > + struct v4l2_pix_format_mplane *pix_mp;
> > + struct sps_attributes attributes = {0};
> > + void *new_sps = NULL;
> > +
> > + /*
> > + * SPS structures are not filled until the control handler is set up
> > + */
> > + if (!ctx->fh.ctrl_handler)
> > + return 0;
>
> The control handler is embedded in the context, and the fh.ctrl_handler
> is initialized when the context is returned.
>
> You cannot have a context without a control handler (see hantro_open).
>
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_STATELESS_H264_SPS:
> > + new_sps = (void *)ctrl->p_new.p_h264_sps;
> > + break;
> > + case V4L2_CID_STATELESS_HEVC_SPS:
> > + new_sps = (void *)ctrl->p_new.p_hevc_sps;
> > + break;
> > + default:
> > + dev_err(ctx->dev->dev, "Unsupported stateless control ID: %x\n", ctrl->id);
> > + return -EINVAL;
> > + };
> > + rkvdec_get_sps_attributes(ctx, new_sps, &attributes);
> > +
> > + /*
> > + * Providing an empty SPS is valid but we do not store it.
> > + */
> > + if (attributes.width == 0 && attributes.height == 0)
> > + return 0;
> > +
> > + pix_mp = &ctx->decoded_fmt.fmt.pix_mp;
> > +
> > + /*
> > + * SPS must match the provided format dimension, if it doesn't userspace has to
> > + * first reset the output format
>
> This comment says it's a mismatch check, but the check is checking for
> "larger than".
>
> Other than that, the general idea looks good, can you rework the series to avoid
> the extra storage of the SPS control in the context?

I'm not fan, but the careless alignment code states that the coded size cannot
be lower then 64x64, but while running the conformance, there is bunch of files
that have coded dimensions lower. And here's the reason why its not ==. I don't
really like this, but confusing allocation padding and coded size capability
seems to be deep down into rkvdec driver design.

If I only look at the FMT(OUTPUT) behaviour, anything < 64x64 is strictly not
supported by the driver, this the interpretation ffmpeg request code have, and
they are actually right. GStreamer simply does not check anything, and try
(there is not validation code there yet, and I don't want to add any until we
figure-out a solution).

>
> Thanks,
> Ezequiel
>
> > + */
> > + if ((attributes.width > pix_mp->width) || (attributes.height > pix_mp->height)) {
> > + dev_err(ctx->dev->dev,
> > + "Dimension mismatch. [%s SPS] W: %d, H: %d, [Format] W: %d, H: %d)\n",
> > + ctrl->id == V4L2_CID_STATELESS_HEVC_SPS ? "HEVC" : "H264",
> > + attributes.width, attributes.height, pix_mp->width, pix_mp->height);
> > + return -EINVAL;
> > + }
> > +
> > + if (ctx->sps && pix_mp->pixelformat == rkvdec_get_valid_fmt(ctx)) {
> > + /*
> > + * Userspace is allowed to change the SPS at any point, if the
> > + * pixel format doesn't differ from the format in the context,
> > + * just accept the change even if buffers are queued
> > + */
> > + ctx->sps = new_sps;
> > + } else {
> > + /*
> > + * Do not accept changing the SPS, while buffers are queued,
> > + * when the new SPS would cause switching the CAPTURE pixel format
> > + */
> > + if (pix_mp->pixelformat != rkvdec_get_valid_fmt(ctx)) {
> > + if (rkvdec_queue_busy(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE))
> > + return -EBUSY;
> > + }
> > + ctx->sps = new_sps;
> > + /*
> > + * For the initial SPS setting and when the pixel format is
> > + * changed adjust the pixel format stored in the context
> > + */
> > + pix_mp->pixelformat = rkvdec_get_valid_fmt(ctx);
> > + rkvdec_fill_decoded_pixfmt(ctx, pix_mp);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> > {
> > struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> > @@ -104,8 +177,16 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
> > return 0;
> > }
> >
> > +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> > +
> > + return rkvdec_set_sps(ctx, ctrl);
> > +}
> > +
> > static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = {
> > .try_ctrl = rkvdec_try_ctrl,
> > + .s_ctrl = rkvdec_s_ctrl,
> > };
> >
> > static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = {
> >
> > --
> > 2.25.1

2023-01-12 22:47:49

by Nicolas Dufresne

[permalink] [raw]

Subject: Re: [PATCH v2 09/12] staging: media: rkvdec: h264: Add callbacks for h264

Le jeudi 12 janvier 2023 à 12:21 -0300, Ezequiel Garcia a écrit :
> On Thu, Jan 12, 2023 at 9:56 AM Sebastian Fricke
> <[email protected]> wrote:
> >
> > Implement the valid format and sps validation callbacks for H264.
> > H264 already has a SPS validation function, adjust it to fit the
> > function the declaration and add error messages.
> > Additionally, add a function to fetch attributes from the SPS in a human
> > readable format to make the code more self documenting.
> >
> > Signed-off-by: Sebastian Fricke <[email protected]>
> > ---
> > drivers/staging/media/rkvdec/rkvdec-h264.c | 105 ++++++++++++++++++++++-------
> > 1 file changed, 80 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > index 4fc167b42cf0..17b215874ddd 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > @@ -1026,40 +1026,80 @@ static int rkvdec_h264_adjust_fmt(struct rkvdec_ctx *ctx,
> > return 0;
> > }
> >
> > -static int rkvdec_h264_validate_sps(struct rkvdec_ctx *ctx,
> > - const struct v4l2_ctrl_h264_sps *sps)
> > +/*
> > + * Convert some fields from the SPS structure into human readable attributes.
> > + */
> > +static int rkvdec_h264_get_sps_attributes(struct rkvdec_ctx *ctx, void *sps,
> > + struct sps_attributes *attributes)
> > +{
> > + struct v4l2_ctrl_h264_sps *h264_sps = sps;
> > + unsigned int width = (h264_sps->pic_width_in_mbs_minus1 + 1) * 16;
> > + unsigned int height = (h264_sps->pic_height_in_map_units_minus1 + 1) * 16;
> > + /*
> > + * When frame_mbs_only_flag is not set, this is field height,
> > + * which is half the final height (see (7-18) in the
> > + * specification)
> > + */
> > + if (!(h264_sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
> > + height *= 2;
> > +
> > + attributes->width = width;
> > + attributes->height = height;
> > + attributes->luma_bitdepth = h264_sps->bit_depth_luma_minus8 + 8;
> > + attributes->chroma_bitdepth = h264_sps->bit_depth_chroma_minus8 + 8;
> > + switch (h264_sps->chroma_format_idc) {
> > + case 0:
> > + attributes->subsampling = 400;
> > + break;
> > + case 1:
> > + attributes->subsampling = 420;
> > + break;
> > + case 2:
> > + attributes->subsampling = 422;
> > + break;
> > + case 3:
> > + attributes->subsampling = 444;
> > + break;
> > + };
> > + return 0;
> > +}
> > +
> > +static int rkvdec_h264_validate_sps(struct rkvdec_ctx *ctx, void *sps,
> > + struct v4l2_pix_format_mplane *pix_mp)
> > {
> > - unsigned int width, height;
> > + struct sps_attributes attributes = {0};
> > +
> > + rkvdec_h264_get_sps_attributes(ctx, sps, &attributes);
> >
> > /*
> > * TODO: The hardware supports 10-bit and 4:2:2 profiles,
> > * but it's currently broken in the driver.
> > * Reject them for now, until it's fixed.
> > */
> > - if (sps->chroma_format_idc > 1)
> > - /* Only 4:0:0 and 4:2:0 are supported */
> > + if (attributes.subsampling > 420) {
> > + dev_err(ctx->dev->dev,
> > + "Only 4:0:0 and 4:2:0 subsampling schemes are supported, got: %d\n",
> > + attributes.subsampling);
> > return -EINVAL;
> > - if (sps->bit_depth_luma_minus8 != sps->bit_depth_chroma_minus8)
> > - /* Luma and chroma bit depth mismatch */
> > + }
> > + if (attributes.luma_bitdepth != attributes.chroma_bitdepth) {
> > + dev_err(ctx->dev->dev,
> > + "Luma and chroma bit depth mismatch, luma %d, chroma %d\n",
> > + attributes.luma_bitdepth, attributes.chroma_bitdepth);
> > return -EINVAL;
> > - if (sps->bit_depth_luma_minus8 != 0)
> > - /* Only 8-bit is supported */
> > + }
> > + if (attributes.luma_bitdepth != 8) {
> > + dev_err(ctx->dev->dev, "Only 8-bit H264 formats supported, SPS %d\n",
> > + attributes.luma_bitdepth);
> > return -EINVAL;
> > + }
> >
> > - width = (sps->pic_width_in_mbs_minus1 + 1) * 16;
> > - height = (sps->pic_height_in_map_units_minus1 + 1) * 16;
> > -
> > - /*
> > - * When frame_mbs_only_flag is not set, this is field height,
> > - * which is half the final height (see (7-18) in the
> > - * specification)
> > - */
> > - if (!(sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY))
> > - height *= 2;
> > -
> > - if (width > ctx->coded_fmt.fmt.pix_mp.width ||
> > - height > ctx->coded_fmt.fmt.pix_mp.height)
> > + if (attributes.width > pix_mp->width || attributes.height > pix_mp->height) {
> > + dev_err(ctx->dev->dev,
> > + "Incompatible SPS dimension, SPS %dx%d, Pixel format %dx%d.",
> > + attributes.width, attributes.height, pix_mp->width, pix_mp->height);
> > return -EINVAL;
> > + }
> >
> > return 0;
> > }
> > @@ -1077,8 +1117,9 @@ static int rkvdec_h264_start(struct rkvdec_ctx *ctx)
> > if (!ctrl)
> > return -EINVAL;
> >
> > - ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps);
> > - if (ret)
> > + ret = rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps,
> > + &ctx->coded_fmt.fmt.pix_mp);
>
> Not a problem with this patch, but I wonder why we accepted a validation
> in the start_streaming step.

Its something we've been discussing a lot lately Benjamin / Sebastian an I (in
person, sorry for that, but sometimes we work better off chats). This is
something none of the reviewers/authors seems to have actually cared about so
far. In stateless decoders we have FMT(OUTPUT) that contains the coded size,
S_CTRL(SOME_HEADERS) which is restricted by the size in FMT(OUTPUT)
(restrictions depends on the codec, could be ==, or smaller) but also used to
create and limit what FMT(CAPTURE) can be.

One thing that raised, is that the width/height alignment has been carelessly
applied to OUTPUT and CAPTURE, saved by the fact the real value is in SPS, but
possibly leading to strange bugs. Notably we notice that some coded sizes
rejected by S_FMT(OUTOUT) still lead to working streamon and decode :-S.

The other aspect is validation. The initial thinking was that we can validate
the SPS (SOME_HEADERS) directly when the control is set. But we found a gap,
userspace could come back and change the coded size in FMT(OUTPUT) to miss-match
the header. We can't reject that, since FMT(OUTPUT) should just always works,
its the first step according to the doc. The doc does not impose closing the
device and restarting from scratch (even though this is what our userspace
currently though). So in short, if the SPS coded size no longer match the OUTPUT
reported coded size, we needed to reject streamon, hence the validation there.
Of course, there was some more mistakes done, like trying to validate when
executing a job, but I think all of these got caught at review.

On of the reflection we have, is that to be more "semantically" aligned with
V4L2 control framework, we should instead of this ensure the the SPS is always
valid. But this is a bit of work for a usecase that no one needs. In short,
S_FMT(OUTPUT) would always reset the SPS by creating one that is valid and
maches the coded size (when the bitstream header have that, the reason its
duplicated is that we wanted to support light coding format which does not carry
that information in the bitstream). The FMT(CAPTURE) would also be reset to
something valid according to both.

Then we'd validate the SPS control against the FMT(OUTPUT) when set, and always
reset the FMT(CAPTURE) accordingly. This way, no more validation would be
required at streamon, since we ensure everything stay valid by discarding past
settings. Such a method needs careful thought, and remember a bit of work, but
it would help prevent further broken logic, for those who haven't noticed,
Hantro G2 HEVC decoder will pick NV12_4L4 by default when you pass a 10bit SPS,
cause no one noticed until today ... But that's not what the spec is expecting,
as its not a "native" format for the bitstream. What I like about this proposal
is that it would remove any remaining possible side effect from a previous
session, trully allowing for reuse of codec instance, but again, this is
theoritical, as long as you can't crash the driver (which the current validation
is about), its not a real bug, most userspace will just set everything all the
time with these CODEC.

Ok, this is getting long, but I just want to keep in mind this is multiple years
of accumulated mistakes (or just delaying known limitation in driver designs),
we should probably not block features completely over getting this right
immediately, as all it does it makes the actual suffer longer. Though, we should
encourage cross-driver fixes whenever something is improved in one driver.

>
> At this point, the driver accepted all the format negotiations in try_fmt.
> It's difficult for applications to recover from this, as there would
> be no way to tell what failed,
> we would be returning EINVAL, which as per the spec is "buffer type is
> not supported,
> or no buffers have been allocated (memory mapping) or enqueued (output) yet.".
>
> I think it would really make a lot of sense to fix this now, instead of continue
> abusing it. And also, I'd like to prevent a possible anti-pattern from
> spreading.
>
> Thanks!
> Ezequiel
>
> > + if (ret < 0)
> > return ret;
> >
> > h264_ctx = kzalloc(sizeof(*h264_ctx), GFP_KERNEL);
> > @@ -1175,10 +1216,21 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)
> > return 0;
> > }
> >
> > +static u32 rkvdec_h264_valid_fmt(struct rkvdec_ctx *ctx)
> > +{
> > + /*
> > + * Only 8 bit 4:0:0 and 4:2:0 formats supported for now.
> > + * The SPS is validated at a different function thus we can assume that
> > + * it is correct.
> > + */
> > + return V4L2_PIX_FMT_NV12;
> > +}
> > +
> > static int rkvdec_h264_try_ctrl(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl)
> > {
> > if (ctrl->id == V4L2_CID_STATELESS_H264_SPS)
> > - return rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps);
> > + return rkvdec_h264_validate_sps(ctx, ctrl->p_new.p_h264_sps,
> > + &ctx->coded_fmt.fmt.pix_mp);
> >
> > return 0;
> > }
> > @@ -1189,4 +1241,7 @@ const struct rkvdec_coded_fmt_ops rkvdec_h264_fmt_ops = {
> > .stop = rkvdec_h264_stop,
> > .run = rkvdec_h264_run,
> > .try_ctrl = rkvdec_h264_try_ctrl,
> > + .valid_fmt = rkvdec_h264_valid_fmt,
> > + .sps_check = rkvdec_h264_validate_sps,
> > + .get_sps_attributes = rkvdec_h264_get_sps_attributes,
> > };
> >
> > --
> > 2.25.1

2023-01-13 12:55:53

On 2/17/23 01:00, 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 jeudi 16 février 2023 à 16:57 +0800, Hsia-Jun Li a écrit :
>>
>> On 1/12/23 20:56, Sebastian Fricke wrote:
>>> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>>
>>> From: Jonas Karlman <[email protected]>
>>>
>>> Add the NV15 pixel format used by the Rockchip Video Decoder for 10-bit buffers.
>>>
>> I think this pixel format in the Rockchip platform supports multiple
>> planes buffers. It is all right not to add more variant until the ext
>> pixel format and buffer APIs are merged.
>>
>> I just want to mention the need of this.
>
> Can you extend, I don't see that support in rkvdec driver (nor in mpp).
>
> /* config output base address */
> dst_addr = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
> writel_relaxed(dst_addr, rkvdec->regs + RKVDEC_REG_DECOUT_BASE);
>
> reg = RKVDEC_Y_VIRSTRIDE(y_virstride / 16);
> writel_relaxed(reg, rkvdec->regs + RKVDEC_REG_Y_VIRSTRIDE);
>
> reg = RKVDEC_YUV_VIRSTRIDE(yuv_virstride / 16);
> writel_relaxed(reg, rkvdec->regs + RKVDEC_REG_YUV_VIRSTRIDE);
>
> That looks like a base address and 2 strides only. For NV15M (multiple
I think the rockchip drm does
VOP_WIN_SET(vop, win, uv_vir, DIV_ROUND_UP(fb->pitches[1], 4));
VOP_WIN_SET(vop, win, uv_mst, dma_addr);

vop2_win_write(win, VOP2_WIN_UV_VIR, DIV_ROUND_UP(fb->pitches[1], 4));
vop2_win_write(win, VOP2_WIN_UV_MST, uv_mst);

> allocation) you'd need 2 addresses. It could be that RGA or newer chip have that
> support, but as you know, we add formats only when actually using them.
From the public info, it is not just rockchip but also NXP and xilinx
support this 10bits format. I am not sure whether they would support that.

Besides, I think the rockchip decoder needs some spare space between its
Y plane and UV plane. It would be hard to present such thing with the
single plane format unless you want to get the wrong start address of
the chroma plane.
>
> regards,
> Nicolas
>
>>> NV15 is a packed 10-bit 4:2:0 Y/CbCr format similar to P010 and P210 but has no
>>> padding between components. Instead, luminance and chrominance samples are grouped
>>> into 4s so that each group is packed into an integer number of bytes:
>>>
>>> YYYY = UVUV = 4 * 10 bits = 40 bits = 5 bytes
>>>
>>> The '15' suffix refers to the optimum effective bits per pixel
>>> which is achieved when the total number of luminance samples is a multiple
>>> of 8 for NV15.
>>>
>>> Signed-off-by: Jonas Karlman <[email protected]>
>>> Signed-off-by: Sebastian Fricke <[email protected]>
>>> ---
>>> .../userspace-api/media/v4l/pixfmt-yuv-planar.rst | 75 ++++++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-common.c | 2 +
>>> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>>> include/uapi/linux/videodev2.h | 1 +
>>> 4 files changed, 79 insertions(+)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>>> index f1d5bb7b806d..7d8d228f8063 100644
>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
>>> @@ -79,6 +79,13 @@ All components are stored with the same number of bits per component.
>>> - Cr, Cb
>>> - Yes
>>> - Linear
>>> + * - V4L2_PIX_FMT_NV15
>>> + - 'NV15'
>>> + - 15
>>> + - 4:2:0
>>> + - Cb, Cr
>>> + - Yes
>>> + - Linear
>>> * - V4L2_PIX_FMT_NV12M
>>> - 'NM12'
>>> - 8
>>> @@ -183,6 +190,7 @@ horizontally.
>>>
>>> .. _V4L2-PIX-FMT-NV12:
>>> .. _V4L2-PIX-FMT-NV21:
>>> +.. _V4L2-PIX-FMT-NV15:
>>> .. _V4L2-PIX-FMT-NV12M:
>>> .. _V4L2-PIX-FMT-NV21M:
>>> .. _V4L2-PIX-FMT-P010:
>>> @@ -586,6 +594,73 @@ Data in the 10 high bits, zeros in the 6 low bits, arranged in little endian ord
>>> - Cb\ :sub:`11`
>>> - Cr\ :sub:`11`
>>>
>>> +.. _V4L2_PIX_FMT_NV15:
>>> +
>>> +NV15
>>> +----
>>> +
>>> +Similar to P010, a semi-planar 10-bit Y/CbCr format, but all components are
>>> +packed without any padding between each other. As a side-effect, each group of
>>> +4 components are stored over 5 bytes (YYYY or UVUV = 4 * 10 bits = 40 bits = 5
>>> +bytes).
>>> +
>>> +.. flat-table:: Sample 4x4 NV15 Image
>>> + :header-rows: 0
>>> + :stub-columns: 0
>>> +
>>> + * - start + 0:
>>> + - Y'\ :sub:`00`
>>> + - Y'\ :sub:`01`
>>> + - Y'\ :sub:`02`
>>> + - Y'\ :sub:`03`
>>> + * - start + 8:
>>> + - Y'\ :sub:`04`
>>> + - Y'\ :sub:`10`
>>> + - Y'\ :sub:`11`
>>> + - Y'\ :sub:`12`
>>> + * - start + 16:
>>> + - Y'\ :sub:`13`
>>> + - Y'\ :sub:`14`
>>> + - Y'\ :sub:`20`
>>> + - Y'\ :sub:`21`
>>> + * - start + 24:
>>> + - Y'\ :sub:`22`
>>> + - Y'\ :sub:`23`
>>> + - Y'\ :sub:`24`
>>> + - Y'\ :sub:`30`
>>> + * - start + 32:
>>> + - Y'\ :sub:`31`
>>> + - Y'\ :sub:`32`
>>> + - Y'\ :sub:`33`
>>> + - Y'\ :sub:`34`
>>> +
>>> + * - start + 0:
>>> + - Cb\ :sub:`00`
>>> + - Cr\ :sub:`00`
>>> + - Cb\ :sub:`01`
>>> + - Cr\ :sub:`01`
>>> + * - start + 8:
>>> + - Cb\ :sub:`02`
>>> + - Cr\ :sub:`02`
>>> + - Cb\ :sub:`03`
>>> + - Cr\ :sub:`03`
>>> + * - start + 16:
>>> + - Cb\ :sub:`04`
>>> + - Cr\ :sub:`04`
>>> + - Cb\ :sub:`10`
>>> + - Cr\ :sub:`10`
>>> + * - start + 24:
>>> + - Cb\ :sub:`11`
>>> + - Cr\ :sub:`11`
>>> + - Cb\ :sub:`12`
>>> + - Cr\ :sub:`12`
>>> + * - start + 32:
>>> + - Cb\ :sub:`13`
>>> + - Cr\ :sub:`13`
>>> + - Cb\ :sub:`14`
>>> + - Cr\ :sub:`14`
>>> +
>>> +.. raw:: latex
>>>
>>> Fully Planar YUV Formats
>>> ========================
>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
>>> index 40f56e044640..be23e319fb3a 100644
>>> --- a/drivers/media/v4l2-core/v4l2-common.c
>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>>> @@ -262,6 +262,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>>> /* YUV planar formats */
>>> { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
>>> { .format = V4L2_PIX_FMT_NV21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
>>> + { .format = V4L2_PIX_FMT_NV15, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .hdiv = 2, .vdiv = 2,
>>> + .block_w = { 4, 2, 0, 0 }, .block_h = { 1, 1, 0, 0 } },
>>> { .format = V4L2_PIX_FMT_NV16, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
>>> { .format = V4L2_PIX_FMT_NV61, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 },
>>> { .format = V4L2_PIX_FMT_NV24, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> index 8e0a0ff62a70..1c80ad78ef00 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>> @@ -1343,6 +1343,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>> case V4L2_PIX_FMT_M420: descr = "YUV 4:2:0 (M420)"; break;
>>> case V4L2_PIX_FMT_NV12: descr = "Y/UV 4:2:0"; break;
>>> case V4L2_PIX_FMT_NV21: descr = "Y/VU 4:2:0"; break;
>>> + case V4L2_PIX_FMT_NV15: descr = "10-bit Y/UV 4:2:0 (Packed)"; break;
>>> case V4L2_PIX_FMT_NV16: descr = "Y/UV 4:2:2"; break;
>>> case V4L2_PIX_FMT_NV61: descr = "Y/VU 4:2:2"; break;
>>> case V4L2_PIX_FMT_NV24: descr = "Y/UV 4:4:4"; break;
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 1befd181a4cc..e9731286dc77 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -621,6 +621,7 @@ struct v4l2_pix_format {
>>> /* two planes -- one Y, one Cr + Cb interleaved */
>>> #define V4L2_PIX_FMT_NV12 v4l2_fourcc('N', 'V', '1', '2') /* 12 Y/CbCr 4:2:0 */
>>> #define V4L2_PIX_FMT_NV21 v4l2_fourcc('N', 'V', '2', '1') /* 12 Y/CrCb 4:2:0 */
>>> +#define V4L2_PIX_FMT_NV15 v4l2_fourcc('N', 'V', '1', '5') /* 15 Y/CbCr 4:2:0 10-bit packed */
>>> #define V4L2_PIX_FMT_NV16 v4l2_fourcc('N', 'V', '1', '6') /* 16 Y/CbCr 4:2:2 */
>>> #define V4L2_PIX_FMT_NV61 v4l2_fourcc('N', 'V', '6', '1') /* 16 Y/CrCb 4:2:2 */
>>> #define V4L2_PIX_FMT_NV24 v4l2_fourcc('N', 'V', '2', '4') /* 24 Y/CbCr 4:4:4 */
>>>
>>> --
>>> 2.25.1
>>
>

--
Hsia-Jun(Randy) Li