Re: [PATCH v1 4/6] soc: mediatek: mmsys: add config api for RSZ switching and DCM

[AngeloGioacchino Del Regno]

Il 04/10/22 11:33, Moudy Ho ha scritto:
> From: "Roy-CW.Yeh" <roy-cw.yeh@xxxxxxxxxxxx>
>
> Due to MT8195 HW design, some RSZs have additional settings that
> need to be configured in MMSYS.
>
> Signed-off-by: Roy-CW.Yeh <roy-cw.yeh@xxxxxxxxxxxx>

Hello Moudy,

please remember that you have to add your Signed-off-by tag to all of the commits
that you're sending, even if you're not the author, otherwise they are not
acceptable.

> ---
>   drivers/soc/mediatek/mt8195-mmsys.h    |  8 ++++++
>   drivers/soc/mediatek/mtk-mmsys.c       | 40 ++++++++++++++++++++++++++
>   include/linux/soc/mediatek/mtk-mmsys.h |  4 +++
>   3 files changed, 52 insertions(+)
>
> diff --git a/drivers/soc/mediatek/mt8195-mmsys.h b/drivers/soc/mediatek/mt8195-mmsys.h
> index abfe94a30248..e0cf13d09763 100644
> --- a/drivers/soc/mediatek/mt8195-mmsys.h
> +++ b/drivers/soc/mediatek/mt8195-mmsys.h
> @@ -75,6 +75,14 @@
>   #define MT8195_SOUT_DSC_WRAP1_OUT_TO_SINA_VIRTUAL0		(2 << 16)
>   #define MT8195_SOUT_DSC_WRAP1_OUT_TO_VPP_MERGE			(3 << 16)
>   
> +/* VPPSYS1 */
> +#define MT8195_SVPP1_HW_DCM_1ST_DIS0				0x150
> +#define MT8195_SVPP1_HW_DCM_1ST_DIS1				0x160
> +#define MT8195_SVPP1_HW_DCM_2ND_DIS0				0x1a0
> +#define MT8195_SVPP1_HW_DCM_2ND_DIS1				0x1b0
> +#define MT8195_SVPP2_BUF_BF_RSZ_SWITCH				0xf48
> +#define MT8195_SVPP3_BUF_BF_RSZ_SWITCH				0xf74
> +
>   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 c4d15f99f853..c98cfcb7db38 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -261,6 +261,46 @@ void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val)
>   }
>   EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_dpi_fmt_config);
>   
> +void mtk_mmsys_merge_config(struct device *dev, u32 id, bool enable)

void mtk_mmsys_merge_config(struct device *dev, u32 svpp_id, bool enable)

or

void mtk_mmsys_vpp_merge_config(struct device *dev, u32 id, bool enable)

...adding that "svpp" or "vpp" word makes the function easier to understand :-)

> +{
> +	u32 reg;
> +
> +	switch (id) {
> +	case 2:
> +		reg = MT8195_SVPP2_BUF_BF_RSZ_SWITCH;
> +		break;
> +	case 3:
> +		reg = MT8195_SVPP3_BUF_BF_RSZ_SWITCH;
> +		break;
> +	default:
> +		dev_err(dev, "Invalid id %d\n", id);
> +		return;
> +	}
> +
> +	mtk_mmsys_update_bits(dev_get_drvdata(dev), reg, ~0, enable);
> +}
> +EXPORT_SYMBOL_GPL(mtk_mmsys_merge_config);
> +
> +void mtk_mmsys_rsz_dcm_config(struct device *dev, bool enable)

...would be the same here, but only about the function name, so I'd go with
changing the name for both.

> +{
> +	u32 val = 0;
> +
> +	if (enable)
> +		val = BIT(25);

No magic bits please, add a definition for them

> +	mtk_mmsys_update_bits(dev_get_drvdata(dev),
> +			      MT8195_SVPP1_HW_DCM_1ST_DIS0, BIT(25), val);
> +	mtk_mmsys_update_bits(dev_get_drvdata(dev),
> +			      MT8195_SVPP1_HW_DCM_2ND_DIS0, BIT(25), val);
> +
> +	if (enable)
> +		val = (BIT(4) | BIT(5));

same here

> +	mtk_mmsys_update_bits(dev_get_drvdata(dev),
> +			      MT8195_SVPP1_HW_DCM_1ST_DIS1, (BIT(4) | BIT(5)), val);
> +	mtk_mmsys_update_bits(dev_get_drvdata(dev),
> +			      MT8195_SVPP1_HW_DCM_2ND_DIS1, (BIT(4) | BIT(5)), val);
> +}
> +EXPORT_SYMBOL_GPL(mtk_mmsys_rsz_dcm_config);
> +
>   static int mtk_mmsys_reset_update(struct reset_controller_dev *rcdev, unsigned long id,
>   				  bool assert)
>   {
> diff --git a/include/linux/soc/mediatek/mtk-mmsys.h b/include/linux/soc/mediatek/mtk-mmsys.h
> index d2b02bb43768..2d5c7fe920b0 100644
> --- a/include/linux/soc/mediatek/mtk-mmsys.h
> +++ b/include/linux/soc/mediatek/mtk-mmsys.h
> @@ -67,4 +67,8 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
>   
>   void mtk_mmsys_ddp_dpi_fmt_config(struct device *dev, u32 val);
>   
> +void mtk_mmsys_merge_config(struct device *dev, u32 id, bool enable);
> +
> +void mtk_mmsys_rsz_dcm_config(struct device *dev, bool enable);
> +
>   #endif /* __MTK_MMSYS_H */