Re: [PATCH v3 02/14] media: mediatek: vcodec: add decoder compatible to support mt8196

[Yunfei Dong (董云飞)]

Hi Nicolas Dufresne & Nicolas Frattaroli,

Thanks for your suggestion.

On Thu, 2026-02-12 at 14:31 -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le jeudi 12 février 2026 à 17:48 +0100, Nicolas Frattaroli a écrit :
> > On Wednesday, 11 February 2026 06:41:29 Central European Standard
> > Time Yunfei
> > Dong wrote:
> > > MT8196 is lat single core architecture. Support its compatible
> > > and
> > > use `mtk_lat_sig_core_pdata` to initialize platform data.
> > > 
> > > Signed-off-by: Yunfei Dong <yunfei.dong@xxxxxxxxxxxx>
> > > ---
> > >  .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c   | 6
> > > ++++++
> > >  .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h   | 1
> > > +
> > >  2 files changed, 7 insertions(+)
> > > 
> > > diff --git
> > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.c
> > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.c
> > > index 3b81fae9f913..d9f722698198 100644
> > > ---
> > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.c
> > > +++
> > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.c
> > > @@ -347,6 +347,8 @@ static void
> > > mtk_vcodec_dec_get_chip_name(struct
> > > mtk_vcodec_dec_dev *vdec_dev)
> > >  		vdec_dev->chip_name = MTK_VDEC_MT8186;
> > >  	else if (of_device_is_compatible(dev->of_node,
> > > "mediatek,mt8188-
> > > vcodec-dec"))
> > >  		vdec_dev->chip_name = MTK_VDEC_MT8188;
> > > +	else if (of_device_is_compatible(dev->of_node,
> > > "mediatek,mt8196-
> > > vcodec-dec"))
> > > +		vdec_dev->chip_name = MTK_VDEC_MT8196;
> > 
> > Why is this entire function written like this, and why does it
> > exist
> > at all? You can store the chip name in the platform data for a
> > compatible,
> > thereby avoiding a massive of_device_is_compatible if block because
> > that's
> > what platform data is for.
> > 
> > The only place where this function is even used is in probe.
> > 
> > Just store it in your mtk_vcodec_match's .data struct and pull it
> > from
> > dev->vdec_pdata. No need for the function.
> 
> I have already asked this to the dev working on some other MTK codec
> patchset,
> and I think he already posted some proper refactoring (but had more
> changes to
> finish it up). Mind, Yunfei, coordinating the effort on removing all
> the if/else
> please ? We are doing the same code review again and again with every
> single
> individual working on this driver.
> 

I had already confirmed with kyrie, he had changed the compatible patch
in mt8189's patch set. So I just need to add mt8196's compatible.


https://patchwork.linuxtv.org/project/linux-media/patch/20260127024248.18406-3-kyrie.wu@xxxxxxxxxxxx/

mt8189's patch set is based on this patch set(8196's).

> regards,
> Nicolas
> 
Best Regards,
Yunfei Dong
> > 
> > >  	else
> > >  		vdec_dev->chip_name = MTK_VDEC_INVAL;
> > >  }
> > > @@ -560,6 +562,10 @@ static const struct of_device_id
> > > mtk_vcodec_match[] = {
> > >  		.compatible = "mediatek,mt8188-vcodec-dec",
> > >  		.data = &mtk_lat_sig_core_pdata,
> > >  	},
> > > +	{
> > > +		.compatible = "mediatek,mt8196-vcodec-dec",
> > > +		.data = &mtk_lat_sig_core_pdata,
> > > +	},
> > >  	{},
> > >  };
> > >  
> > > diff --git
> > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.h
> > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.h
> > > index c9d27534c63e..f06dfc1a3455 100644
> > > ---
> > > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.h
> > > +++
> > > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_d
> > > rv.h
> > > @@ -29,6 +29,7 @@ enum mtk_vcodec_dec_chip_name {
> > >  	MTK_VDEC_MT8188 = 8188,
> > >  	MTK_VDEC_MT8192 = 8192,
> > >  	MTK_VDEC_MT8195 = 8195,
> > > +	MTK_VDEC_MT8196 = 8196,
> > >  };
> > >  
> > >  /*
> > > 
> > 
> > 
> >