linux-kernel - Re: [PATCH v2,0/4] media: mediatek: vcodec: fix ctrl request complete fail

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <4DFF7ED9-CD67-415E-965D-D83337CBE8A7@collabora.com>
Date: Fri, 15 Mar 2024 15:24:44 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>
Cc: "Nícolas F . R . A . Prado" <nfraprado@...labora.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Hans Verkuil <hverkuil-cisco@...all.nl>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 Nathan Hebert <nhebert@...omium.org>,
 Sebastian Fricke <sebastian.fricke@...labora.com>,
 Hsin-Yi Wang <hsinyi@...omium.org>,
 Fritz Koenig <frkoenig@...omium.org>,
 Daniel Vetter <daniel@...ll.ch>,
 Steve Cho <stevecho@...omium.org>,
 linux-media@...r.kernel.org,
 devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org,
 Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v2,0/4] media: mediatek: vcodec: fix ctrl request complete
 fail

Hi Yunfei!

I say this very respectfully, but I believe that you must improve the wording of some of your commit messages. It is hard to understand your changes otherwise. More importantly, it is hard to understand why they are needed or what exactly they fix.

This series has some checkpatch errors: 

total: 53 errors, 0 warnings, 0 checks, 1047 lines checked

Did you test this with Fluster? We should really make sure that this does not regress any of the tests there.

See a few comments from me in the thread

— Daniel

> On 14 Mar 2024, at 08:44, Yunfei Dong <yunfei.dong@...iatek.com> wrote:
> 
> Moving v4l2_ctrl_request_complete to before of function
> v4l2_m2m_buf_done to make sure the status of request correctly.
> 
> Replace v4l2_m2m_next_src_buf with v4l2_m2m_src_buf_remove to
> make sure the src buffer won't be removed for some unknown
> reason leading to buffer done error.
> 
> Patch 1 setting request complete before buffer done
> Patch 3 flush decoder before remove all source buffer
> Patch 2 change flush decode from capture to output when stream off
> Patch 4 replace v4l2_m2m_next_src_buf with v4l2_m2m_src_buf_remove
> ---
> compared with v1:
> - add patch 2/3/4 to fix timing issue.
> ---
> Yunfei Dong (4):
>  media: mediatek: vcodec: setting request complete before buffer done
>  media: mediatek: vcodec: change flush decode from capture to output
>    when stream off
>  media: mediatek: vcodec: flush decoder before remove all source buffer
>  media: mediatek: vcodec: replace v4l2_m2m_next_src_buf with
>    v4l2_m2m_src_buf_remove
> 
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c  | 52 +++++++++----------
> .../vcodec/decoder/mtk_vcodec_dec_drv.h       |  3 +-
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 28 +++++++---
> .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 28 +++++-----
> .../decoder/vdec/vdec_h264_req_multi_if.c     |  3 +-
> .../decoder/vdec/vdec_hevc_req_multi_if.c     |  3 +-
> .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 37 ++++++-------
> .../mediatek/vcodec/decoder/vdec_msg_queue.h  |  2 +
> 8 files changed, 84 insertions(+), 72 deletions(-)
> 
> -- 
> 2.18.0
> 
>