| Message ID | 20220601094537.3390127-5-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add interconnect for i.MX8MP blk ctrl | expand |
Hi Peng, Thank you for the patch. On Wed, Jun 01, 2022 at 05:45:33PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > Add interconnect paths for i.MX8MP media blk ctrl > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 1 + > drivers/soc/imx/imx8m-blk-ctrl.c | 31 +++++++++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > index d9542dfff83f..2a1c6ff37e03 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -4,6 +4,7 @@ > */ > > #include <dt-bindings/clock/imx8mp-clock.h> > +#include <dt-bindings/interconnect/fsl,imx8mp.h> > #include <dt-bindings/power/imx8mp-power.h> > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/input/input.h> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > index 7f49385ed2f8..423cac0c9cb6 100644 > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > @@ -5,6 +5,7 @@ > */ > > #include <linux/device.h> > +#include <linux/interconnect.h> > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data { > const char *name; > const char * const *clk_names; > int num_clks; > + const char * const *path_names; > + int num_paths; > const char *gpc_name; > u32 rst_mask; > u32 clk_mask; > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data { > }; > > #define DOMAIN_MAX_CLKS 4 > +#define DOMAIN_MAX_PATHS 4 > > struct imx8m_blk_ctrl_domain { > struct generic_pm_domain genpd; > const struct imx8m_blk_ctrl_domain_data *data; > struct clk_bulk_data clks[DOMAIN_MAX_CLKS]; > + struct icc_bulk_data paths[DOMAIN_MAX_PATHS]; > struct device *power_dev; > struct imx8m_blk_ctrl *bc; > }; > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd) > if (data->mipi_phy_rst_mask) > regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask); > > + ret = icc_bulk_set_bw(data->num_paths, domain->paths); > + if (ret) > + dev_err(bc->dev, "failed to set icc bw\n"); Don't we need to "release" the bandwidth on power off ? > + > /* disable upstream clocks */ > clk_bulk_disable_unprepare(data->num_clks, domain->clks); > > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev) > for (j = 0; j < data->num_clks; j++) > domain->clks[j].id = data->clk_names[j]; > > + for (j = 0; j < data->num_paths; j++) { > + domain->paths[j].name = data->path_names[j]; > + domain->paths[j].avg_bw = INT_MAX; > + domain->paths[j].peak_bw = INT_MAX; That's harsh :-) I suppose it's good enough to start with, but I wonder if we'll need more control later. > + } > + > + ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths); > + if (ret) { > + dev_err_probe(dev, ret, "failed to get noc entries\n"); > + goto cleanup_pds; > + } > + > ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks); > if (ret) { > dev_err_probe(dev, ret, "failed to get clock\n"); > @@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "lcdif1", > .rst_mask = BIT(4) | BIT(5) | BIT(23), > .clk_mask = BIT(4) | BIT(5) | BIT(23), > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > + .num_paths = 2, > }, > [IMX8MP_MEDIABLK_PD_ISI] = { > .name = "mediablk-isi", > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "isi", > .rst_mask = BIT(6) | BIT(7), > .clk_mask = BIT(6) | BIT(7), > + .path_names = (const char *[]){"isi0", "isi1", "isi2"}, > + .num_paths = 3, > }, > [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = { > .name = "mediablk-mipi-csi2-2", > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "lcdif2", > .rst_mask = BIT(11) | BIT(12) | BIT(24), > .clk_mask = BIT(11) | BIT(12) | BIT(24), > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > + .num_paths = 2, > }, > [IMX8MP_MEDIABLK_PD_ISP] = { > .name = "mediablk-isp", > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "isp", > .rst_mask = BIT(16) | BIT(17) | BIT(18), > .clk_mask = BIT(16) | BIT(17) | BIT(18), > + .path_names = (const char *[]){"isp0", "isp1"}, > + .num_paths = 2, As far as I understand, there's a single power domain for both ISP instances (I'm not entirely sure this is on purpose or a design mistake, but that's another story), but one interconnect path for each ISP instance. Would there be any use case for controlling them separately ? I can't really think of any myself. > }, > [IMX8MP_MEDIABLK_PD_DWE] = { > .name = "mediablk-dwe", > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "dwe", > .rst_mask = BIT(19) | BIT(20) | BIT(21), > .clk_mask = BIT(19) | BIT(20) | BIT(21), > + .path_names = (const char *[]){"dwe"}, > + .num_paths = 1, > }, > [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = { > .name = "mediablk-mipi-dsi-2",
> Subject: Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl > > Hi Peng, > > Thank you for the patch. > > On Wed, Jun 01, 2022 at 05:45:33PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Add interconnect paths for i.MX8MP media blk ctrl > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 1 + > > drivers/soc/imx/imx8m-blk-ctrl.c | 31 > +++++++++++++++++++++++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > index d9542dfff83f..2a1c6ff37e03 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > @@ -4,6 +4,7 @@ > > */ > > > > #include <dt-bindings/clock/imx8mp-clock.h> > > +#include <dt-bindings/interconnect/fsl,imx8mp.h> > > #include <dt-bindings/power/imx8mp-power.h> > > #include <dt-bindings/gpio/gpio.h> > > #include <dt-bindings/input/input.h> > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c > > b/drivers/soc/imx/imx8m-blk-ctrl.c > > index 7f49385ed2f8..423cac0c9cb6 100644 > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > > @@ -5,6 +5,7 @@ > > */ > > > > #include <linux/device.h> > > +#include <linux/interconnect.h> > > #include <linux/module.h> > > #include <linux/of_device.h> > > #include <linux/platform_device.h> > > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data { > > const char *name; > > const char * const *clk_names; > > int num_clks; > > + const char * const *path_names; > > + int num_paths; > > const char *gpc_name; > > u32 rst_mask; > > u32 clk_mask; > > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data { }; > > > > #define DOMAIN_MAX_CLKS 4 > > +#define DOMAIN_MAX_PATHS 4 > > > > struct imx8m_blk_ctrl_domain { > > struct generic_pm_domain genpd; > > const struct imx8m_blk_ctrl_domain_data *data; > > struct clk_bulk_data clks[DOMAIN_MAX_CLKS]; > > + struct icc_bulk_data paths[DOMAIN_MAX_PATHS]; > > struct device *power_dev; > > struct imx8m_blk_ctrl *bc; > > }; > > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct > generic_pm_domain *genpd) > > if (data->mipi_phy_rst_mask) > > regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, > > data->mipi_phy_rst_mask); > > > > + ret = icc_bulk_set_bw(data->num_paths, domain->paths); > > + if (ret) > > + dev_err(bc->dev, "failed to set icc bw\n"); > > Don't we need to "release" the bandwidth on power off ? In the NXP ATF, the NoC settings are only configured during power on. So I just follow ATF implementation. Per IC design team, SW only get a value when power up. So I would not diverge here. If bandwidth in NoC registers are taken into control, release is needed. > > > + > > /* disable upstream clocks */ > > clk_bulk_disable_unprepare(data->num_clks, domain->clks); > > > > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct > platform_device *pdev) > > for (j = 0; j < data->num_clks; j++) > > domain->clks[j].id = data->clk_names[j]; > > > > + for (j = 0; j < data->num_paths; j++) { > > + domain->paths[j].name = data->path_names[j]; > > + domain->paths[j].avg_bw = INT_MAX; > > + domain->paths[j].peak_bw = INT_MAX; > > That's harsh :-) I suppose it's good enough to start with, but I wonder if we'll > need more control later. The current settings are only for priority, ext_control, mode settings. Actually bandwidth settings are not provided. The value here is just to make icc_bulk_set_bw could configure HW. Yes, the values are not very good, we could step by step to move on to make better. > > > + } > > + > > + ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths); > > + if (ret) { > > + dev_err_probe(dev, ret, "failed to get noc entries\n"); > > + goto cleanup_pds; > > + } > > + > > ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks); > > if (ret) { > > dev_err_probe(dev, ret, "failed to get clock\n"); @@ -647,6 > +668,8 > > @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "lcdif1", > > .rst_mask = BIT(4) | BIT(5) | BIT(23), > > .clk_mask = BIT(4) | BIT(5) | BIT(23), > > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > > + .num_paths = 2, > > }, > > [IMX8MP_MEDIABLK_PD_ISI] = { > > .name = "mediablk-isi", > > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "isi", > > .rst_mask = BIT(6) | BIT(7), > > .clk_mask = BIT(6) | BIT(7), > > + .path_names = (const char *[]){"isi0", "isi1", "isi2"}, > > + .num_paths = 3, > > }, > > [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = { > > .name = "mediablk-mipi-csi2-2", > > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "lcdif2", > > .rst_mask = BIT(11) | BIT(12) | BIT(24), > > .clk_mask = BIT(11) | BIT(12) | BIT(24), > > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > > + .num_paths = 2, > > }, > > [IMX8MP_MEDIABLK_PD_ISP] = { > > .name = "mediablk-isp", > > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "isp", > > .rst_mask = BIT(16) | BIT(17) | BIT(18), > > .clk_mask = BIT(16) | BIT(17) | BIT(18), > > + .path_names = (const char *[]){"isp0", "isp1"}, > > + .num_paths = 2, > > As far as I understand, there's a single power domain for both ISP instances (I'm > not entirely sure this is on purpose or a design mistake, but that's another > story), but one interconnect path for each ISP instance. Would there be any use > case for controlling them separately ? > I can't really think of any myself. I have no idea. I just follow the DoC to list them. Thanks, Peng. > > > }, > > [IMX8MP_MEDIABLK_PD_DWE] = { > > .name = "mediablk-dwe", > > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "dwe", > > .rst_mask = BIT(19) | BIT(20) | BIT(21), > > .clk_mask = BIT(19) | BIT(20) | BIT(21), > > + .path_names = (const char *[]){"dwe"}, > > + .num_paths = 1, > > }, > > [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = { > > .name = "mediablk-mipi-dsi-2", > > -- > Regards, > > Laurent Pinchart
Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS): > From: Peng Fan <peng.fan@nxp.com> > > Add interconnect paths for i.MX8MP media blk ctrl > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 1 + > drivers/soc/imx/imx8m-blk-ctrl.c | 31 +++++++++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > index d9542dfff83f..2a1c6ff37e03 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > @@ -4,6 +4,7 @@ > */ > > #include <dt-bindings/clock/imx8mp-clock.h> > +#include <dt-bindings/interconnect/fsl,imx8mp.h> > #include <dt-bindings/power/imx8mp-power.h> > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/input/input.h> > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > index 7f49385ed2f8..423cac0c9cb6 100644 > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > @@ -5,6 +5,7 @@ > */ > > #include <linux/device.h> > +#include <linux/interconnect.h> > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data { > const char *name; > const char * const *clk_names; > int num_clks; > + const char * const *path_names; > + int num_paths; > const char *gpc_name; > u32 rst_mask; > u32 clk_mask; > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data { > }; > > #define DOMAIN_MAX_CLKS 4 > +#define DOMAIN_MAX_PATHS 4 > > struct imx8m_blk_ctrl_domain { > struct generic_pm_domain genpd; > const struct imx8m_blk_ctrl_domain_data *data; > struct clk_bulk_data clks[DOMAIN_MAX_CLKS]; > + struct icc_bulk_data paths[DOMAIN_MAX_PATHS]; > struct device *power_dev; > struct imx8m_blk_ctrl *bc; > }; > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd) > if (data->mipi_phy_rst_mask) > regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask); > > + ret = icc_bulk_set_bw(data->num_paths, domain->paths); > + if (ret) > + dev_err(bc->dev, "failed to set icc bw\n"); > + > /* disable upstream clocks */ > clk_bulk_disable_unprepare(data->num_clks, domain->clks); > > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev) > for (j = 0; j < data->num_clks; j++) > domain->clks[j].id = data->clk_names[j]; > > + for (j = 0; j < data->num_paths; j++) { > + domain->paths[j].name = data->path_names[j]; > + domain->paths[j].avg_bw = INT_MAX; > + domain->paths[j].peak_bw = INT_MAX; > + } > + > + ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths); > + if (ret) { > + dev_err_probe(dev, ret, "failed to get noc entries\n"); I don't like that this introduces a new requirement to the kernel config and DT, which is a backwards compat breaking change. Now one could argue that the NoC configuration is pretty critical and should not be skipped, but it has the potential to break currently working systems. I think it would be better do a if (ret != -EPROBE_DEFER) dev_warn_once(dev, "Could not get interconnect paths, NoC will stay unconfigured!\n"); here and ignore the error to allow the blk-ctrl to probe even if the interconnect paths couldn't be found due to lacking kernel config or DT. Regards, Lucas > + goto cleanup_pds; > + } > + > ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks); > if (ret) { > dev_err_probe(dev, ret, "failed to get clock\n"); > @@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "lcdif1", > .rst_mask = BIT(4) | BIT(5) | BIT(23), > .clk_mask = BIT(4) | BIT(5) | BIT(23), > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > + .num_paths = 2, > }, > [IMX8MP_MEDIABLK_PD_ISI] = { > .name = "mediablk-isi", > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "isi", > .rst_mask = BIT(6) | BIT(7), > .clk_mask = BIT(6) | BIT(7), > + .path_names = (const char *[]){"isi0", "isi1", "isi2"}, > + .num_paths = 3, > }, > [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = { > .name = "mediablk-mipi-csi2-2", > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "lcdif2", > .rst_mask = BIT(11) | BIT(12) | BIT(24), > .clk_mask = BIT(11) | BIT(12) | BIT(24), > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > + .num_paths = 2, > }, > [IMX8MP_MEDIABLK_PD_ISP] = { > .name = "mediablk-isp", > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "isp", > .rst_mask = BIT(16) | BIT(17) | BIT(18), > .clk_mask = BIT(16) | BIT(17) | BIT(18), > + .path_names = (const char *[]){"isp0", "isp1"}, > + .num_paths = 2, > }, > [IMX8MP_MEDIABLK_PD_DWE] = { > .name = "mediablk-dwe", > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ > .gpc_name = "dwe", > .rst_mask = BIT(19) | BIT(20) | BIT(21), > .clk_mask = BIT(19) | BIT(20) | BIT(21), > + .path_names = (const char *[]){"dwe"}, > + .num_paths = 1, > }, > [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = { > .name = "mediablk-mipi-dsi-2",
> Subject: Re: [PATCH 4/8] soc: imx: add icc paths for i.MX8MP media blk ctrl > > Am Mittwoch, dem 01.06.2022 um 17:45 +0800 schrieb Peng Fan (OSS): > > From: Peng Fan <peng.fan@nxp.com> > > > > Add interconnect paths for i.MX8MP media blk ctrl > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 1 + > > drivers/soc/imx/imx8m-blk-ctrl.c | 31 +++++++++++++++++++++++ > > 2 files changed, 32 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > index d9542dfff83f..2a1c6ff37e03 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi > > @@ -4,6 +4,7 @@ > > */ > > > > #include <dt-bindings/clock/imx8mp-clock.h> > > +#include <dt-bindings/interconnect/fsl,imx8mp.h> > > #include <dt-bindings/power/imx8mp-power.h> > > #include <dt-bindings/gpio/gpio.h> > > #include <dt-bindings/input/input.h> > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c > > b/drivers/soc/imx/imx8m-blk-ctrl.c > > index 7f49385ed2f8..423cac0c9cb6 100644 > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > > @@ -5,6 +5,7 @@ > > */ > > > > #include <linux/device.h> > > +#include <linux/interconnect.h> > > #include <linux/module.h> > > #include <linux/of_device.h> > > #include <linux/platform_device.h> > > @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data { > > const char *name; > > const char * const *clk_names; > > int num_clks; > > + const char * const *path_names; > > + int num_paths; > > const char *gpc_name; > > u32 rst_mask; > > u32 clk_mask; > > @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data { }; > > > > #define DOMAIN_MAX_CLKS 4 > > +#define DOMAIN_MAX_PATHS 4 > > > > struct imx8m_blk_ctrl_domain { > > struct generic_pm_domain genpd; > > const struct imx8m_blk_ctrl_domain_data *data; > > struct clk_bulk_data clks[DOMAIN_MAX_CLKS]; > > + struct icc_bulk_data paths[DOMAIN_MAX_PATHS]; > > struct device *power_dev; > > struct imx8m_blk_ctrl *bc; > > }; > > @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct > generic_pm_domain *genpd) > > if (data->mipi_phy_rst_mask) > > regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, > > data->mipi_phy_rst_mask); > > > > + ret = icc_bulk_set_bw(data->num_paths, domain->paths); > > + if (ret) > > + dev_err(bc->dev, "failed to set icc bw\n"); > > + > > /* disable upstream clocks */ > > clk_bulk_disable_unprepare(data->num_clks, domain->clks); > > > > @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct > platform_device *pdev) > > for (j = 0; j < data->num_clks; j++) > > domain->clks[j].id = data->clk_names[j]; > > > > + for (j = 0; j < data->num_paths; j++) { > > + domain->paths[j].name = data->path_names[j]; > > + domain->paths[j].avg_bw = INT_MAX; > > + domain->paths[j].peak_bw = INT_MAX; > > + } > > + > > + ret = devm_of_icc_bulk_get(dev, data->num_paths, > domain->paths); > > + if (ret) { > > + dev_err_probe(dev, ret, "failed to get noc > entries\n"); > > I don't like that this introduces a new requirement to the kernel config and > DT, which is a backwards compat breaking change. Now one could argue > that the NoC configuration is pretty critical and should not be skipped, but it > has the potential to break currently working systems. > > I think it would be better do a > if (ret != -EPROBE_DEFER) > dev_warn_once(dev, "Could not get interconnect paths, NoC will > stay unconfigured!\n"); > > here and ignore the error to allow the blk-ctrl to probe even if the > interconnect paths couldn't be found due to lacking kernel config or DT. [Peng Fan] Good point. So If (ret && ret != -EPROBE_DEFER) dev_warn_once(dev, "Could not get interconnect paths, NoC will stay unconfigured!\n"); Thanks, Peng. > > Regards, > Lucas > > > + goto cleanup_pds; > > + } > > + > > ret = devm_clk_bulk_get(dev, data->num_clks, domain- > >clks); > > if (ret) { > > dev_err_probe(dev, ret, "failed to get clock\n"); @@ > -647,6 +668,8 > > @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "lcdif1", > > .rst_mask = BIT(4) | BIT(5) | BIT(23), > > .clk_mask = BIT(4) | BIT(5) | BIT(23), > > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > > + .num_paths = 2, > > }, > > [IMX8MP_MEDIABLK_PD_ISI] = { > > .name = "mediablk-isi", > > @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "isi", > > .rst_mask = BIT(6) | BIT(7), > > .clk_mask = BIT(6) | BIT(7), > > + .path_names = (const char *[]){"isi0", "isi1", "isi2"}, > > + .num_paths = 3, > > }, > > [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = { > > .name = "mediablk-mipi-csi2-2", > > @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "lcdif2", > > .rst_mask = BIT(11) | BIT(12) | BIT(24), > > .clk_mask = BIT(11) | BIT(12) | BIT(24), > > + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, > > + .num_paths = 2, > > }, > > [IMX8MP_MEDIABLK_PD_ISP] = { > > .name = "mediablk-isp", > > @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "isp", > > .rst_mask = BIT(16) | BIT(17) | BIT(18), > > .clk_mask = BIT(16) | BIT(17) | BIT(18), > > + .path_names = (const char *[]){"isp0", "isp1"}, > > + .num_paths = 2, > > }, > > [IMX8MP_MEDIABLK_PD_DWE] = { > > .name = "mediablk-dwe", > > @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data > imx8mp_media_blk_ctl_domain_data[ > > .gpc_name = "dwe", > > .rst_mask = BIT(19) | BIT(20) | BIT(21), > > .clk_mask = BIT(19) | BIT(20) | BIT(21), > > + .path_names = (const char *[]){"dwe"}, > > + .num_paths = 1, > > }, > > [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = { > > .name = "mediablk-mipi-dsi-2", >
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index d9542dfff83f..2a1c6ff37e03 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi @@ -4,6 +4,7 @@ */ #include <dt-bindings/clock/imx8mp-clock.h> +#include <dt-bindings/interconnect/fsl,imx8mp.h> #include <dt-bindings/power/imx8mp-power.h> #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/input/input.h> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c index 7f49385ed2f8..423cac0c9cb6 100644 --- a/drivers/soc/imx/imx8m-blk-ctrl.c +++ b/drivers/soc/imx/imx8m-blk-ctrl.c @@ -5,6 +5,7 @@ */ #include <linux/device.h> +#include <linux/interconnect.h> #include <linux/module.h> #include <linux/of_device.h> #include <linux/platform_device.h> @@ -37,6 +38,8 @@ struct imx8m_blk_ctrl_domain_data { const char *name; const char * const *clk_names; int num_clks; + const char * const *path_names; + int num_paths; const char *gpc_name; u32 rst_mask; u32 clk_mask; @@ -52,11 +55,13 @@ struct imx8m_blk_ctrl_domain_data { }; #define DOMAIN_MAX_CLKS 4 +#define DOMAIN_MAX_PATHS 4 struct imx8m_blk_ctrl_domain { struct generic_pm_domain genpd; const struct imx8m_blk_ctrl_domain_data *data; struct clk_bulk_data clks[DOMAIN_MAX_CLKS]; + struct icc_bulk_data paths[DOMAIN_MAX_PATHS]; struct device *power_dev; struct imx8m_blk_ctrl *bc; }; @@ -117,6 +122,10 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd) if (data->mipi_phy_rst_mask) regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask); + ret = icc_bulk_set_bw(data->num_paths, domain->paths); + if (ret) + dev_err(bc->dev, "failed to set icc bw\n"); + /* disable upstream clocks */ clk_bulk_disable_unprepare(data->num_clks, domain->clks); @@ -228,6 +237,18 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev) for (j = 0; j < data->num_clks; j++) domain->clks[j].id = data->clk_names[j]; + for (j = 0; j < data->num_paths; j++) { + domain->paths[j].name = data->path_names[j]; + domain->paths[j].avg_bw = INT_MAX; + domain->paths[j].peak_bw = INT_MAX; + } + + ret = devm_of_icc_bulk_get(dev, data->num_paths, domain->paths); + if (ret) { + dev_err_probe(dev, ret, "failed to get noc entries\n"); + goto cleanup_pds; + } + ret = devm_clk_bulk_get(dev, data->num_clks, domain->clks); if (ret) { dev_err_probe(dev, ret, "failed to get clock\n"); @@ -647,6 +668,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ .gpc_name = "lcdif1", .rst_mask = BIT(4) | BIT(5) | BIT(23), .clk_mask = BIT(4) | BIT(5) | BIT(23), + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, + .num_paths = 2, }, [IMX8MP_MEDIABLK_PD_ISI] = { .name = "mediablk-isi", @@ -655,6 +678,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ .gpc_name = "isi", .rst_mask = BIT(6) | BIT(7), .clk_mask = BIT(6) | BIT(7), + .path_names = (const char *[]){"isi0", "isi1", "isi2"}, + .num_paths = 3, }, [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = { .name = "mediablk-mipi-csi2-2", @@ -672,6 +697,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ .gpc_name = "lcdif2", .rst_mask = BIT(11) | BIT(12) | BIT(24), .clk_mask = BIT(11) | BIT(12) | BIT(24), + .path_names = (const char *[]){"lcdif-rd", "lcdif-wr"}, + .num_paths = 2, }, [IMX8MP_MEDIABLK_PD_ISP] = { .name = "mediablk-isp", @@ -680,6 +707,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ .gpc_name = "isp", .rst_mask = BIT(16) | BIT(17) | BIT(18), .clk_mask = BIT(16) | BIT(17) | BIT(18), + .path_names = (const char *[]){"isp0", "isp1"}, + .num_paths = 2, }, [IMX8MP_MEDIABLK_PD_DWE] = { .name = "mediablk-dwe", @@ -688,6 +717,8 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[ .gpc_name = "dwe", .rst_mask = BIT(19) | BIT(20) | BIT(21), .clk_mask = BIT(19) | BIT(20) | BIT(21), + .path_names = (const char *[]){"dwe"}, + .num_paths = 1, }, [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = { .name = "mediablk-mipi-dsi-2",