Re: [PATCH v7 05/10] media: mediatek: vcodec: Add Decoder profile & level Initialization

["Kyrie Wu (吴晗)" <Kyrie.Wu@mediatek.com>]

On Wed, 2026-01-28 at 15:41 +0100, AngeloGioacchino Del Regno wrote:
> Il 28/01/26 14:39, Nicolas Dufresne ha scritto:
> > Hi,
> > 
> > Le mercredi 28 janvier 2026 à 10:53 +0100, AngeloGioacchino Del
> > Regno a écrit :
> > > Il 28/01/26 08:45, Kyrie Wu (吴晗) ha scritto:
> > > > On Tue, 2026-01-27 at 13:07 +0100, AngeloGioacchino Del Regno
> > > > wrote:
> > > > > Il 27/01/26 03:42, Kyrie Wu ha scritto:
> > > > > > This commit initializes codec profile & level for VDEC. It
> > > > > > sets
> > > > > > default values for H264, H265, and VP9 codecs across
> > > > > > multiple
> > > > > > chipset configurations.
> > > > > > 
> > > > > 
> > > > > The previous patch "Refactor decoder profile and level
> > > > > handling" will
> > > > > break the
> > > > > driver if this patch is not also applied at the same time.
> > > > > 
> > > > > The change looks good, but you should squash 05/10 in 04/10,
> > > > > and
> > > > > assign the params
> > > > > that you're adding here along with the refactoring, so that
> > > > > you get
> > > > > one single
> > > > > patch that, if applied, doesn't break anything as it doesn't
> > > > > depend
> > > > > on additional
> > > > > (future, as this is number 5) patch.
> > > > > 
> > > > > Please squash
> > > > > 
> > > > > Cheers,
> > > > > Angelo
> > > > 
> > > > Dear Angelo,
> > > > 
> > > > The 04/10 and 05/10 were designed in one patch in v6,
> > > > but Nicolas thought that refactor patch and configuration
> > > > were two different changes, should separate them in this
> > > > comments:
> > > > 
https://urldefense.com/v3/__https://patchwork.linuxtv.org/project/linux-media/patch/20251202074038.3173-5-kyrie.wu@mediatek.com/__;!!CTRNKA9wMg0ARbw!mpwkB-IxdCmuVuRkqAbB30jxPDQQv95G68LXeKB4Fm-6AWCmiMWRWHWihquINz7K5ePYOxDuyimg4mlljsxwONT8bXTH3kY$
> > > >  
> > > 
> > > Even though in this case I disagree with Nicolas, I'm the
> > > mediatek maintainer,
> > > not a linux-media maintainer... so... let's go with whatever the
> > > media
> > > maintainers think it's best.
> > 
> > No need to disagree, I was not very clear in my reply indeed. What
> > I want to see
> > is the added MT8189 configuration in isolation. So when I said two
> > patches, the
> > first one should reorganize the code for existing platform, without
> > breaking it,
> > and the second should add MT8189 support. I will have a read at
> > these two patch
> > to understand what is being done wrong, and can provide further
> > feedback later.
> > 
> > The commit description in v6 was only saying that it was adding a
> > configuration
> > for MT8189.
> > 
> 
> Re-reading what I wrote - my words could be misunderstood as well, so
> - just in
> case anyone did: sorry, that wasn't meant as any form of attack and
> was purely
> about sharing an opinion in a friendlier way than what appears from
> my previous
> words. Eh, writing emails fast .... :-P
> 
> Btw.
> 
> Happy to see that we're on the same page.
> 
> Cheers,
> Angelo
> 
> > Nicolas

Dear Nicolas and Angelo,

Thanks for both of you to discuss the differences and reach a
consensus. 

I will modify the patch in the next version based on the latest
suggestions.

Thank you both sincerely once again.


