| Message ID | 20230206152928.918562-36-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MediaTek clocks: full module build and cleanups | expand |
On Mon, Feb 6, 2023 at 11:30 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > MT8195 clock drivers were encapsulated in one single (and big) Kconfig > option: there's no reason to do that, as it is totally unnecessary to > build in all or none of them. > > Split them out: keep boot-critical clocks as bool and allow choosing > non critical clocks as tristate. The power domain controller references vppsys*, vdecsys*, vdosys*, wpesys, imgsys and camsys. I'd argue that this makes these clock drivers semi-boot-critical. Maybe mfgcfg as well when we add the GPU? They should be bundled together at the very least. The power domain controller not probing disables all display and multimedia capabilities. Also wondering if we should have "default COMMON_CLK_MT8195" ... I suppose the same questions apply to other SoCs. ChenYu
Il 08/02/23 09:28, Chen-Yu Tsai ha scritto: > On Mon, Feb 6, 2023 at 11:30 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> MT8195 clock drivers were encapsulated in one single (and big) Kconfig >> option: there's no reason to do that, as it is totally unnecessary to >> build in all or none of them. >> >> Split them out: keep boot-critical clocks as bool and allow choosing >> non critical clocks as tristate. > > The power domain controller references vppsys*, vdecsys*, vdosys*, wpesys, > imgsys and camsys. I'd argue that this makes these clock drivers > semi-boot-critical. Maybe mfgcfg as well when we add the GPU? You don't need to power on additional power domains if you want to load modules from a ramdisk! :-) Besides, you caught me: mtk-pm-domains will be my next target after clocks... I don't like how it behaves in regard to probe deferrals. Specifically, I dislike the fact that you either register *all domains* or *none at all* (unless instantiating two different driver instances and that's ugly). Angelo > > They should be bundled together at the very least. The power domain > controller not probing disables all display and multimedia capabilities. > > Also wondering if we should have "default COMMON_CLK_MT8195" ... > > I suppose the same questions apply to other SoCs. > > ChenYu
On Wed, Feb 8, 2023 at 5:00 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 08/02/23 09:28, Chen-Yu Tsai ha scritto: > > On Mon, Feb 6, 2023 at 11:30 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > >> > >> MT8195 clock drivers were encapsulated in one single (and big) Kconfig > >> option: there's no reason to do that, as it is totally unnecessary to > >> build in all or none of them. > >> > >> Split them out: keep boot-critical clocks as bool and allow choosing > >> non critical clocks as tristate. > > > > The power domain controller references vppsys*, vdecsys*, vdosys*, wpesys, > > imgsys and camsys. I'd argue that this makes these clock drivers > > semi-boot-critical. Maybe mfgcfg as well when we add the GPU? > > You don't need to power on additional power domains if you want to load modules > from a ramdisk! :-) Right. > Besides, you caught me: mtk-pm-domains will be my next target after clocks... > I don't like how it behaves in regard to probe deferrals. Specifically, > I dislike the fact that you either register *all domains* or *none at all* > (unless instantiating two different driver instances and that's ugly). I don't really like it either, but is it possible to split probe deferrals? I mean, if you skip a couple power domains because the clocks aren't available, how do you come back to them? And IIRC for a clock provider that is _not_ marked as disabled in the DT, trying to fetch a clock from it would just give -EPROBEDEFER until the provider is registered. ChenYu > > > > They should be bundled together at the very least. The power domain > > controller not probing disables all display and multimedia capabilities. > > > > Also wondering if we should have "default COMMON_CLK_MT8195" ... > > > > I suppose the same questions apply to other SoCs. > > > > ChenYu > >
Il 09/02/23 04:46, Chen-Yu Tsai ha scritto: > On Wed, Feb 8, 2023 at 5:00 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 08/02/23 09:28, Chen-Yu Tsai ha scritto: >>> On Mon, Feb 6, 2023 at 11:30 PM AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> wrote: >>>> >>>> MT8195 clock drivers were encapsulated in one single (and big) Kconfig >>>> option: there's no reason to do that, as it is totally unnecessary to >>>> build in all or none of them. >>>> >>>> Split them out: keep boot-critical clocks as bool and allow choosing >>>> non critical clocks as tristate. >>> >>> The power domain controller references vppsys*, vdecsys*, vdosys*, wpesys, >>> imgsys and camsys. I'd argue that this makes these clock drivers >>> semi-boot-critical. Maybe mfgcfg as well when we add the GPU? >> >> You don't need to power on additional power domains if you want to load modules >> from a ramdisk! :-) > > Right. > >> Besides, you caught me: mtk-pm-domains will be my next target after clocks... >> I don't like how it behaves in regard to probe deferrals. Specifically, >> I dislike the fact that you either register *all domains* or *none at all* >> (unless instantiating two different driver instances and that's ugly). > > I don't really like it either, but is it possible to split probe deferrals? > I mean, if you skip a couple power domains because the clocks aren't > available, how do you come back to them? > Honestly, I have no clue right now - I didn't even think about any possible implementation for now... but let's see what I can come up with whenever I get a chance to actually take a look. Surely not before finishing work on this series, though. > And IIRC for a clock provider that is _not_ marked as disabled in the DT, > trying to fetch a clock from it would just give -EPROBEDEFER until > the provider is registered. > Yes it will give a probe deferral. An internal probe retry mechanism on the power domains that couldn't probe would be one of the possible options. Actually, we have almost endless options on how to resolve that power domains issue, so it's not worrying me at all! Cheers, Angelo > ChenYu > >>> >>> They should be bundled together at the very least. The power domain >>> controller not probing disables all display and multimedia capabilities. >>> >>> Also wondering if we should have "default COMMON_CLK_MT8195" ... >>> >>> I suppose the same questions apply to other SoCs. >>> >>> ChenYu >> >>
diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig index 45b7aea7648d..88937d111e98 100644 --- a/drivers/clk/mediatek/Kconfig +++ b/drivers/clk/mediatek/Kconfig @@ -692,6 +692,92 @@ config COMMON_CLK_MT8195 help This driver supports MediaTek MT8195 clocks. +config COMMON_CLK_MT8195_APUSYS + tristate "Clock driver for MediaTek MT8195 apusys" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 AI Processor Unit System clocks. + +config COMMON_CLK_MT8195_AUDSYS + tristate "Clock driver for MediaTek MT8195 audsys" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 audsys clocks. + +config COMMON_CLK_MT8195_CAMSYS + tristate "Clock driver for MediaTek MT8195 camsys" + depends on COMMON_CLK_MT8195_VPPSYS + help + This driver supports MediaTek MT8195 camsys and camsys_raw clocks. + +config COMMON_CLK_MT8195_IMGSYS + tristate "Clock driver for MediaTek MT8195 imgsys" + depends on COMMON_CLK_MT8195_VPPSYS + help + This driver supports MediaTek MT8195 imgsys and imgsys2 clocks. + +config COMMON_CLK_MT8195_IMP_IIC_WRAP + tristate "Clock driver for MediaTek MT8195 imp_iic_wrap" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 I2C/I3C clocks. + +config COMMON_CLK_MT8195_IPESYS + tristate "Clock driver for MediaTek MT8195 ipesys" + depends on COMMON_CLK_MT8195_IMGSYS + help + This driver supports MediaTek MT8195 ipesys clocks. + +config COMMON_CLK_MT8195_MFGCFG + tristate "Clock driver for MediaTek MT8195 mfgcfg" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 mfgcfg clocks. + +config COMMON_CLK_MT8195_VDOSYS + tristate "Clock driver for MediaTek MT8195 vdosys" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 vdosys0/1 (multimedia) clocks. + +config COMMON_CLK_MT8195_MSDC + tristate "Clock driver for MediaTek MT8195 msdc" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 MMC and SD Controller's + msdc and msdc_top clocks. + +config COMMON_CLK_MT8195_SCP_ADSP + tristate "Clock driver for MediaTek MT8195 scp_adsp" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 System Companion Processor + Audio DSP clocks. + +config COMMON_CLK_MT8195_VPPSYS + tristate "Clock driver for MediaTek MT8195 vppsys" + depends on COMMON_CLK_MT8195 + help + This driver supports MediaTek MT8195 vppsys0/1 clocks. + +config COMMON_CLK_MT8195_VDECSYS + tristate "Clock driver for MediaTek MT8195 vdecsys" + depends on COMMON_CLK_MT8195_VPPSYS + help + This driver supports MediaTek MT8195 vdecsys and vdecsys_soc clocks. + +config COMMON_CLK_MT8195_VENCSYS + tristate "Clock driver for MediaTek MT8195 vencsys" + depends on COMMON_CLK_MT8195_VPPSYS + help + This driver supports MediaTek MT8195 vencsys clocks. + +config COMMON_CLK_MT8195_WPESYS + tristate "Clock driver for MediaTek MT8195 wpesys" + depends on COMMON_CLK_MT8195_IMGSYS + help + This driver supports MediaTek MT8195 Warp Engine clocks. + config COMMON_CLK_MT8365 tristate "Clock driver for MediaTek MT8365" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile index 3133ad8c2028..63351957f862 100644 --- a/drivers/clk/mediatek/Makefile +++ b/drivers/clk/mediatek/Makefile @@ -106,13 +106,19 @@ obj-$(CONFIG_COMMON_CLK_MT8192_SCP_ADSP) += clk-mt8192-scp_adsp.o obj-$(CONFIG_COMMON_CLK_MT8192_VDECSYS) += clk-mt8192-vdec.o obj-$(CONFIG_COMMON_CLK_MT8192_VENCSYS) += clk-mt8192-venc.o obj-$(CONFIG_COMMON_CLK_MT8195) += clk-mt8195-apmixedsys.o clk-mt8195-topckgen.o \ - clk-mt8195-peri_ao.o clk-mt8195-infra_ao.o \ - clk-mt8195-cam.o clk-mt8195-ccu.o clk-mt8195-img.o \ - clk-mt8195-ipe.o clk-mt8195-mfg.o clk-mt8195-scp_adsp.o \ - clk-mt8195-vdec.o clk-mt8195-vdo0.o clk-mt8195-vdo1.o \ - clk-mt8195-venc.o clk-mt8195-vpp0.o clk-mt8195-vpp1.o \ - clk-mt8195-wpe.o clk-mt8195-imp_iic_wrap.o \ - clk-mt8195-apusys_pll.o + clk-mt8195-peri_ao.o clk-mt8195-infra_ao.o +obj-$(CONFIG_COMMON_CLK_MT8195_APUSYS) += clk-mt8195-apusys_pll.o +obj-$(CONFIG_COMMON_CLK_MT8195_CAMSYS) += clk-mt8195-cam.o clk-mt8195-ccu.o +obj-$(CONFIG_COMMON_CLK_MT8195_IMGSYS) += clk-mt8195-img.o +obj-$(CONFIG_COMMON_CLK_MT8195_IMP_IIC_WRAP) += clk-mt8195-imp_iic_wrap.o +obj-$(CONFIG_COMMON_CLK_MT8195_IPESYS) += clk-mt8195-ipe.o +obj-$(CONFIG_COMMON_CLK_MT8195_MFGCFG) += clk-mt8195-mfg.o +obj-$(CONFIG_COMMON_CLK_MT8195_SCP_ADSP) += clk-mt8195-scp_adsp.o +obj-$(CONFIG_COMMON_CLK_MT8195_VDECSYS) += clk-mt8195-vdec.o +obj-$(CONFIG_COMMON_CLK_MT8195_VDOSYS) += clk-mt8195-vdo0.o clk-mt8195-vdo1.o +obj-$(CONFIG_COMMON_CLK_MT8195_VENCSYS) += clk-mt8195-venc.o +obj-$(CONFIG_COMMON_CLK_MT8195_VPPSYS) += clk-mt8195-vpp0.o clk-mt8195-vpp1.o +obj-$(CONFIG_COMMON_CLK_MT8195_WPESYS) += clk-mt8195-wpe.o obj-$(CONFIG_COMMON_CLK_MT8365) += clk-mt8365.o clk-mt8365-apmixedsys.o obj-$(CONFIG_COMMON_CLK_MT8365_APU) += clk-mt8365-apu.o obj-$(CONFIG_COMMON_CLK_MT8365_CAM) += clk-mt8365-cam.o
MT8195 clock drivers were encapsulated in one single (and big) Kconfig option: there's no reason to do that, as it is totally unnecessary to build in all or none of them. Split them out: keep boot-critical clocks as bool and allow choosing non critical clocks as tristate. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/clk/mediatek/Kconfig | 86 +++++++++++++++++++++++++++++++++++ drivers/clk/mediatek/Makefile | 20 +++++--- 2 files changed, 99 insertions(+), 7 deletions(-)