| Message ID | 20211006000505.627334-1-aford173@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/7] dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
Am Dienstag, dem 05.10.2021 um 19:05 -0500 schrieb Adam Ford: > This adds the description for the i.MX8MN disp blk-ctrl. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > drivers/soc/imx/imx8m-blk-ctrl.c | 70 ++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > index e172d295c441..8fcd5ed62f50 100644 > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > @@ -14,6 +14,7 @@ > #include <linux/clk.h> > > #include <dt-bindings/power/imx8mm-power.h> > +#include <dt-bindings/power/imx8mn-power.h> > > #define BLK_SFT_RSTN 0x0 > #define BLK_CLK_EN 0x4 > @@ -498,6 +499,75 @@ static const struct imx8m_blk_ctrl_data imx8mm_disp_blk_ctl_dev_data = { > .num_domains = ARRAY_SIZE(imx8mm_disp_blk_ctl_domain_data), > }; > > + > +static int imx8mn_disp_power_notifier(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl, > + power_nb); > + > + if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF) > + return NOTIFY_OK; > + > + /* Enable bus clock and deassert bus reset */ > + regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(8)); > + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(8)); > + > + /* > + * On power up we have no software backchannel to the GPC to > + * wait for the ADB handshake to happen, so we just delay for a > + * bit. On power down the GPC driver waits for the handshake. > + */ > + if (action == GENPD_NOTIFY_ON) > + udelay(5); > + > + > + return NOTIFY_OK; > +} > + > +static const struct imx8m_blk_ctrl_domain_data imx8mn_disp_blk_ctl_domain_data[] = { > + [IMX8MN_DISPBLK_PD_MIPI_DSI] = { > + .name = "dispblk-mipi-dsi", > + .clk_names = (const char *[]){ "dsi-pclk", "dsi-ref", }, > + .num_clks = 2, > + .gpc_name = "mipi-dsi", > + .rst_mask = BIT(0) | BIT(1), > + .clk_mask = BIT(0) | BIT(1), > + }, > + [IMX8MN_DISPBLK_PD_MIPI_CSI] = { > + .name = "dispblk-mipi-csi", > + .clk_names = (const char *[]){ "csi-aclk", "csi-pclk" }, > + .num_clks = 2, > + .gpc_name = "mipi-csi", > + .rst_mask = BIT(2) | BIT(3), > + .clk_mask = BIT(2) | BIT(3), > + }, > + [IMX8MN_DISPBLK_PD_LCDIF] = { > + .name = "dispblk-lcdif", > + .clk_names = (const char *[]){ "lcdif-axi", "lcdif-apb", "lcdif-pix", }, > + .num_clks = 3, > + .gpc_name = "lcdif", > + .rst_mask = BIT(4) | BIT(5), > + .clk_mask = BIT(4) | BIT(5), > + }, > + [IMX8MN_DISPBLK_PD_ISI] = { > + .name = "dispblk-isi", > + .clk_names = (const char *[]){ "disp_axi", "disp_apb", "disp_axi_root", > + "disp_apb_root"}, > + .num_clks = 2, I think those are wrong. At least the num_clks and the number of clock names is inconsistent. > + .gpc_name = "isi", > + .rst_mask = BIT(6) | BIT(7), > + .clk_mask = BIT(6) | BIT(7), > + }, > +}; > + > +static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { > + .max_reg = 0x84, > + .power_notifier_fn = imx8mn_disp_power_notifier, > + .domains = imx8mn_disp_blk_ctl_domain_data, > + .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), > +}; > + You need to hook this up in imx8m_blk_ctrl_of_match, otherwise this is all just dead code. Regards, Lucas > static const struct of_device_id imx8m_blk_ctrl_of_match[] = { > { > .compatible = "fsl,imx8mm-vpu-blk-ctrl",
Am Dienstag, dem 05.10.2021 um 19:05 -0500 schrieb Adam Ford: > Add the DT node for the GPC, including all the PGC power domains, > some of them are not fully functional yet, as they require interaction > with the blk-ctrls to properly power up/down the peripherals. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 49 +++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > index da6c942fb7f9..4191b5bfcdf3 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > @@ -4,6 +4,8 @@ > */ > > #include <dt-bindings/clock/imx8mn-clock.h> > +#include <dt-bindings/power/imx8mn-power.h> > +#include <dt-bindings/reset/imx8mq-reset.h> > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/input/input.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -612,6 +614,53 @@ src: reset-controller@30390000 { > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>; > #reset-cells = <1>; > }; > + > + gpc: gpc@303a0000 { > + compatible = "fsl,imx8mn-gpc"; > + reg = <0x303a0000 0x10000>; > + interrupt-parent = <&gic>; > + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>; > + > + pgc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pgc_hsiomix: power-domain@0 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_HSIOMIX>; > + clocks = <&clk IMX8MN_CLK_USB1_CTRL_ROOT>; This should be IMX8MN_CLK_USB_BUS. Regards, Lucas > + }; > + > + pgc_otg1: power-domain@1 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_OTG1>; > + power-domains = <&pgc_hsiomix>; > + }; > + > + pgc_gpumix: power-domain@2 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_GPUMIX>; > + clocks = <&clk IMX8MN_CLK_GPU_CORE_ROOT>, > + <&clk IMX8MN_CLK_GPU_SHADER_DIV>, > + <&clk IMX8MN_CLK_GPU_BUS_ROOT>, > + <&clk IMX8MN_CLK_GPU_AHB>; > + resets = <&src IMX8MQ_RESET_GPU_RESET>; > + }; > + > + pgc_dispmix: power-domain@3 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_DISPMIX>; > + clocks = <&clk IMX8MN_CLK_DISP_AXI_ROOT>, > + <&clk IMX8MN_CLK_DISP_APB_ROOT>; > + }; > + > + pgc_mipi: power-domain@4 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_MIPI>; > + power-domains = <&pgc_dispmix>; > + }; > + }; > + }; > }; > > aips2: bus@30400000 {
Am Dienstag, dem 05.10.2021 um 19:05 -0500 schrieb Adam Ford: > Now that we have support for the power domain controller on the i.MX8MN, > we can put the USB controller in the respective power domain to allow > it to power down the PHY when possible. > > Signed-off-by: Adam Ford <aford173@gmail.com> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > index 4191b5bfcdf3..ea994dd9fb03 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > @@ -1021,6 +1021,7 @@ usbotg1: usb@32e40000 { > assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_500M>; > phys = <&usbphynop1>; > fsl,usbmisc = <&usbmisc1 0>; > + power-domains = <&pgc_otg1>; > status = "disabled"; > }; >
Am Dienstag, dem 05.10.2021 um 19:05 -0500 schrieb Adam Ford: > The i.MX8M-Nano features a GC7000. The Etnaviv driver detects it as: > > etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6203 > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 25 +++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > index 57f67257d2fd..46144a7482d3 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > @@ -1089,6 +1089,31 @@ gpmi: nand-controller@33002000 { > status = "disabled"; > }; > > + gpu: gpu@38000000 { > + compatible = "vivante,gc"; > + reg = <0x38000000 0x8000>; > + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MN_CLK_GPU_AHB>, > + <&clk IMX8MN_CLK_GPU_BUS_ROOT>, > + <&clk IMX8MN_CLK_GPU_CORE_ROOT>, > + <&clk IMX8MN_CLK_GPU_SHADER_DIV>; Last one should be just IMX8MN_CLK_GPU_SHADER. > + clock-names = "reg", "bus", "core", "shader"; > + assigned-clocks = <&clk IMX8MN_CLK_GPU_CORE_SRC>, > + <&clk IMX8MN_CLK_GPU_SHADER_SRC>, > + <&clk IMX8MN_CLK_GPU_AXI>, > + <&clk IMX8MN_CLK_GPU_AHB>, > + <&clk IMX8MN_GPU_PLL>, > + <&clk IMX8MN_CLK_GPU_CORE_DIV>, > + <&clk IMX8MN_CLK_GPU_SHADER_DIV>; _SRC and _DIV clocks are just legacy aliases for the now composite clocks and should not be used in new DTs. Please just use the composite clock handles directly. Regards, Lucas > + assigned-clock-parents = <&clk IMX8MN_GPU_PLL_OUT>, > + <&clk IMX8MN_GPU_PLL_OUT>, > + <&clk IMX8MN_SYS_PLL1_800M>, > + <&clk IMX8MN_SYS_PLL1_800M>; > + assigned-clock-rates = <0>, <0>, <800000000>, <400000000>, <1200000000>, > + <400000000>, <400000000>; > + power-domains = <&pgc_gpumix>; > + }; > + > gic: interrupt-controller@38800000 { > compatible = "arm,gic-v3"; > reg = <0x38800000 0x10000>,
On Wed, Oct 6, 2021 at 2:45 AM Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Dienstag, dem 05.10.2021 um 19:05 -0500 schrieb Adam Ford: > > This adds the description for the i.MX8MN disp blk-ctrl. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > --- > > drivers/soc/imx/imx8m-blk-ctrl.c | 70 ++++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) > > > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > > index e172d295c441..8fcd5ed62f50 100644 > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > > @@ -14,6 +14,7 @@ > > #include <linux/clk.h> > > > > #include <dt-bindings/power/imx8mm-power.h> > > +#include <dt-bindings/power/imx8mn-power.h> > > > > #define BLK_SFT_RSTN 0x0 > > #define BLK_CLK_EN 0x4 > > @@ -498,6 +499,75 @@ static const struct imx8m_blk_ctrl_data imx8mm_disp_blk_ctl_dev_data = { > > .num_domains = ARRAY_SIZE(imx8mm_disp_blk_ctl_domain_data), > > }; > > > > + > > +static int imx8mn_disp_power_notifier(struct notifier_block *nb, > > + unsigned long action, void *data) > > +{ > > + struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl, > > + power_nb); > > + > > + if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF) > > + return NOTIFY_OK; > > + > > + /* Enable bus clock and deassert bus reset */ > > + regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(8)); > > + regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(8)); > > + > > + /* > > + * On power up we have no software backchannel to the GPC to > > + * wait for the ADB handshake to happen, so we just delay for a > > + * bit. On power down the GPC driver waits for the handshake. > > + */ > > + if (action == GENPD_NOTIFY_ON) > > + udelay(5); > > + > > + > > + return NOTIFY_OK; > > +} > > + > > +static const struct imx8m_blk_ctrl_domain_data imx8mn_disp_blk_ctl_domain_data[] = { > > + [IMX8MN_DISPBLK_PD_MIPI_DSI] = { > > + .name = "dispblk-mipi-dsi", > > + .clk_names = (const char *[]){ "dsi-pclk", "dsi-ref", }, > > + .num_clks = 2, > > + .gpc_name = "mipi-dsi", > > + .rst_mask = BIT(0) | BIT(1), > > + .clk_mask = BIT(0) | BIT(1), > > + }, > > + [IMX8MN_DISPBLK_PD_MIPI_CSI] = { > > + .name = "dispblk-mipi-csi", > > + .clk_names = (const char *[]){ "csi-aclk", "csi-pclk" }, > > + .num_clks = 2, > > + .gpc_name = "mipi-csi", > > + .rst_mask = BIT(2) | BIT(3), > > + .clk_mask = BIT(2) | BIT(3), > > + }, > > + [IMX8MN_DISPBLK_PD_LCDIF] = { > > + .name = "dispblk-lcdif", > > + .clk_names = (const char *[]){ "lcdif-axi", "lcdif-apb", "lcdif-pix", }, > > + .num_clks = 3, > > + .gpc_name = "lcdif", > > + .rst_mask = BIT(4) | BIT(5), > > + .clk_mask = BIT(4) | BIT(5), > > + }, > > + [IMX8MN_DISPBLK_PD_ISI] = { > > + .name = "dispblk-isi", > > + .clk_names = (const char *[]){ "disp_axi", "disp_apb", "disp_axi_root", > > + "disp_apb_root"}, > > + .num_clks = 2, > > I think those are wrong. At least the num_clks and the number of clock > names is inconsistent. The NXP tree shows 4 clocks on the ISI node are enabled before the ISI can be pulled out of reset. I'll make the num_clks = 4. > > > + .gpc_name = "isi", > > + .rst_mask = BIT(6) | BIT(7), > > + .clk_mask = BIT(6) | BIT(7), > > + }, > > +}; > > + > > +static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { > > + .max_reg = 0x84, > > + .power_notifier_fn = imx8mn_disp_power_notifier, > > + .domains = imx8mn_disp_blk_ctl_domain_data, > > + .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), > > +}; > > + > You need to hook this up in imx8m_blk_ctrl_of_match, otherwise this is > all just dead code. Oops. I had that, but i forgot to commit after the save. It'll be fixed shortly. > > Regards, > Lucas > thanks for the review. adam > > static const struct of_device_id imx8m_blk_ctrl_of_match[] = { > > { > > .compatible = "fsl,imx8mm-vpu-blk-ctrl", > >
diff --git a/include/dt-bindings/power/imx8mn-power.h b/include/dt-bindings/power/imx8mn-power.h index 102ee85a9b62..eedd0e581939 100644 --- a/include/dt-bindings/power/imx8mn-power.h +++ b/include/dt-bindings/power/imx8mn-power.h @@ -12,4 +12,9 @@ #define IMX8MN_POWER_DOMAIN_DISPMIX 3 #define IMX8MN_POWER_DOMAIN_MIPI 4 +#define IMX8MN_DISPBLK_PD_MIPI_DSI 0 +#define IMX8MN_DISPBLK_PD_MIPI_CSI 1 +#define IMX8MN_DISPBLK_PD_LCDIF 2 +#define IMX8MN_DISPBLK_PD_ISI 3 + #endif
This adds the defines for the power domains provided by the DISP blk-ctrl. Signed-off-by: Adam Ford <aford173@gmail.com> --- include/dt-bindings/power/imx8mn-power.h | 5 +++++ 1 file changed, 5 insertions(+)