[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <92cabfc7-b35c-4ed8-9654-c4ef52bcbe03@collabora.com>
Date: Wed, 28 Jan 2026 15:41:46 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Kyrie Wu (吴晗) <Kyrie.Wu@...iatek.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
George Sun (孙林) <George.Sun@...iatek.com>,
Tiffany Lin (林慧珊) <tiffany.lin@...iatek.com>,
"nhebert@...omium.org" <nhebert@...omium.org>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"mchehab@...nel.org" <mchehab@...nel.org>,
"hverkuil@...all.nl" <hverkuil@...all.nl>,
Yunfei Dong (董云飞) <Yunfei.Dong@...iatek.com>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
Irui Wang (王瑞) <Irui.Wang@...iatek.com>,
"robh@...nel.org" <robh@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "arnd@...db.de" <arnd@...db.de>,
Andrew-CT Chen (陳智迪)
<Andrew-CT.Chen@...iatek.com>
Cc: "andrzejtp2010@...il.com" <andrzejtp2010@...il.com>,
"neil.armstrong@...aro.org" <neil.armstrong@...aro.org>,
Yilong Zhou (周易龙) <Yilong.Zhou@...iatek.com>
Subject: Re: [PATCH v7 05/10] media: mediatek: vcodec: Add Decoder profile &
level Initialization
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://patchwork.linuxtv.org/project/linux-media/patch/20251202074038.3173-5-kyrie.wu@mediatek.com/
>>
>> 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
>
>>
>>>
>>> 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@...labora.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@...iatek.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,
>>>>> + },
>>>>> };
>>>>
>>>>
Powered by blists - more mailing lists