Re: [PATCH v18 05/21] soc: mediatek: add mtk-mmsys config API for mt8195 vdosys1

[Nancy.Lin <nancy.lin@mediatek.com>]

Hi CK,

Thanks for the review.

On Fri, 2022-04-29 at 16:40 +0800, CK Hu wrote:
> Hi, Nancy:
> 
> On Thu, 2022-04-28 at 18:53 +0800, Nancy.Lin wrote:
> > Add four mmsys config APIs. The config APIs are used for config
> > mmsys reg. Some mmsys regs need to be set according to the
> > HW engine binding to the mmsys simultaneously.
> > 
> > 1. mtk_mmsys_merge_async_config: config merge async width/height.
> >    async is used for cross-clock domain synchronization.
> > 2. mtk_mmsys_hdr_confing: config hdr backend async width/height.
> > 3. mtk_mmsys_mixer_in_config and mtk_mmsys_mixer_in_config:
> >    config mixer related settings.
> > 
> > Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > ---
> >  drivers/soc/mediatek/mt8195-mmsys.h    |  6 ++++
> >  drivers/soc/mediatek/mtk-mmsys.c       | 43
> > ++++++++++++++++++++++++++
> >  include/linux/soc/mediatek/mtk-mmsys.h |  9 ++++++
> >  3 files changed, 58 insertions(+)
> > 
> > diff --git a/drivers/soc/mediatek/mt8195-mmsys.h
> > b/drivers/soc/mediatek/mt8195-mmsys.h
> > index 51031d75e81e..5469073e3073 100644
> > --- a/drivers/soc/mediatek/mt8195-mmsys.h
> > +++ b/drivers/soc/mediatek/mt8195-mmsys.h
> > @@ -139,6 +139,12 @@
> >  #define MT8195_VDO1_MIXER_SOUT_SEL_IN				
> > 0xf68
> >  #define MT8195_MIXER_SOUT_SEL_IN_FROM_DISP_MIXER			
> > 0
> >  
> > +#define MT8195_VDO1_MERGE0_ASYNC_CFG_WD	0xe30
> > +#define MT8195_VDO1_HDRBE_ASYNC_CFG_WD	0xe70
> > +#define MT8195_VDO1_HDR_TOP_CFG		0xd00
> > +#define MT8195_VDO1_MIXER_IN1_ALPHA	0xd30
> > +#define MT8195_VDO1_MIXER_IN1_PAD	0xd40
> 
> I would like to sort these by value.
> 
OK. I will refine it.

> > +
> >  static const struct mtk_mmsys_routes mmsys_mt8195_routing_table[]
> > =
> > {
> >  	{
> >  		DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c
> > b/drivers/soc/mediatek/mtk-mmsys.c
> > index 03c75a82c8d3..7b262cefef85 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -280,6 +280,49 @@ static const struct reset_control_ops
> > mtk_mmsys_reset_ops = {
> >  	.reset = mtk_mmsys_reset,
> >  };
> >  
> > +static void mtk_mmsys_write_reg(struct device *dev, u32 offset,
> > u32
> > mask, u32 val)
> > +{
> > +	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> > +	u32 tmp;
> > +
> > +	tmp = readl(mmsys->regs + offset);
> > +	tmp = (tmp & ~mask) | val;
> > +	writel(tmp, mmsys->regs + offset);
> > +}
> 
> It looks like that mtk_mmsys_ddp_connect() and
> mtk_mmsys_ddp_disconnect() could call this function to simplify code.
> So I would like this function to be an independent patch which also
> simplify mtk_mmsys_ddp_connect() and mtk_mmsys_ddp_disconnect().
> 
> Regards,
> CK
> 
OK, I will separate the mtk_mmsys_write_reg function to another patch.

Regards,
Nancy

> > +
> > +void mtk_mmsys_merge_async_config(struct device *dev, int idx, int
> > width, int height)
> > +{
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_MERGE0_ASYNC_CFG_WD + 0x10
> > * idx,
> > +			    ~0, height << 16 | width);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_merge_async_config);
> > +
> > +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int
> > be_height)
> > +{
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_HDRBE_ASYNC_CFG_WD, ~0,
> > +			    be_height << 16 | be_width);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_hdr_confing);
> > +
> > +void mtk_mmsys_mixer_in_config(struct device *dev, int idx, bool
> > alpha_sel, u16 alpha,
> > +			       u8 mode, u32 biwidth)
> > +{
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_MIXER_IN1_ALPHA + (idx -
> > 1) * 4, ~0,
> > +			    alpha << 16 | alpha);
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_HDR_TOP_CFG, BIT(19 +
> > idx),
> > +			    alpha_sel << (19 + idx));
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_MIXER_IN1_PAD + (idx - 1)
> > * 4,
> > +			    GENMASK(31, 16) | GENMASK(1, 0), biwidth <<
> > 16 | mode);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_mixer_in_config);
> > +
> > +void mtk_mmsys_mixer_in_channel_swap(struct device *dev, int idx,
> > bool channel_swap)
> > +{
> > +	mtk_mmsys_write_reg(dev, MT8195_VDO1_MIXER_IN1_PAD + (idx - 1)
> > * 4, BIT(4),
> > +			    channel_swap << 4);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mmsys_mixer_in_channel_swap);
> > +
> >  static int mtk_mmsys_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> > diff --git a/include/linux/soc/mediatek/mtk-mmsys.h
> > b/include/linux/soc/mediatek/mtk-mmsys.h
> > index b4388ba43341..fe620929b0f9 100644
> > --- a/include/linux/soc/mediatek/mtk-mmsys.h
> > +++ b/include/linux/soc/mediatek/mtk-mmsys.h
> > @@ -73,4 +73,13 @@ void mtk_mmsys_ddp_disconnect(struct device
> > *dev,
> >  			      enum mtk_ddp_comp_id cur,
> >  			      enum mtk_ddp_comp_id next);
> >  
> > +void mtk_mmsys_merge_async_config(struct device *dev, int idx, int
> > width, int height);
> > +
> > +void mtk_mmsys_hdr_confing(struct device *dev, int be_width, int
> > be_height);
> > +
> > +void mtk_mmsys_mixer_in_config(struct device *dev, int idx, bool
> > alpha_sel, u16 alpha,
> > +			       u8 mode, u32 biwidth);
> > +
> > +void mtk_mmsys_mixer_in_channel_swap(struct device *dev, int idx,
> > bool channel_swap);
> > +
> >  #endif /* __MTK_MMSYS_H */
> 
>