Re: [PATCH v2 0/4] wave5 codec driver

From: Sebastian Fricke
Date: Mon Mar 11 2024 - 11:32:33 EST


Hey Jackson,

On 11.03.2024 11:01, jackson.lee wrote:
Hello Sebastian

Thanks for your review.

just to clarify, what I meant with the title is not that it needs to be a
mini summary, as that is what the body of the cover letter is for. But
it should show clearly what to expect in the series, like do you:
- introduce a new driver?
- fix bugs on an existing driver?
- add features to an existing driver?
- refactor code?
- add support for new hardware?

That makes it easier for a maintainer for example to quickly judge
priority of a patch set and makes it easy to distinguish the different
patch series in the mail reader.

So if you have a single change you can just describe that fully (if you
even need a cover letter in that case), otherwise try to basically
describe on a very broad level what you have done. For these patches a
title like: "additional features and formats for the Wave5 Codec" might
be fitting, but no need to RESEND for that again, just keep that in mind
for the future.

Greetings,
Sebastian



-----Original Message-----
From: Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx>
Sent: Monday, March 11, 2024 6:39 PM
To: jackson.lee <jackson.lee@xxxxxxxxxxxxxxx>
Cc: mchehab@xxxxxxxxxx; nicolas@xxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
hverkuil@xxxxxxxxx; Nas Chung <nas.chung@xxxxxxxxxxxxxxx>; lafley.kim
<lafley.kim@xxxxxxxxxxxxxxx>
Subject: Re: [PATCH v2 0/4] wave5 codec driver

Hey Jackson,

Ah and also you have to resend the series because you didn't send to:
linux-media@xxxxxxxxxxxxxxx

Here what I wrote on V0:
> you forgot to send this mail series to the media mailing list
> (linux-media@xxxxxxxxxxxxxxx), which makes it a bit hard for people to
> review the patches.
>
> Please use either the `./scripts/get_maintainer.p` Script in the
> kernel root directory:
> ```
> ❯ ./scripts/get_maintainer.pl
> ~/Mail/patches/_PATCH_v0_1-5_wave5_Support_yuv422_input_format_for_enc
> oder.patch Nas Chung <nas.chung@xxxxxxxxxxxxxxx> (maintainer:WAVE5 VPU
> CODEC DRIVER) Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
> (maintainer:WAVE5 VPU CODEC DRIVER) Mauro Carvalho Chehab
> <mchehab@xxxxxxxxxx> (maintainer:MEDIA INPUT INFRASTRUCTURE (V4L/DVB))
> linux-media@xxxxxxxxxxxxxxx (open list:WAVE5 VPU CODEC DRIVER)
> linux-kernel@xxxxxxxxxxxxxxx (open list) ```
>
> Or use a tool like b4 to generate the list of receivers:
> https://b4.docs.kernel.org/en/latest/contributor/prep.html#prepare-the
> -list-of-recipients
>
> After that you can then add any additional receivers you like, but
> this makes sure that the most essential mails are present.
> also just noting in case you want to just resend the series without
> doing any actual changes to the patches, please add a RESEND tag to
> the mails, like here:
> https://lore.kernel.org/all/20240126192128.1210579-1-cristian.ciocalte
> a@xxxxxxxxxxxxx/T/#t
>
> e.g. [RESEND PATCH v0 0/5]
>
> This can be changed for example like this:
> `git format-patch --subject-prefix="RESEND PATCH" ...`

Greetings,
Sebastian

