Messages in this thread | | | From | jackson.lee <> | Subject | RE: [PATCH v2 0/4] wave5 codec driver | Date | Wed, 13 Mar 2024 06:16:22 +0000 |
| |
Thanks for your comment
Jackson
> -----Original Message----- > From: Sebastian Fricke <sebastian.fricke@collabora.com> > Sent: Tuesday, March 12, 2024 12:25 AM > 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, > > 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.cioca > >> > lte > >> > 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 > >> >>
| |