Re: [PATCH v5 23/27] clk: mediatek: Add MT8196 disp-ao clock support

[Laura Nao]

Hi Jason-JH,

On 4/2/26 08:30, Jason-JH Lin (林睿祥) wrote:
> On Fri, 2025-08-29 at 11:19 +0200, Laura Nao wrote:
>> Add support for the MT8196 disp-ao clock controller, which provides
>> clock gate control for the display system. It is integrated with the
>> mtk-mmsys driver, which registers the disp-ao clock driver via
>> platform_device_register_data().
>>
>> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
>> Reviewed-by: AngeloGioacchino Del Regno
>> <angelogioacchino.delregno@xxxxxxxxxxxxx>
>> Signed-off-by: Laura Nao <laura.nao@xxxxxxxxxxxxx>
>> ---
>>  drivers/clk/mediatek/Makefile              |  2 +-
>>  drivers/clk/mediatek/clk-mt8196-vdisp_ao.c | 80
>> ++++++++++++++++++++++
>>  2 files changed, 81 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/clk/mediatek/clk-mt8196-vdisp_ao.c
>>
>> diff --git a/drivers/clk/mediatek/Makefile
>> b/drivers/clk/mediatek/Makefile
>> index fe5699411d8b..5b8969ff1985 100644
>> --- a/drivers/clk/mediatek/Makefile
>> +++ b/drivers/clk/mediatek/Makefile
>> @@ -157,7 +157,7 @@ obj-$(CONFIG_COMMON_CLK_MT8196_IMP_IIC_WRAP) +=
>> clk-mt8196-imp_iic_wrap.o
>>  obj-$(CONFIG_COMMON_CLK_MT8196_MCUSYS) += clk-mt8196-mcu.o
>>  obj-$(CONFIG_COMMON_CLK_MT8196_MDPSYS) += clk-mt8196-mdpsys.o
>>  obj-$(CONFIG_COMMON_CLK_MT8196_MFGCFG) += clk-mt8196-mfg.o
>> -obj-$(CONFIG_COMMON_CLK_MT8196_MMSYS) += clk-mt8196-disp0.o clk-
>> mt8196-disp1.o
>> +obj-$(CONFIG_COMMON_CLK_MT8196_MMSYS) += clk-mt8196-disp0.o clk-
>> mt8196-disp1.o clk-mt8196-vdisp_ao.o
>>  obj-$(CONFIG_COMMON_CLK_MT8196_PEXTPSYS) += clk-mt8196-pextp.o
>>  obj-$(CONFIG_COMMON_CLK_MT8196_UFSSYS) += clk-mt8196-ufs_ao.o
>>  obj-$(CONFIG_COMMON_CLK_MT8365) += clk-mt8365-apmixedsys.o clk-
>> mt8365.o
>> diff --git a/drivers/clk/mediatek/clk-mt8196-vdisp_ao.c
>> b/drivers/clk/mediatek/clk-mt8196-vdisp_ao.c
>> new file mode 100644
>> index 000000000000..fddb69d1c3eb
>> --- /dev/null
>> +++ b/drivers/clk/mediatek/clk-mt8196-vdisp_ao.c
>> @@ -0,0 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2025 MediaTek Inc.
>> + *                    Guangjie Song <guangjie.song@xxxxxxxxxxxx>
>> + * Copyright (c) 2025 Collabora Ltd.
>> + *                    Laura Nao <laura.nao@xxxxxxxxxxxxx>
>> + */
>> +#include <dt-bindings/clock/mediatek,mt8196-clock.h>
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "clk-gate.h"
>> +#include "clk-mtk.h"
>> +
>> +static const struct mtk_gate_regs mm_v_cg_regs = {
>> +	.set_ofs = 0x104,
>> +	.clr_ofs = 0x108,
>> +	.sta_ofs = 0x100,
>> +};
>> +
>> +static const struct mtk_gate_regs mm_v_hwv_regs = {
>> +	.set_ofs = 0x0030,
>> +	.clr_ofs = 0x0034,
>> +	.sta_ofs = 0x2c18,
>> +};
>> +
>> +#define GATE_MM_AO_V(_id, _name, _parent, _shift) {	\
>> +		.id = _id,				\
>> +		.name = _name,				\
>> +		.parent_name = _parent,			\
>> +		.regs = &mm_v_cg_regs,			\
>> +		.shift = _shift,			\
>> +		.ops = &mtk_clk_gate_ops_setclr,	\
>> +		.flags = CLK_OPS_PARENT_ENABLE |	\
>> +			 CLK_IS_CRITICAL,		\
>> +	}
>> +
>> +#define GATE_HWV_MM_V(_id, _name, _parent, _shift) {	\
>> +		.id = _id,				\
>> +		.name = _name,				\
>> +		.parent_name = _parent,			\
>> +		.regs = &mm_v_cg_regs,			\
>> +		.hwv_regs = &mm_v_hwv_regs,		\
>> +		.shift = _shift,			\
>> +		.ops = &mtk_clk_gate_hwv_ops_setclr,	\
>> +		.flags = CLK_OPS_PARENT_ENABLE,		\
>> +	}
>> +
>> +static const struct mtk_gate mm_v_clks[] = {
>> +	GATE_HWV_MM_V(CLK_MM_V_DISP_VDISP_AO_CONFIG,
>> "mm_v_disp_vdisp_ao_config", "disp", 0),
>> +	GATE_HWV_MM_V(CLK_MM_V_DISP_DPC, "mm_v_disp_dpc", "disp",
>> 16),
>> +	GATE_MM_AO_V(CLK_MM_V_SMI_SUB_SOMM0, "mm_v_smi_sub_somm0",
>> "disp", 2),
>> +};
>> +
>> +static const struct mtk_clk_desc mm_v_mcd = {
>> +	.clks = mm_v_clks,
>> +	.num_clks = ARRAY_SIZE(mm_v_clks),
>> +};
>> +
>> +static const struct of_device_id of_match_clk_mt8196_vdisp_ao[] = {
>> +	{ .compatible = "mediatek,mt8196-vdisp-ao", .data =
>> &mm_v_mcd },
>
> Hi Laura,
>
> We are going to send mtk-mmsys driver for MT8196 recently, but we found
> the compatible name is used here.
>
> As your commit message, vdisp-ao is integrated with the mtk-mmsys
> driver, which registers the vdisp-ao clock driver via 
> platform_device_register_data().
>
> Shouldn't this compatible name belong to mmsys driver for MT8196?
>

That's right, my fault for missing that! Thanks for the heads up.

I'm aware Angelo is currently restructuring mediatek-drm (including 
mmsys and mutex), and that might affect the way vdisp-ao is loaded too. 
So I'm not sure whether it makes sense to send a patch to fix this 
right away.

Best,

Laura