| Message ID | 20230525021219.23638-9-yunfei.dong@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: mediatek: vcodec: Add debugfs file for decode and encode | expand |
Il 25/05/23 04:12, Yunfei Dong ha scritto: > Getting dbgfs help information with command "echo -help > vdec". > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > .../mediatek/vcodec/mtk_vcodec_dbgfs.c | 24 ++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > index 237d0dc8a1fc..2372fc449b45 100644 > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf > *used += curr_len; > } > > +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total) > +{ > + int curr_len; > + > + curr_len = snprintf(buf + *used, total - *used, > + "help: (1: echo -'info' > vdec 2: cat vdec)\n"); > + *used += curr_len; > + > + curr_len = snprintf(buf + *used, total - *used, > + "\t-picinfo: get resolution\n"); > + *used += curr_len; > + > + curr_len = snprintf(buf + *used, total - *used, > + "\t-format: get output & capture queue format\n"); > + *used += curr_len; > +} > + > static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf, > size_t count, loff_t *ppos) > { > @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf, > if (!buf) > return -ENOMEM; > > + if (strstr(dbgfs->dbgfs_buf, "-help")) { I would print the help strings in two conditions: 1. -help 2. (nothing) ...so that if you don't echo anything to vdec (no params), you get the help text. Otherwise, you would have to know that "-help" is a parameter that gives you help text in the first place. As for this commit "as is", it works as intended and it is useful to retrieve the help text; you can either send a followup commit that extends the help to the corner case that I've explained, or send a v6 adding that to this same commit. I would prefer to see a v6 -- BUT -- since this series was sent a long time ago, you will get my R-b and I will leave the final choice to Hans. Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On Tue, May 30, 2023 at 6:06 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 25/05/23 04:12, Yunfei Dong ha scritto: > > Getting dbgfs help information with command "echo -help > vdec". > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > --- > > .../mediatek/vcodec/mtk_vcodec_dbgfs.c | 24 ++++++++++++++++++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > > index 237d0dc8a1fc..2372fc449b45 100644 > > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > > @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf > > *used += curr_len; > > } > > > > +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total) > > +{ > > + int curr_len; > > + > > + curr_len = snprintf(buf + *used, total - *used, > > + "help: (1: echo -'info' > vdec 2: cat vdec)\n"); > > + *used += curr_len; > > + > > + curr_len = snprintf(buf + *used, total - *used, > > + "\t-picinfo: get resolution\n"); > > + *used += curr_len; > > + > > + curr_len = snprintf(buf + *used, total - *used, > > + "\t-format: get output & capture queue format\n"); > > + *used += curr_len; > > +} > > + > > static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf, > > size_t count, loff_t *ppos) > > { > > @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf, > > if (!buf) > > return -ENOMEM; > > > > + if (strstr(dbgfs->dbgfs_buf, "-help")) { > > I would print the help strings in two conditions: > 1. -help > 2. (nothing) > > ...so that if you don't echo anything to vdec (no params), you get the help text. > Otherwise, you would have to know that "-help" is a parameter that gives you help > text in the first place. > > As for this commit "as is", it works as intended and it is useful to retrieve > the help text; you can either send a followup commit that extends the help to > the corner case that I've explained, or send a v6 adding that to this same commit. > > I would prefer to see a v6 -- BUT -- since this series was sent a long time ago, > you will get my R-b and I will leave the final choice to Hans. > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> The help file could just exist as a separate file. You can then use cleaner interfaces for it. ChenYu
On 5/30/23 12:06, AngeloGioacchino Del Regno wrote: > Il 25/05/23 04:12, Yunfei Dong ha scritto: >> Getting dbgfs help information with command "echo -help > vdec". >> >> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> >> --- >> .../mediatek/vcodec/mtk_vcodec_dbgfs.c | 24 ++++++++++++++++++- >> 1 file changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c >> index 237d0dc8a1fc..2372fc449b45 100644 >> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c >> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c >> @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf >> *used += curr_len; >> } >> >> +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total) >> +{ >> + int curr_len; >> + >> + curr_len = snprintf(buf + *used, total - *used, >> + "help: (1: echo -'info' > vdec 2: cat vdec)\n"); >> + *used += curr_len; >> + >> + curr_len = snprintf(buf + *used, total - *used, >> + "\t-picinfo: get resolution\n"); >> + *used += curr_len; >> + >> + curr_len = snprintf(buf + *used, total - *used, >> + "\t-format: get output & capture queue format\n"); >> + *used += curr_len; >> +} >> + >> static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf, >> size_t count, loff_t *ppos) >> { >> @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf, >> if (!buf) >> return -ENOMEM; >> >> + if (strstr(dbgfs->dbgfs_buf, "-help")) { > > I would print the help strings in two conditions: > 1. -help > 2. (nothing) > > ...so that if you don't echo anything to vdec (no params), you get the help text. > Otherwise, you would have to know that "-help" is a parameter that gives you help > text in the first place. > > As for this commit "as is", it works as intended and it is useful to retrieve > the help text; you can either send a followup commit that extends the help to > the corner case that I've explained, or send a v6 adding that to this same commit. > > I would prefer to see a v6 -- BUT -- since this series was sent a long time ago, > you will get my R-b and I will leave the final choice to Hans. > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > I prefer a v6, rebased on top of the media_stage tree. Regards, Hans
Hi Hans, On Tue, 2023-05-30 at 12:33 +0200, Hans Verkuil wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 5/30/23 12:06, AngeloGioacchino Del Regno wrote: > > Il 25/05/23 04:12, Yunfei Dong ha scritto: > >> Getting dbgfs help information with command "echo -help > vdec". > >> > >> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > >> --- > >> .../mediatek/vcodec/mtk_vcodec_dbgfs.c | 24 > ++++++++++++++++++- > >> 1 file changed, 23 insertions(+), 1 deletion(-) > >> > >> diff --git > a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > >> index 237d0dc8a1fc..2372fc449b45 100644 > >> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > >> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c > >> @@ -52,6 +52,23 @@ static void > mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf > >> *used += curr_len; > >> } > >> > >> +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int > total) > >> +{ > >> +int curr_len; > >> + > >> +curr_len = snprintf(buf + *used, total - *used, > >> + "help: (1: echo -'info' > vdec 2: cat vdec)\n"); > >> +*used += curr_len; > >> + > >> +curr_len = snprintf(buf + *used, total - *used, > >> + "\t-picinfo: get resolution\n"); > >> +*used += curr_len; > >> + > >> +curr_len = snprintf(buf + *used, total - *used, > >> + "\t-format: get output & capture queue format\n"); > >> +*used += curr_len; > >> +} > >> + > >> static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const > char __user *ubuf, > >> size_t count, loff_t *ppos) > >> { > >> @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file > *filp, char __user *ubuf, > >> if (!buf) > >> return -ENOMEM; > >> > >> +if (strstr(dbgfs->dbgfs_buf, "-help")) { > > > > I would print the help strings in two conditions: > > 1. -help > > 2. (nothing) > > > > ...so that if you don't echo anything to vdec (no params), you get > the help text. > > Otherwise, you would have to know that "-help" is a parameter that > gives you help > > text in the first place. > > > > As for this commit "as is", it works as intended and it is useful > to retrieve > > the help text; you can either send a followup commit that extends > the help to > > the corner case that I've explained, or send a v6 adding that to > this same commit. > > > > I would prefer to see a v6 -- BUT -- since this series was sent a > long time ago, > > you will get my R-b and I will leave the final choice to Hans. > > > > Reviewed-by: AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> > > > > > > I prefer a v6, rebased on top of the media_stage tree. > > Regards, > > Hans I already have sent patch v6 to review. Best Regards, Yunfei Dong
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c index 237d0dc8a1fc..2372fc449b45 100644 --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf *used += curr_len; } +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total) +{ + int curr_len; + + curr_len = snprintf(buf + *used, total - *used, + "help: (1: echo -'info' > vdec 2: cat vdec)\n"); + *used += curr_len; + + curr_len = snprintf(buf + *used, total - *used, + "\t-picinfo: get resolution\n"); + *used += curr_len; + + curr_len = snprintf(buf + *used, total - *used, + "\t-format: get output & capture queue format\n"); + *used += curr_len; +} + static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf, size_t count, loff_t *ppos) { @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf, if (!buf) return -ENOMEM; + if (strstr(dbgfs->dbgfs_buf, "-help")) { + mtk_vdec_dbgfs_get_help(buf, &used_len, total_len); + goto read_buffer; + } + if (strstr(dbgfs->dbgfs_buf, "-picinfo")) dbgfs_index[MTK_VDEC_DBGFS_PICINFO] = true; @@ -110,7 +132,7 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf, mtk_vdec_dbgfs_get_format_type(ctx, buf, &used_len, total_len); } mutex_unlock(&dbgfs->dbgfs_lock); - +read_buffer: ret = simple_read_from_buffer(ubuf, count, ppos, buf, used_len); kfree(buf); return ret;
Getting dbgfs help information with command "echo -help > vdec". Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../mediatek/vcodec/mtk_vcodec_dbgfs.c | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)