From: "Aakarsh Jain" <aakarsh.jain@samsung.com>
To: "'Nicolas Dufresne'" <nicolas@ndufresne.ca>,
<linux-arm-kernel@lists.infradead.org>,
<linux-media@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <m.szyprowski@samsung.com>, <andrzej.hajda@intel.com>,
<mchehab@kernel.org>, <hverkuil-cisco@xs4all.nl>,
<krzysztof.kozlowski+dt@linaro.org>,
<linux-samsung-soc@vger.kernel.org>, <gost.dev@samsung.com>,
<aswani.reddy@samsung.com>, <pankaj.dubey@samsung.com>
Subject: RE: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes
Date: Wed, 7 Aug 2024 17:23:15 +0530 [thread overview]
Message-ID: <047401dae8c0$65352ec0$2f9f8c40$@samsung.com> (raw)
In-Reply-To: <3765b1674e276afdc302def55327396a0a29cc63.camel@ndufresne.ca>
[-- Attachment #1: Type: text/plain, Size: 4426 bytes --]
Hi Nocolas,
> -----Original Message-----
> From: Nicolas Dufresne <nicolas@ndufresne.ca>
> Sent: 06 August 2024 20:08
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; hverkuil-cisco@xs4all.nl;
> krzysztof.kozlowski+dt@linaro.org; linux-samsung-soc@vger.kernel.org;
> gost.dev@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com
> Subject: Re: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes
>
> Hi Jain,
>
> I haven't dig much, but I have a quick question below.
>
> Le mardi 06 août 2024 à 17:27 +0530, Aakarsh Jain a écrit :
> > There is a possibility of getting page fault if the overall buffer
> > size is not aligned to 256bytes. Since MFC does read operation only
> > and it won't corrupt the data values even if it reads the extra bytes.
> > Corrected luma and chroma plane sizes for V4L2_PIX_FMT_NV12M and
> > V4L2_PIX_FMT_NV21M pixel format.
>
> Have you re-run v4l2 compliance ? (better be safe then sorry).
>
I ran v4l2-compliance and didn't found any issue wrt to the changes added in this patch.
Please find the v4l2-compliance report attached.
> >
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> > .../media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> > b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> > index 73f7af674c01..03c957221fc4 100644
> > --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> > +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_opr_v6.c
> > @@ -498,8 +498,8 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct
> s5p_mfc_ctx *ctx)
> > case V4L2_PIX_FMT_NV21M:
> > ctx->stride[0] = ALIGN(ctx->img_width,
> S5P_FIMV_NV12MT_HALIGN_V6);
> > ctx->stride[1] = ALIGN(ctx->img_width,
> S5P_FIMV_NV12MT_HALIGN_V6);
> > - ctx->luma_size = calc_plane(ctx->stride[0], ctx->img_height);
> > - ctx->chroma_size = calc_plane(ctx->stride[1], (ctx-
> >img_height / 2));
> > + ctx->luma_size = calc_plane(ctx->img_width, ctx-
> >img_height);
> > + ctx->chroma_size = calc_plane(ctx->img_width, (ctx-
> >img_height >>
> > +1));
>
> These size needs to match the sizes reported through TRY_FMT (and S_FMT)
> sizeimage for each planes. Is this code being call withing try_fmt ? Will these
> value match or will this change cause the value to miss-match ?
>
This code is getting called within try_fmt. In MFC driver we are not returning any sizes in TRY_FMT. We are only validating codec and the pixel format. We are setting luma, chroma and stride size in S_FMT
to inform user space for further buffer allocation. So, this change is not going to cause any mismatch.
> The reason is that correct value is needed for allocating this memory from
> the outside (like using a DMAbuf Heap). Perhaps its all right, let me know.
>
> Nicolas
>
> > break;
> > case V4L2_PIX_FMT_YUV420M:
> > case V4L2_PIX_FMT_YVU420M:
> > @@ -539,9 +539,11 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct
> > s5p_mfc_ctx *ctx) static void s5p_mfc_enc_calc_src_size_v6(struct
> > s5p_mfc_ctx *ctx) {
> > unsigned int mb_width, mb_height;
> > + unsigned int default_size;
> >
> > mb_width = MB_WIDTH(ctx->img_width);
> > mb_height = MB_HEIGHT(ctx->img_height);
> > + default_size = (mb_width * mb_height) * 256;
> >
> > if (IS_MFCV12(ctx->dev)) {
> > switch (ctx->src_fmt->fourcc) {
> > @@ -549,8 +551,8 @@ static void s5p_mfc_enc_calc_src_size_v6(struct
> s5p_mfc_ctx *ctx)
> > case V4L2_PIX_FMT_NV21M:
> > ctx->stride[0] = ALIGN(ctx->img_width,
> S5P_FIMV_NV12M_HALIGN_V6);
> > ctx->stride[1] = ALIGN(ctx->img_width,
> S5P_FIMV_NV12M_HALIGN_V6);
> > - ctx->luma_size = ctx->stride[0] * ALIGN(ctx-
> >img_height, 16);
> > - ctx->chroma_size = ctx->stride[0] * ALIGN(ctx-
> >img_height / 2, 16);
> > + ctx->luma_size = ALIGN(default_size, 256);
> > + ctx->chroma_size = ALIGN(default_size / 2, 256);
> > break;
> > case V4L2_PIX_FMT_YUV420M:
> > case V4L2_PIX_FMT_YVU420M:
[-- Attachment #2: v4l2-compliance.txt --]
[-- Type: text/plain, Size: 11186 bytes --]
# v4l2-compliance -d /dev/video1
v4l2-compliance 1.22.1, 64 bits, 64-bit time_t
[ 1000.848486] vidioc_g_parm:2312: Setting FPS is only possible for the output queue
[ 1000.852609] s5p-mfc 12880000.mfc: Encoding not initialised
[ 1000.857181] s5p-mfc 12880000.mfc: Encoding not initialised
[ 1000.862794] vidioc_g_parm:2312: Setting FPS is only possible for the output queue
[ 1000.870120] vidioc_try_fmt:1440: failed to try output format
Compliance test for s5p-mfc device /dev/video1:
Driver Info:
Driver name : s5p-mfc
Card type : s5p-mfc-enc
Bus info : platform:12880000.mfc
Driver version : 6.10.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Detected Stateful Encoder
Required ioctls:
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video1 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
fail: v4l2-compliance.cpp(736): !ok
test for unlimited opens: FAIL
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
fail: v4l2-test-controls.cpp(473): g_ctrl returned an error (22)
test VIDIOC_G/S_CTRL: FAIL
fail: v4l2-test-controls.cpp(704): g_ext_ctrls returned an error (22)
test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
fail: v4l2-test-controls.cpp(872): subscribe event for control 'User Controls' failed
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 107 Private Controls: 11
Format ioctls:
fail: v4l2-test-formats.cpp(282): node->codec_mask & STATEFUL_ENCODER
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
fail: v4l2-test-formats.cpp(1310): is_stateful_enc && !out->capability
test VIDIOC_G/S_PARM: FAIL
test VIDIOC_G_FBUF: OK (Not Supported)
fail: v4l2-test-formats.cpp(474): !pix_mp.width || !pix_mp.height
test VIDIOC_G_FMT: FAIL
fail: v4l2-test-formats.cpp(474): !pix_mp.width || !pix_mp.height
test VIDIOC_TRY_FMT: FAIL
warn: v4l2-test-formats.cpp(1147): S_FMT cannot handle an invalid pixelformat.
warn: v4l2-test-formats.cpp(1148): This may or may not be a problem. For more information see:
warn: v4l2-test-formats.cpp(1149): http://www.mail-archive.com/linux-media@vger.kernel.org/msg56550.html
fail: v4l2-test-formats.cpp(478): pixelformat 34363248 (H264) for buftype 9 not reported by ENUM_FMT
test VIDIOC_S_FMT: FAIL
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Codec ioctls:
fail: v4l2-test-codecs.cpp(35): node->function[ 1001.213314] s5p_mfc_queue_setup:2426: invalid state: 0
[ 1001.217449] vidioc_reqbufs:1558: error in vb2_reqbufs() for E(D)
[ 1001.223600] vidioc_g_parm:2312: Setting FPS is only possible for the output queue
!= MEDIA_ENT_F_PROC_VIDEO_ENCODER
test VIDIOC_(TRY_)ENCODER_CMD: FAIL
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
fail: v4l2-test-buffers.cpp(607): q.reqbufs(node, 1)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
fail: v4l2-test-buffers.cpp(783): VIDIOC_EXPBUF is supported, but the V4L2_MEMORY_MMAP support is missing or malfunctioning.
fail: v4l2-test-buffers.cpp(784): VIDIOC_EXPBUF is supported, but the V4L2_MEMORY_MMAP support is missing, probably due to earlier failing format tests.
test VIDIOC_EXPBUF: OK (Not Supported)
test Requests: OK (Not Supported)
Total for s5p-mfc device /dev/video1: 45, Succeeded: 34, Failed: 11, Warnings: 3
#
# v4l2-compliance -d /dev/video0
# v4l2-compliance -d /dev/video0
v4l2-compliance 1.22.1, 64 bits, 64-bit time_t
[ 1867.640818] vidioc_g_selection:816: Can not get compose information
[ 1867.644432] vidioc_g_selection:816: Can not get compose information
[ 1867.650265] vidioc_g_fmt:397: Format could not be read
[ 1867.655301] vidioc_g_selection:816: Can not get compose information
[ 1867.661511] vidioc_g_selection:816: Can not get compose information
[ 1867.668211] s5p-mfc 12880000.mfc: Decoding not initialised
[ 1867.673235] s5p-mfc 12880000.mfc: Decoding not initialised
[ 1867.678894] vidioc_g_fmt:397: Format could not be read
[ 1867.683811] vidioc_g_selection:816: Can not get compose information
[ 1867.690068] vidioc_g_selection:816: Can not get compose information
[ 1867.696246] vidioc_g_selection:816: Can not get compose information
[ 1867.702533] vidioc_g_selection:816: Can not get compose information
[ 1867.708731] vidioc_g_selection:816: Can not get compose information
[ 1867.715044] vidioc_g_selection:816: Can not get compose information
[ 1867.721412] vidioc_g_selection:816: Can not get compose information
[ 1867.727660] vidioc_g_selection:816: Can not get compose information
[ 1867.733809] vidioc_g_selection:816: Can not get compose information
[ 1867.740145] vidioc_g_selection:816: Can not get compose information
[ 1867.746172] vidioc_try_fmt:429: Unsupported format for destination.
[ 1867.752579] vidioc_g_selection:816: Can not get compose information
[ 1867.758688] vidioc_g_selection:816: Can not get compose information
[ 1867.764938] vidioc_try_fmt:429: Unsupported format for destination.
[ 1867.771261] vidioc_g_selection:816: Can not get compose information
Compliance test for s5p-mfc device /dev/video0:
Driver Info:
Driver name : s5p-mfc
Card type : s5p-mfc-dec
Bus info : platform:12880000.mfc
Driver version : 6.10.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Detected Stateful Decoder
Required ioctls:
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video0 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
fail: v4l2-compliance.cpp(736): !ok
test for unlimited opens: FAIL
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
fail: v4l2-test-controls.cpp(473): g_ctrl returned an error (22)
test VIDIOC_G/S_CTRL: FAIL
fail: v4l2-test-controls.cpp(704): g_ext_ctrls returned an error (22)
test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
fail: v4l2-test-controls.cpp(872): subscribe event for control 'User Controls' failed
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 7 Private Controls: 2
Format ioctls:
fail: v4l2-test-formats.cpp(263): fmtdesc.description mismatch: was 'Y/UV 4:2:0 (N-C)', expected 'Y/CbCr 4:2:0 (N-C)'
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
fail: v4l2-test-formats.cpp(620): Video Capture Multiplanar cap set, but no Video Capture Multiplanar formats defined
test VIDIOC_G_FMT: FAIL
test VIDIOC_TRY_FMT: OK (Not Supported)
test VIDIOC_S_FMT: OK (Not Supported)
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
fail: v4l2-test-codecs.cpp(104): node->function != MEDIA_ENT_F_PROC_VIDEO_DECODER
test VIDIOC_(TRY_)DECODER_CMD: FAIL
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK (Not Supported)
test Requests: OK (Not Supported)
Total for s5p-mfc device /dev/video0: 45, Succeeded: 38, Failed: 7, Warnings: 0
next prev parent reply other threads:[~2024-08-07 13:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20240806120911epcas5p1b0defc027a7f03ee9bf5f21036d3ae5e@epcas5p1.samsung.com>
2024-08-06 11:57 ` [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes Aakarsh Jain
2024-08-06 14:37 ` Nicolas Dufresne
2024-08-07 11:53 ` Aakarsh Jain [this message]
2024-08-26 9:13 ` Aakarsh Jain
2024-10-11 12:46 ` Hans Verkuil
2025-02-26 10:25 ` Aakarsh Jain/Aakarsh Jain
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='047401dae8c0$65352ec0$2f9f8c40$@samsung.com' \
--to=aakarsh.jain@samsung.com \
--cc=andrzej.hajda@intel.com \
--cc=aswani.reddy@samsung.com \
--cc=gost.dev@samsung.com \
--cc=hverkuil-cisco@xs4all.nl \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mchehab@kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=pankaj.dubey@samsung.com \
/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).