On 11.03.2024 10:29, Sebastian Fricke wrote:
>Hey Jackson,
>
>just as a quick info, I feel like the title of the cover letter is a
>bit confusing, as it sounds like it contains the source code for the
>wave5 codec driver, which we already have. Usually the title of the
>cover letter should indicate to the reader what to expect in the
>patch-set (which can also be a really nice indication for when you try
>to do too much in a single patch-set, because then it will be hard to
>make a concise title).
>
>Greetings,
>Sebastian
>
>On 11.03.2024 13:24, jackson.lee wrote:
>>From: "Jackson.lee" <jackson.lee@xxxxxxxxxxxxxxx>
>>
>>The wave5 codec driver is a stateful encoder/decoder.
>>The following patches is for supporting yuv422 inpuy format,
>>supporting runtime suspend/resume feature and extra things.
>>
>>v4l2-compliance results:
>>========================
>>
>>v4l2-compliance 1.24.1, 64 bits, 64-bit time_t
>>
>>Buffer ioctls:
>> warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not
supported
>> warn: v4l2-test-buffers.cpp(693): VIDIOC_CREATE_BUFS not
supported
>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>> test VIDIOC_EXPBUF: OK
>> test Requests: OK (Not Supported)
>>
>>Total for wave5-dec device /dev/video0: 45, Succeeded: 45, Failed: 0,
>>Warnings: 2 Total for wave5-enc device /dev/video1: 45, Succeeded: 45,
>>Failed: 0, Warnings: 0
>>
>>Fluster test results:
>>=====================
>>
>>Running test suite JCT-VC-HEVC_V1 with decoder GStreamer-H.265-V4L2-
Gst1.0 Using 1 parallel job(s)
>>Ran 132/147 tests successfully in 94.782 secs
>>
>>(1 test fails because of not supporting to parse multi frames, 1 test
>>fails because of a missing frame and slight corruption, 2 tests fail
>>because of sizes which are incompatible with the IP, 11 tests fail
>>because of unsupported 10 bit format)
>>
>>Running test suite JVT-AVC_V1 with decoder GStreamer-H.264-V4L2-Gst1.0
Using 1 parallel job(s)
>>Ran 77/135 tests successfully in 37.364 secs
>>
>>(58 fail because the hardware is unable to decode MBAFF / FMO / Field
>>/ Extended profile streams.)
>>
>>
>>Chnage since v1:
>>=================
>>
>>* For [PATCH v2 0/4] media: chips-media: wave5: Support SPS/PPS
>>generation for each IDR
>>- define a macro for register addresses
>>
>>* For [PATCH v2 1/4] media: chips-media: wave5: Support runtime
>>suspend/resume
>>- add auto suspend/resume
>>
>>* For [PATCH v2 2/4] media: chips-media: wave5: Use helpers to
>>calculate bytesperline and sizeimage
>>- use helper functions to calculate bytesperline and sizeimage
>>
>>* For [PATCH v2 3/4] media: chips-media: wave5: Support YUV422 raw
>>pixel-formats on the encoder
>>- remove unnecessary codes
>>
>>Change since v0:
>>=================
>>The DEFAULT_SRC_SIZE macro was defined using multiple lines, To make a
>>simple define, tab and multiple lines has been removed, The macro is
>>defined using one line.
>>
>>
>>Jackson.lee (4):
>> media: chips-media: wave5: Support SPS/PPS generation for each IDR
>> media: chips-media: wave5: Support runtime suspend/resume
>> media: chips-media: wave5: Use helpers to calculate bytesperline and
>> sizeimage.
>> media: chips-media: wave5: Support YUV422 raw pixel-formats on the
>> encoder.
>>
>>.../platform/chips-media/wave5/wave5-helper.c | 24 ++
>>.../platform/chips-media/wave5/wave5-helper.h | 4 +
>>.../platform/chips-media/wave5/wave5-hw.c | 23 +-
>>.../chips-media/wave5/wave5-vpu-dec.c | 261 +++++-------------
>>.../chips-media/wave5/wave5-vpu-enc.c | 260 +++++++++--------
>>.../platform/chips-media/wave5/wave5-vpu.c | 43 +++
>>.../platform/chips-media/wave5/wave5-vpu.h | 4 -
>>.../platform/chips-media/wave5/wave5-vpuapi.c | 14 +-
>>.../platform/chips-media/wave5/wave5-vpuapi.h | 1 +
>>.../chips-media/wave5/wave5-vpuconfig.h | 25 +-
>>.../media/platform/chips-media/wave5/wave5.h | 3 +
>>11 files changed, 329 insertions(+), 333 deletions(-)
>>
>>--
>>2.43.0
>>