Re: [PATCH 08/26] power: Add iMX8M block ctrl driver for dispmix

Heiko Schocher
2024-09-24T09:07:43Z

Hello Dario,

On 13.09.24 11:55, Dario Binacchi wrote:
> From: Michael Trimarchi <michael@amarulasolutions.com>
>
> Add iMX8 block ctrl driver for displaymix on iMX8MM/iMX8MN and
> mediamix on iMX8MP.
>
> To support blk ctrl driver, the power domain driver on iMX8M needs
> update to add relevant PGC domains
>
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> drivers/power/domain/Kconfig | 6 +
> drivers/power/domain/Makefile | 1 +
> drivers/power/domain/imx8m-blk-ctrl.c | 438 ++++++++++++++++++++++
> drivers/power/domain/imx8m-power-domain.c | 213 ++++++++++-
> 4 files changed, 656 insertions(+), 2 deletions(-)
> create mode 100644 drivers/power/domain/imx8m-blk-ctrl.c
>
> diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig
> index bd82d2f7044b..fb006b6e8e28 100644
> --- a/drivers/power/domain/Kconfig
> +++ b/drivers/power/domain/Kconfig
> @@ -40,6 +40,12 @@ config IMX8M_POWER_DOMAIN
> Enable support for manipulating NXP i.MX8M on-SoC power domains via
> requests to the ATF.
>
> +config IMX8M_BLK_CTRL
> + bool "Enable i.MX8M block control driver"
> + depends on POWER_DOMAIN && ARCH_IMX8M
> + help
> + Enable support for manipulating NXP i.MX8M on-SoC block control driver
> +
> config IMX8MP_HSIOMIX_BLKCTRL
> bool "Enable i.MX8MP HSIOMIX domain driver"
> depends on POWER_DOMAIN && IMX8MP
> diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile
> index 2daab73eb758..46849fd2a4db 100644
> --- a/drivers/power/domain/Makefile
> +++ b/drivers/power/domain/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_APPLE_PMGR_POWER_DOMAIN) += apple-pmgr.o
> obj-$(CONFIG_BCM6328_POWER_DOMAIN) += bcm6328-power-domain.o
> obj-$(CONFIG_IMX8_POWER_DOMAIN) += imx8-power-domain-legacy.o imx8-power-domain.o
> obj-$(CONFIG_IMX8M_POWER_DOMAIN) += imx8m-power-domain.o
> +obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8m-blk-ctrl.o
> obj-$(CONFIG_IMX8MP_HSIOMIX_BLKCTRL) += imx8mp-hsiomix.o
> obj-$(CONFIG_MTK_POWER_DOMAIN) += mtk-power-domain.o
> obj-$(CONFIG_MESON_GX_VPU_POWER_DOMAIN) += meson-gx-pwrc-vpu.o
> diff --git a/drivers/power/domain/imx8m-blk-ctrl.c b/drivers/power/domain/imx8m-blk-ctrl.c
> new file mode 100644
> index 000000000000..4c89078b991b
> --- /dev/null
> +++ b/drivers/power/domain/imx8m-blk-ctrl.c
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2023 NXP
> + */
> +
> +#include <dm.h>
> +#include <malloc.h>
> +#include <power-domain-uclass.h>
> +#include <asm/io.h>
> +#include <dm/device-internal.h>
> +#include <dm/device.h>
> +#include <dt-bindings/power/imx8mm-power.h>
> +#include <dt-bindings/power/imx8mn-power.h>
> +#include <dt-bindings/power/imx8mp-power.h>
> +#include <clk.h>
> +#include <linux/delay.h>
> +
> +#define BLK_SFT_RSTN 0x0
> +#define BLK_CLK_EN 0x4
> +#define BLK_MIPI_RESET_DIV 0x8 /* Mini/Nano/Plus DISPLAY_BLK_CTRL only */
> +
> +#define DOMAIN_MAX_CLKS 4
> +
> +struct imx8m_blk_ctrl_domain {
> + struct clk clks[DOMAIN_MAX_CLKS];
> + struct power_domain power_dev;
> +};
> +
> +struct imx8m_blk_ctrl {
> + void __iomem *base;
> + struct power_domain bus_power_dev;
> + struct imx8m_blk_ctrl_domain *domains;
> +};
> +
> +struct imx8m_blk_ctrl_domain_data {
> + const char *name;
> + const char * const *clk_names;
> + const char *gpc_name;
> + int num_clks;
> + u32 rst_mask;
> + u32 clk_mask;
> + u32 mipi_phy_rst_mask;
> +};
> +
> +struct imx8m_blk_ctrl_data {
> + int max_reg;
> + const struct imx8m_blk_ctrl_domain_data *domains;
> + int num_domains;
> + u32 bus_rst_mask;
> + u32 bus_clk_mask;
> +};
> +
> +static int imx8m_blk_ctrl_request(struct power_domain *power_domain)
> +{
> + return 0;
> +}
> +
> +static int imx8m_blk_ctrl_free(struct power_domain *power_domain)
> +{
> + return 0;
> +}
> +
> +static int imx8m_blk_ctrl_enable_domain_clk(struct udevice *dev, ulong domain_id, bool enable)
> +{
> + int ret, i;
> + struct imx8m_blk_ctrl *priv = (struct imx8m_blk_ctrl *)dev_get_priv(dev);
> + struct imx8m_blk_ctrl_data *drv_data =
> + (struct imx8m_blk_ctrl_data *)dev_get_driver_data(dev);
> +
> + debug("%s num_clk %u\n", __func__, drv_data->domains[domain_id].num_clks);
> +
> + for (i = 0; i < drv_data->domains[domain_id].num_clks; i++) {
> + debug("%s clk %s\n", __func__, drv_data->domains[domain_id].clk_names[i]);
> + if (enable)
> + ret = clk_enable(&priv->domains[domain_id].clks[i]);
> + else
> + ret = clk_disable(&priv->domains[domain_id].clks[i]);
> + if (ret && ret != -ENOENT) {
> + printf("Failed to %s domain clk %s\n", enable ? "enable" : "disable",
> + drv_data->domains[domain_id].clk_names[i]);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int imx8m_blk_ctrl_power_on(struct power_domain *power_domain)
> +{
> + struct udevice *dev = power_domain->dev;
> + struct imx8m_blk_ctrl *priv = (struct imx8m_blk_ctrl *)dev_get_priv(dev);
> + struct imx8m_blk_ctrl_data *drv_data =
> + (struct imx8m_blk_ctrl_data *)dev_get_driver_data(dev);
> + int ret;
> +
> + debug("%s, id %lu\n", __func__, power_domain->id);
> +
> + if (!priv->domains[power_domain->id].power_dev.dev)
> + return -ENODEV;
> +
> + ret = power_domain_on(&priv->bus_power_dev);
> + if (ret < 0) {
> + printf("Failed to power up bus domain %d\n", ret);
> + return ret;
> + }
> +
> + /* Enable bus clock and deassert bus reset */
> + setbits_le32(priv->base + BLK_CLK_EN, drv_data->bus_clk_mask);
> + setbits_le32(priv->base + BLK_SFT_RSTN, drv_data->bus_rst_mask);
> +
> + /* wait for reset to propagate */
> + udelay(5);
> +
> + /* put devices into reset */
> + clrbits_le32(priv->base + BLK_SFT_RSTN, drv_data->domains[power_domain->id].rst_mask);
> + if (drv_data->domains[power_domain->id].mipi_phy_rst_mask)
> + clrbits_le32(priv->base + BLK_MIPI_RESET_DIV, d
> + rv_data->domains[power_domain->id].mipi_phy_rst_mask);

Does this build for you?

I needed the following fix:

diff --git a/drivers/power/domain/imx8m-blk-ctrl.c b/drivers/power/domain/imx8m-blk-ctrl.c
index 4c89078b99..b772d50480 100644
--- a/drivers/power/domain/imx8m-blk-ctrl.c
+++ b/drivers/power/domain/imx8m-blk-ctrl.c
@@ -114,8 +114,8 @@ static int imx8m_blk_ctrl_power_on(struct power_domain *power_domain)
/* put devices into reset */
clrbits_le32(priv->base + BLK_SFT_RSTN, drv_data->domains[power_domain->id].rst_mask);
if (drv_data->domains[power_domain->id].mipi_phy_rst_mask)
- clrbits_le32(priv->base + BLK_MIPI_RESET_DIV, d
- rv_data->domains[power_domain->id].mipi_phy_rst_mask);
+ clrbits_le32(priv->base + BLK_MIPI_RESET_DIV,
+ drv_data->domains[power_domain->id].mipi_phy_rst_mask);

/* enable upstream and blk-ctrl clocks to allow reset to propagate */
ret = imx8m_blk_ctrl_enable_domain_clk(dev, power_domain->id, true);

to get it building.

BTW: Just also working on bootlogo support for an imx8mp based board!

I now applied your patches:

clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux
clk: clk-uclass: Implement CLK_OPS_PARENT_ENABLE

And added in my adapted /drivers/clk/imx/clk-imx8mp.c (imported from linux)

clk_dm(IMX8MP_CLK_MEDIA_AXI, imx8m_clk_composite_flags("media_axi", imx8mp_media_axi_sels,
ARRAY_SIZE(imx8mp_media_axi_sels), base + 0x8a00, CLK_IS_CRITICAL));

instead of using imx8m_clk_composite, and dropped my approach to
get clocks up and working.

Also dropped my similiar approach for mediablock and used your

power: Add iMX8M block ctrl driver for dispmix

And with this 3 patches, bootlogo works also/still fine for me!

I did not applied/need your patches:

clk: imx8mn: Prevent clock critical path from disabling during reparent and set_rate
clk: imx8mm: Prevent clock critical path from disabling during reparent and set_rate
clk: imx8mm: Mark IMX8MM_SYS_PLL2 and IMX8MM_SYS_PLL3 as enabled
clk: imx8mn: Mark IMX8MN_SYS_PLL2 and IMX8MN_SYS_PLL3 as enabled

Of course, they are for imx8mm, but I mean I do not need similiar patches
for imx8mp!

Also not applied (as for imx8mm)
clk: imx8mn: add video clocks support

but as said, have similiar patch for clk-imx8mp.c

May you check if using imx8m_clk_composite_flags() is working for you?

I did not applied your patches 09/26 and the following patches from
your series, as I made a video bridge driver based on

linux driver drivers/gpu/drm/bridge/fsl-ldb.c

and a lcdif driver based on linux:/drivers/gpu/drm/mxsfb/lcdif_drv.c

bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de