Regards,
Kyrie.
> > 
> > > 
> > > > 
> > > > In my mind, refactor profile and level setting and assign
> > > > former ICs' parameters could merge into same patch. The
> > > > configuration of MT8189 should split to another one, because
> > > > it is a new setting.
> > > > 
> > > > Do you agree with my opinion? I look forward to your further
> > > > reply.
> > > > 
> > > 
> > > I agree with your opinion, but if linux-media maintainers want
> > > those two
> > > patches separated as you just did, I will raise my hands, so...
> > > 
> > > Reviewed-by: AngeloGioacchino Del Regno
> > > <angelogioacchino.delregno@collabora.com
> > > 
> > > ...because when the two patches are applied at the same time,
> > > there's no
> > > breakage. But I want to still repeat my stance on this: single
> > > patches
> > > should never depend on subsequent patches to avoid regressions
> > > (and I know
> > > you agree as your previous version was compliant with that
> > > stance).
> > > 
> > > Cheers,
> > > Angelo
> > > 
> > > > Thanks.
> > > > 
> > > > Regards,
> > > > Kyrie.
> > > > 
> > > > 
> > > > > 
> > > > > > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> > > > > > ---
> > > > > >     .../vcodec/decoder/mtk_vcodec_dec_stateful.c  | 12 +++
> > > > > >     .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 84
> > > > > > +++++++++++++++++++
> > > > > >     2 files changed, 96 insertions(+)
> > > > > > 
> > > > > > diff --git
> > > > > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teful.c
> > > > > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teful.c
> > > > > > index 8ddb61670dc6..a47906b9d717 100644
> > > > > > ---
> > > > > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teful.c
> > > > > > +++
> > > > > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teful.c
> > > > > > @@ -619,4 +619,16 @@ const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_8173_pdata = {
> > > > > >     	.is_subdev_supported = false,
> > > > > >     	.hw_arch = MTK_VDEC_PURE_SINGLE_CORE,
> > > > > >     	.chip_name = 8173,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_4_1,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_1,
> > > > > > +	},
> > > > > >     };
> > > > > > diff --git
> > > > > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teless.c
> > > > > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teless.c
> > > > > > index a1f419202a24..b571c4ed3f79 100644
> > > > > > ---
> > > > > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teless.c
> > > > > > +++
> > > > > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec
> > > > > > _dec_sta
> > > > > > teless.c
> > > > > > @@ -830,6 +830,18 @@ const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_8183_pdata = {
> > > > > >     	.is_subdev_supported = false,
> > > > > >     	.hw_arch = MTK_VDEC_PURE_SINGLE_CORE,
> > > > > >     	.chip_name = 8183,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_4_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_4_0,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_1,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     /* This platform data is used for one lat and one core
> > > > > > architecture. */
> > > > > > @@ -869,24 +881,72 @@ const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_8188_pdata = {
> > > > > >     	MTK_STATELESS_DEC_DATA,
> > > > > >     	.hw_arch = MTK_VDEC_LAT_SINGLE_CORE,
> > > > > >     	.chip_name = 8188,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_5_2,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_5_1,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     const struct mtk_vcodec_dec_pdata mtk_vdec_8192_pdata =
> > > > > > {
> > > > > >     	MTK_STATELESS_DEC_DATA,
> > > > > >     	.hw_arch = MTK_VDEC_LAT_SINGLE_CORE,
> > > > > >     	.chip_name = 8192,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_5_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_5_1,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_1,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     const struct mtk_vcodec_dec_pdata mtk_vdec_8195_pdata =
> > > > > > {
> > > > > >     	MTK_STATELESS_DEC_DATA,
> > > > > >     	.hw_arch = MTK_VDEC_LAT_SINGLE_CORE,
> > > > > >     	.chip_name = 8195,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_6_0,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_5_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     const struct mtk_vcodec_dec_pdata mtk_vdec_8196_pdata =
> > > > > > {
> > > > > >     	MTK_STATELESS_DEC_DATA,
> > > > > >     	.hw_arch = MTK_VDEC_LAT_SINGLE_CORE,
> > > > > >     	.chip_name = 8196,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_6_0,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_5_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_single_core_pdata = {
> > > > > > @@ -910,6 +970,18 @@ const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_8186_pdata = {
> > > > > >     	MTK_STATELESS_DEC_DATA,
> > > > > >     	.hw_arch = MTK_VDEC_PURE_SINGLE_CORE,
> > > > > >     	.chip_name = 8186,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_4_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_4_1,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_1,
> > > > > > +	},
> > > > > >     };
> > > > > >     
> > > > > >     const struct mtk_vcodec_dec_pdata mtk_vdec_8189_pdata =
> > > > > > {
> > > > > > @@ -928,4 +1000,16 @@ const struct mtk_vcodec_dec_pdata
> > > > > > mtk_vdec_8189_pdata = {
> > > > > >     	.is_subdev_supported = true,
> > > > > >     	.hw_arch = MTK_VDEC_PURE_SINGLE_CORE,
> > > > > >     	.chip_name = 8189,
> > > > > > +	.h264_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_H264_LEVEL_5_2,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10,
> > > > > > +	},
> > > > > > +	.h265_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_HEVC_LEVEL_4,
> > > > > > +		.profile =
> > > > > > V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE,
> > > > > > +	},
> > > > > > +	.vp9_params = {
> > > > > > +		.level = V4L2_MPEG_VIDEO_VP9_LEVEL_5_2,
> > > > > > +		.profile = V4L2_MPEG_VIDEO_VP9_PROFILE_2,
> > > > > > +	},
> > > > > >     };
> > > > > 
> > > > > 
> 
>