Messages in this thread | | | Date | Mon, 11 Mar 2024 16:24:46 +0100 | From | Sebastian Fricke <> | Subject | Re: [PATCH v2 0/4] wave5 codec driver |
| |
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@collabora.com> >> Sent: Monday, March 11, 2024 6:39 PM >> To: jackson.lee <jackson.lee@chipsnmedia.com> >> Cc: mchehab@kernel.org; nicolas@ndufresne.ca; linux-kernel@vger.kernel.org; >> hverkuil@xs4all.nl; Nas Chung <nas.chung@chipsnmedia.com>; lafley.kim >> <lafley.kim@chipsnmedia.com> >> 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@vger.kernel.org >> >> Here what I wrote on V0: >> > you forgot to send this mail series to the media mailing list >> > (linux-media@vger.kernel.org), 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@chipsnmedia.com> (maintainer:WAVE5 VPU >> > CODEC DRIVER) Jackson Lee <jackson.lee@chipsnmedia.com> >> > (maintainer:WAVE5 VPU CODEC DRIVER) Mauro Carvalho Chehab >> > <mchehab@kernel.org> (maintainer:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)) >> > linux-media@vger.kernel.org (open list:WAVE5 VPU CODEC DRIVER) >> > linux-kernel@vger.kernel.org (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@collabora.com/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@chipsnmedia.com> >> >> >> >>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 >> >>
| |