Date:   Sat, 29 Jul 2023 02:54:08 +0000
From:   Yunfei Dong (董云飞) 
        <Yunfei.Dong@...iatek.com>
To:     "nhebert@...omium.org" <nhebert@...omium.org>,
        "benjamin.gaignard@...labora.com" <benjamin.gaignard@...labora.com>,
        "nfraprado@...labora.com" <nfraprado@...labora.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "nicolas.dufresne@...labora.com" <nicolas.dufresne@...labora.com>,
        "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "frkoenig@...omium.org" <frkoenig@...omium.org>,
        "stevecho@...omium.org" <stevecho@...omium.org>,
        "wenst@...omium.org" <wenst@...omium.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "hsinyi@...omium.org" <hsinyi@...omium.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v7,0/11] media: mediatek: vcodec: separate encoder and
 decoder

Hi Hans,

Thanks for your help and suggestion.

On Fri, 2023-07-28 at 11:25 +0200, Hans Verkuil wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 25/07/2023 11:58, Hans Verkuil wrote:
> > On 22/07/2023 09:42, Yunfei Dong wrote:
> >> From: Yunfei Dong <yunfei.dong@...iatek.corp-partner.google.com>
> >>
> >> With the driver more and more complex, encoder and decoder need to
> add more parameter
> >> in shared struct 'mtk_vcodec_ctx' and 'mtk_vcodec_dev'. Encoder
> use about 40% and
> >> decoder use 60% parameter. Need to allocate extra unused memory
> when encoder and decoder
> >> working.
> >>
> >> Separate encoder and decoder in different folder and use
> independent data struct.
> >>
> >> patch 1 remove unused parameter.
> >> patch 2~3 align fw and interrupt related interface.
> >> patch 4~6 remove the dependency of debug log
> >> patch 7~8 separate mtk_vcodec_ctx and mtk_vcodec_dev
> >> patch 9 fix unreasonable parameter
> >> patch 10 removed unused header files
> >> patch 11 separate encoder and decoder in different folder
> >> ---
> >> Changed from v6:
> >> - rebase to: 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=fo-v6.6g.
> >> Changed from v5:
> >> - fix some words error for patch 3/6/11.
> >> - rename mtk_vcodec_comm_drv.h to mtk_vcodec_cmn_drv.h for patch
> 7.
> >> Changed from v4:
> >> - add one parameter to record register base for reg_base for patch
> 3.
> >> - add debug string for non ctx log for patch 6.
> >> - change the comment of struct mtk_vcodec_dec_ctx and struct
> mtk_vcodec_enc_ctx for patch 7.
> >> - prefer to use struct mtk_vcodec_dec_dev an current period, will
> re-construct in the future for patch 8.
> >> Changed from v3:
> >> - re-write commit message for patch 3.
> >> Changed from v2:
> >> - This patch main changed:
> >>   1: add different macro mtk_dec_debug and mtk_enc_debug calling
> common
> >>      macro mtk_vcodec_debug in order to use dev_dbg instead of
> pr_debug.
> >>   2: add different macro mtk_v4l2_venc_dbg and mtk_v4l2_vdec_dbg
> calling common
> >>      macro  in order to use dev_dbg instead of pr_debug.
> >> Changed from v1:
> >> - Change pr_dbg to dev_dbg for mtk_v4l2_level and mtk_vcodec_dbg
> for patch 4.
> >> - Change pr_err to dev_err for mtk_v4l2_err and mtk_vcodec_err for
> patch 5.
> >> - Fix unreasonable parameter for patch 8.
> >> ---
> >> Yunfei Dong (11):
> >>   media: mediatek: vcodec: remove unused parameter
> >>   media: mediatek: vcodec: align fw interface
> >>   media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev'
> for
> >>     shared interface
> >>   media: mediatek: vcodec: Removing useless debug log
> >>   media: mediatek: vcodec: remove the dependency of vcodec debug
> log
> >>   media: mediatek: vcodec: replace pr_* with dev_* for v4l2 debug
> >>     message
> >>   media: mediatek: vcodec: separate struct 'mtk_vcodec_ctx'
> >>   media: mediatek: vcodec: separate struct mtk_vcodec_dev
> >>   media: mediatek: vcodec: fix unreasonable parameter definition
> and
> >>     style
> >>   media: mediatek: vcodec: remove unused include header
> >>   media: mediatek: vcodec: separate decoder and encoder
> > 
> > Besides the missing argument in patch 6/11 I also get this compiler
> warning:
> > 
> > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c: In
> function 'vpu_enc_ipi_handler':
> > drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c:64:31:
> warning: 'vpu' may be used uninitialized [-Wmaybe-uninitialized]
> >    64 |         struct venc_vpu_inst *vpu;
> >       |                               ^~~
> > 
> > and this smatch error:
> > 
> >
> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:1
> 38 mtk_vcodec_get_reg_bases() error: buffer overflow
> 'mtk_dec_reg_names' 11 <= 11
> > 
> > However, I believe that was introduced by Nicolas' patch series.
> > 
> > I'll try to pinpoint the precise patch.
> 
> That smatch error is now found and fixed in the staging tree.
> 
> Can you post a v8 fixing the other issues?
> 
> Thanks!

The build fail for patch 06 and smatch fail will be fixed in next
patch.

Best Regards,
Yunfei Dong

> 
> Regards,
> 
> Hans
> 
> > 
> > Regards,
> > 
> > Hans
> 

Powered by blists - more mailing lists