Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230525021219.23638-1-yunfei.dong@mediatek.com>
In-Reply-To: <20230525021219.23638-1-yunfei.dong@mediatek.com>
From:   Chen-Yu Tsai <wenst@chromium.org>
Date:   Tue, 30 May 2023 18:13:34 +0800
Message-ID: <CAGXv+5HLXiD10UV+Z-CTDrHQC16RFMYi6hsiGs3B0m4kgXkCZA@mail.gmail.com>
Subject: Re: [PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for
 decode and encode
To:     Yunfei Dong <yunfei.dong@mediatek.com>
Cc:     Nicolas Dufresne <nicolas.dufresne@collabora.com>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Benjamin Gaignard <benjamin.gaignard@collabora.com>,
        =?UTF-8?B?TsOtY29sYXMgRiAuIFIgLiBBIC4gUHJhZG8=?= 
        <nfraprado@collabora.com>, Nathan Hebert <nhebert@chromium.org>,
        Hsin-Yi Wang <hsinyi@chromium.org>,
        Fritz Koenig <frkoenig@chromium.org>,
        Daniel Vetter <daniel@ffwll.ch>,
        Steve Cho <stevecho@chromium.org>, linux-media@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org,
        Project_Global_Chrome_Upstream_Group@mediatek.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, May 25, 2023 at 10:12=E2=80=AFAM Yunfei Dong <yunfei.dong@mediatek.=
com> wrote:
>
> Need to change kernel driver to open decode and encode debug log at curre=
nt period,
> it's very unreasonable. Adding debugfs common interface to support decode=
 and encode,
> using echo command to control debug log level and getting useful informat=
ion for each
> instance.
>
> patch 1 add dbgfs common interface.
> patch 2~5 support decode.
> patch 6~7 support encode
> patch 8 add help function

I find the interface kind of weird. A lot of debugfs usage in other places
just dumps out the current state. Here you are writing to it to ask it to
do a snapshot, and then you read it later. This ends up requiring manual
management of a buffer, instead of using seq_file, which is commonly used
for virtual files.

ChenYu

> ---
> changed with v4:
> - rebase to the top of media stage header.
>
> changed with v3:
> - add help function for patch 8
> - remove append '\0' and enlarge buffer size for patch 4
>
> changed with v2:
> - using pr_debug and dev_dbg instead of pr_info for patch 2.
> - fix word fail: informatiaoin -> information for patch 3.
> - used to print each instance format information for patch 5.
>
> changed with v1:
> - add new patch 4 and 5.
> - using cmd 'cat vdec' to show debug information instead of pr_info direc=
tly.
> ---
> Yunfei Dong (8):
>   media: mediatek: vcodec: Add debugfs interface to get debug
>     information
>   media: mediatek: vcodec: Add debug params to control different log
>     level
>   media: mediatek: vcodec: Add a debugfs file to get different useful
>     information
>   media: mediatek: vcodec: Get each context resolution information
>   media: mediatek: vcodec: Get each instance format type
>   media: mediatek: vcodec: Change dbgfs interface to support encode
>   media: mediatek: vcodec: Add encode to support dbgfs
>   media: mediatek: vcodec: Add dbgfs help function
>
>  .../media/platform/mediatek/vcodec/Makefile   |   6 +
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 216 ++++++++++++++++++
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  72 ++++++
>  .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |   4 +
>  .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   4 +
>  .../mediatek/vcodec/mtk_vcodec_enc_drv.c      |   2 +
>  .../mediatek/vcodec/mtk_vcodec_util.c         |   8 +
>  .../mediatek/vcodec/mtk_vcodec_util.h         |  26 ++-
>  8 files changed, 335 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbg=
fs.c
>  create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbg=
fs.h
>
> --
> 2.25.1
>