RE: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes

From: Aakarsh Jain
Date: Wed Aug 07 2024 - 09:02:06 EST

Next message: Thomas Gleixner: "Re: [PATCH 2/2] regmap: Allow setting IRQ domain name suffix"
Previous message: Dev Jain: "Re: Race condition observed between page migration and page fault handling on arm64 machines"
In reply to: Nicolas Dufresne: "Re: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Nocolas,

> -----Original Message-----
> From: Nicolas Dufresne <nicolas@xxxxxxxxxxxx>
> Sent: 06 August 2024 20:08
> To: Aakarsh Jain <aakarsh.jain@xxxxxxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: m.szyprowski@xxxxxxxxxxx; andrzej.hajda@xxxxxxxxx;
> mchehab@xxxxxxxxxx; hverkuil-cisco@xxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx;
> gost.dev@xxxxxxxxxxx; aswani.reddy@xxxxxxxxxxx;
> pankaj.dubey@xxxxxxxxxxx
> 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@xxxxxxxxxxx>
> > ---
> > .../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:

# 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@xxxxxxxxxxxxxxx/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 message: Thomas Gleixner: "Re: [PATCH 2/2] regmap: Allow setting IRQ domain name suffix"
Previous message: Dev Jain: "Re: Race condition observed between page migration and page fault handling on arm64 machines"
In reply to: Nicolas Dufresne: "Re: [PATCH] media: s5p-mfc: Corrected NV12M/NV21M plane-sizes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]