| Message ID | 20210407073534.376722-4-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add HANTRO G2/HEVC decoder support for IMX8MQ | expand |
Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > In order to be able to share the control hardware block between > VPUs use a syscon instead a ioremap it in the driver. > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > phandle is not found look at 'ctrl' reg-name. > With the method it becomes useless to provide a list of register > names so remove it. Sorry for putting a spoke in the wheel after many iterations of the series. We just discussed a way forward on how to handle the clocks and resets provided by the blkctl block on i.MX8MM and later and it seems there is a consensus on trying to provide virtual power domains from a blkctl driver, controlling clocks and resets for the devices in the power domain. I would like to avoid introducing yet another way of handling the blkctl and thus would like to align the i.MX8MQ VPU blkctl with what we are planning to do on the later chip generations. CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this virtual power domain thing a shot. Regards, Lucas > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > version 9: > - Corrections in commit message > > version 7: > - Add Philipp reviewed-by tag. > - Change syscon phandle name. > > > > > version 5: > - use syscon instead of VPU reset driver. > - if DT doesn't provide syscon keep backward compatibilty by using > 'ctrl' reg-name. > > drivers/staging/media/hantro/hantro.h | 5 +- > drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 ++++++++++++--------- > 2 files changed, 34 insertions(+), 23 deletions(-) > > diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h > index 6c1b888abe75..37b9ce04bd4e 100644 > --- a/drivers/staging/media/hantro/hantro.h > +++ b/drivers/staging/media/hantro/hantro.h > @@ -13,6 +13,7 @@ > #define HANTRO_H_ > > > > > #include <linux/platform_device.h> > +#include <linux/regmap.h> > #include <linux/videodev2.h> > #include <linux/wait.h> > #include <linux/clk.h> > @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) > * @reg_bases: Mapped addresses of VPU registers. > * @enc_base: Mapped address of VPU encoder register for convenience. > * @dec_base: Mapped address of VPU decoder register for convenience. > - * @ctrl_base: Mapped address of VPU control block. > + * @ctrl_base: Regmap of VPU control block. > * @vpu_mutex: Mutex to synchronize V4L2 calls. > * @irqlock: Spinlock to synchronize access to data structures > * shared with interrupt handlers. > @@ -186,7 +187,7 @@ struct hantro_dev { > void __iomem **reg_bases; > void __iomem *enc_base; > void __iomem *dec_base; > - void __iomem *ctrl_base; > + struct regmap *ctrl_base; > > > > > struct mutex vpu_mutex; /* video_device lock */ > spinlock_t irqlock; > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c > index c222de075ef4..8d0c3425234b 100644 > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c > @@ -7,6 +7,7 @@ > > > > > #include <linux/clk.h> > #include <linux/delay.h> > +#include <linux/mfd/syscon.h> > > > > > #include "hantro.h" > #include "hantro_jpeg.h" > @@ -24,30 +25,28 @@ > #define CTRL_G1_PP_FUSE 0x0c > #define CTRL_G2_DEC_FUSE 0x10 > > > > > +static const struct regmap_config ctrl_regmap_ctrl = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 0x14, > +}; > + > static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) > { > - u32 val; > - > /* Assert */ > - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); > - val &= ~reset_bits; > - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); > + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); > > > > > udelay(2); > > > > > /* Release */ > - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); > - val |= reset_bits; > - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); > + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, > + reset_bits, reset_bits); > } > > > > > static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) > { > - u32 val; > - > - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); > - val |= clock_bits; > - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); > + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, > + clock_bits, clock_bits); > } > > > > > static int imx8mq_runtime_resume(struct hantro_dev *vpu) > @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) > imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); > > > > > /* Set values of the fuse registers */ > - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); > - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); > - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); > + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); > + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); > + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); > > > > > clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); > > > > > @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) > > > > > static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) > { > - vpu->dec_base = vpu->reg_bases[0]; > - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; > + struct device_node *np = vpu->dev->of_node; > + > + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); > + if (IS_ERR(vpu->ctrl_base)) { > + struct resource *res; > + void __iomem *ctrl; > + > + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); > + ctrl = devm_ioremap_resource(vpu->dev, res); > + if (IS_ERR(ctrl)) > + return PTR_ERR(ctrl); > + > + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); > + if (IS_ERR(vpu->ctrl_base)) > + return PTR_ERR(vpu->ctrl_base); > + } > > > > > return 0; > } > @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { > }; > > > > > static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; > -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; > > > > > const struct hantro_variant imx8mq_vpu_variant = { > .dec_fmts = imx8m_vpu_dec_fmts, > @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { > .num_irqs = ARRAY_SIZE(imx8mq_irqs), > .clk_names = imx8mq_clk_names, > .num_clocks = ARRAY_SIZE(imx8mq_clk_names), > - .reg_names = imx8mq_reg_names, > - .num_regs = ARRAY_SIZE(imx8mq_reg_names) > };
Le 16/04/2021 à 12:54, Lucas Stach a écrit : > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >> In order to be able to share the control hardware block between >> VPUs use a syscon instead a ioremap it in the driver. >> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >> phandle is not found look at 'ctrl' reg-name. >> With the method it becomes useless to provide a list of register >> names so remove it. > Sorry for putting a spoke in the wheel after many iterations of the > series. > > We just discussed a way forward on how to handle the clocks and resets > provided by the blkctl block on i.MX8MM and later and it seems there is > a consensus on trying to provide virtual power domains from a blkctl > driver, controlling clocks and resets for the devices in the power > domain. I would like to avoid introducing yet another way of handling > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > what we are planning to do on the later chip generations. > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > virtual power domain thing a shot. That could replace the 3 first patches and Dt patche of this series but that will not impact the hevc part, so I wonder if pure hevc patches could be merged anyway ? They are reviewed and don't depend of how the ctrl block is managed. Regards, Benjamin > > Regards, > Lucas > >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> >> --- >> version 9: >> - Corrections in commit message >> >> version 7: >> - Add Philipp reviewed-by tag. >> - Change syscon phandle name. >> >> >> >> >> version 5: >> - use syscon instead of VPU reset driver. >> - if DT doesn't provide syscon keep backward compatibilty by using >> 'ctrl' reg-name. >> >> drivers/staging/media/hantro/hantro.h | 5 +- >> drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 ++++++++++++--------- >> 2 files changed, 34 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h >> index 6c1b888abe75..37b9ce04bd4e 100644 >> --- a/drivers/staging/media/hantro/hantro.h >> +++ b/drivers/staging/media/hantro/hantro.h >> @@ -13,6 +13,7 @@ >> #define HANTRO_H_ >> >> >> >> >> #include <linux/platform_device.h> >> +#include <linux/regmap.h> >> #include <linux/videodev2.h> >> #include <linux/wait.h> >> #include <linux/clk.h> >> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) >> * @reg_bases: Mapped addresses of VPU registers. >> * @enc_base: Mapped address of VPU encoder register for convenience. >> * @dec_base: Mapped address of VPU decoder register for convenience. >> - * @ctrl_base: Mapped address of VPU control block. >> + * @ctrl_base: Regmap of VPU control block. >> * @vpu_mutex: Mutex to synchronize V4L2 calls. >> * @irqlock: Spinlock to synchronize access to data structures >> * shared with interrupt handlers. >> @@ -186,7 +187,7 @@ struct hantro_dev { >> void __iomem **reg_bases; >> void __iomem *enc_base; >> void __iomem *dec_base; >> - void __iomem *ctrl_base; >> + struct regmap *ctrl_base; >> >> >> >> >> struct mutex vpu_mutex; /* video_device lock */ >> spinlock_t irqlock; >> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c >> index c222de075ef4..8d0c3425234b 100644 >> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c >> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c >> @@ -7,6 +7,7 @@ >> >> >> >> >> #include <linux/clk.h> >> #include <linux/delay.h> >> +#include <linux/mfd/syscon.h> >> >> >> >> >> #include "hantro.h" >> #include "hantro_jpeg.h" >> @@ -24,30 +25,28 @@ >> #define CTRL_G1_PP_FUSE 0x0c >> #define CTRL_G2_DEC_FUSE 0x10 >> >> >> >> >> +static const struct regmap_config ctrl_regmap_ctrl = { >> + .reg_bits = 32, >> + .val_bits = 32, >> + .reg_stride = 0x14, >> +}; >> + >> static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) >> { >> - u32 val; >> - >> /* Assert */ >> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >> - val &= ~reset_bits; >> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); >> >> >> >> >> udelay(2); >> >> >> >> >> /* Release */ >> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >> - val |= reset_bits; >> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, >> + reset_bits, reset_bits); >> } >> >> >> >> >> static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) >> { >> - u32 val; >> - >> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); >> - val |= clock_bits; >> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); >> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, >> + clock_bits, clock_bits); >> } >> >> >> >> >> static int imx8mq_runtime_resume(struct hantro_dev *vpu) >> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) >> imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); >> >> >> >> >> /* Set values of the fuse registers */ >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); >> + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); >> + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); >> + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); >> >> >> >> >> clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); >> >> >> >> >> @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) >> >> >> >> >> static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) >> { >> - vpu->dec_base = vpu->reg_bases[0]; >> - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; >> + struct device_node *np = vpu->dev->of_node; >> + >> + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); >> + if (IS_ERR(vpu->ctrl_base)) { >> + struct resource *res; >> + void __iomem *ctrl; >> + >> + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); >> + ctrl = devm_ioremap_resource(vpu->dev, res); >> + if (IS_ERR(ctrl)) >> + return PTR_ERR(ctrl); >> + >> + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); >> + if (IS_ERR(vpu->ctrl_base)) >> + return PTR_ERR(vpu->ctrl_base); >> + } >> >> >> >> >> return 0; >> } >> @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { >> }; >> >> >> >> >> static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; >> -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; >> >> >> >> >> const struct hantro_variant imx8mq_vpu_variant = { >> .dec_fmts = imx8m_vpu_dec_fmts, >> @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { >> .num_irqs = ARRAY_SIZE(imx8mq_irqs), >> .clk_names = imx8mq_clk_names, >> .num_clocks = ARRAY_SIZE(imx8mq_clk_names), >> - .reg_names = imx8mq_reg_names, >> - .num_regs = ARRAY_SIZE(imx8mq_reg_names) >> }; > >
Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard: > Le 16/04/2021 à 12:54, Lucas Stach a écrit : > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > In order to be able to share the control hardware block between > > > VPUs use a syscon instead a ioremap it in the driver. > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > phandle is not found look at 'ctrl' reg-name. > > > With the method it becomes useless to provide a list of register > > > names so remove it. > > Sorry for putting a spoke in the wheel after many iterations of the > > series. > > > > We just discussed a way forward on how to handle the clocks and resets > > provided by the blkctl block on i.MX8MM and later and it seems there is > > a consensus on trying to provide virtual power domains from a blkctl > > driver, controlling clocks and resets for the devices in the power > > domain. I would like to avoid introducing yet another way of handling > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > what we are planning to do on the later chip generations. > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > virtual power domain thing a shot. > > That could replace the 3 first patches and Dt patche of this series > but that will not impact the hevc part, so I wonder if pure hevc patches > could be merged anyway ? > They are reviewed and don't depend of how the ctrl block is managed. I'm not really in a position to give any informed opinion about that hvec patches, as I only skimmed them, but I don't see any reason to delay patches 04-11 from this series until the i.MX8M platform issues are sorted. AFAICS those things are totally orthogonal. Regards, Lucas > > > > Regards, > > Lucas > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > > > Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> > > > --- > > > version 9: > > > - Corrections in commit message > > > > > > version 7: > > > - Add Philipp reviewed-by tag. > > > - Change syscon phandle name. > > > > > > > > > > > > > > > > > > > > > > > > version 5: > > > - use syscon instead of VPU reset driver. > > > - if DT doesn't provide syscon keep backward compatibilty by using > > > 'ctrl' reg-name. > > > > > > drivers/staging/media/hantro/hantro.h | 5 +- > > > drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 ++++++++++++--------- > > > 2 files changed, 34 insertions(+), 23 deletions(-) > > > > > > diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h > > > index 6c1b888abe75..37b9ce04bd4e 100644 > > > --- a/drivers/staging/media/hantro/hantro.h > > > +++ b/drivers/staging/media/hantro/hantro.h > > > @@ -13,6 +13,7 @@ > > > #define HANTRO_H_ > > > > > > > > > > > > > > > > > > > > > > > > #include <linux/platform_device.h> > > > +#include <linux/regmap.h> > > > #include <linux/videodev2.h> > > > #include <linux/wait.h> > > > #include <linux/clk.h> > > > @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) > > > * @reg_bases: Mapped addresses of VPU registers. > > > * @enc_base: Mapped address of VPU encoder register for convenience. > > > * @dec_base: Mapped address of VPU decoder register for convenience. > > > - * @ctrl_base: Mapped address of VPU control block. > > > + * @ctrl_base: Regmap of VPU control block. > > > * @vpu_mutex: Mutex to synchronize V4L2 calls. > > > * @irqlock: Spinlock to synchronize access to data structures > > > * shared with interrupt handlers. > > > @@ -186,7 +187,7 @@ struct hantro_dev { > > > void __iomem **reg_bases; > > > void __iomem *enc_base; > > > void __iomem *dec_base; > > > - void __iomem *ctrl_base; > > > + struct regmap *ctrl_base; > > > > > > > > > > > > > > > > > > > > > > > > struct mutex vpu_mutex; /* video_device lock */ > > > spinlock_t irqlock; > > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c > > > index c222de075ef4..8d0c3425234b 100644 > > > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c > > > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c > > > @@ -7,6 +7,7 @@ > > > > > > > > > > > > > > > > > > > > > > > > #include <linux/clk.h> > > > #include <linux/delay.h> > > > +#include <linux/mfd/syscon.h> > > > > > > > > > > > > > > > > > > > > > > > > #include "hantro.h" > > > #include "hantro_jpeg.h" > > > @@ -24,30 +25,28 @@ > > > #define CTRL_G1_PP_FUSE 0x0c > > > #define CTRL_G2_DEC_FUSE 0x10 > > > > > > > > > > > > > > > > > > > > > > > > +static const struct regmap_config ctrl_regmap_ctrl = { > > > + .reg_bits = 32, > > > + .val_bits = 32, > > > + .reg_stride = 0x14, > > > +}; > > > + > > > static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) > > > { > > > - u32 val; > > > - > > > /* Assert */ > > > - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); > > > - val &= ~reset_bits; > > > - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); > > > + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); > > > > > > > > > > > > > > > > > > > > > > > > udelay(2); > > > > > > > > > > > > > > > > > > > > > > > > /* Release */ > > > - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); > > > - val |= reset_bits; > > > - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); > > > + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, > > > + reset_bits, reset_bits); > > > } > > > > > > > > > > > > > > > > > > > > > > > > static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) > > > { > > > - u32 val; > > > - > > > - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); > > > - val |= clock_bits; > > > - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); > > > + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, > > > + clock_bits, clock_bits); > > > } > > > > > > > > > > > > > > > > > > > > > > > > static int imx8mq_runtime_resume(struct hantro_dev *vpu) > > > @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) > > > imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); > > > > > > > > > > > > > > > > > > > > > > > > /* Set values of the fuse registers */ > > > - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); > > > - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); > > > - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); > > > + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); > > > + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); > > > + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); > > > > > > > > > > > > > > > > > > > > > > > > clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); > > > > > > > > > > > > > > > > > > > > > > > > @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) > > > > > > > > > > > > > > > > > > > > > > > > static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) > > > { > > > - vpu->dec_base = vpu->reg_bases[0]; > > > - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; > > > + struct device_node *np = vpu->dev->of_node; > > > + > > > + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); > > > + if (IS_ERR(vpu->ctrl_base)) { > > > + struct resource *res; > > > + void __iomem *ctrl; > > > + > > > + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); > > > + ctrl = devm_ioremap_resource(vpu->dev, res); > > > + if (IS_ERR(ctrl)) > > > + return PTR_ERR(ctrl); > > > + > > > + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); > > > + if (IS_ERR(vpu->ctrl_base)) > > > + return PTR_ERR(vpu->ctrl_base); > > > + } > > > > > > > > > > > > > > > > > > > > > > > > return 0; > > > } > > > @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { > > > }; > > > > > > > > > > > > > > > > > > > > > > > > static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; > > > -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; > > > > > > > > > > > > > > > > > > > > > > > > const struct hantro_variant imx8mq_vpu_variant = { > > > .dec_fmts = imx8m_vpu_dec_fmts, > > > @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { > > > .num_irqs = ARRAY_SIZE(imx8mq_irqs), > > > .clk_names = imx8mq_clk_names, > > > .num_clocks = ARRAY_SIZE(imx8mq_clk_names), > > > - .reg_names = imx8mq_reg_names, > > > - .num_regs = ARRAY_SIZE(imx8mq_reg_names) > > > }; > > > > >
Le 16/04/2021 à 17:14, Lucas Stach a écrit : > Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard: >> Le 16/04/2021 à 12:54, Lucas Stach a écrit : >>> Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >>>> In order to be able to share the control hardware block between >>>> VPUs use a syscon instead a ioremap it in the driver. >>>> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >>>> phandle is not found look at 'ctrl' reg-name. >>>> With the method it becomes useless to provide a list of register >>>> names so remove it. >>> Sorry for putting a spoke in the wheel after many iterations of the >>> series. >>> >>> We just discussed a way forward on how to handle the clocks and resets >>> provided by the blkctl block on i.MX8MM and later and it seems there is >>> a consensus on trying to provide virtual power domains from a blkctl >>> driver, controlling clocks and resets for the devices in the power >>> domain. I would like to avoid introducing yet another way of handling >>> the blkctl and thus would like to align the i.MX8MQ VPU blkctl with >>> what we are planning to do on the later chip generations. >>> >>> CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this >>> virtual power domain thing a shot. >> That could replace the 3 first patches and Dt patche of this series >> but that will not impact the hevc part, so I wonder if pure hevc patches >> could be merged anyway ? >> They are reviewed and don't depend of how the ctrl block is managed. > I'm not really in a position to give any informed opinion about that > hvec patches, as I only skimmed them, but I don't see any reason to > delay patches 04-11 from this series until the i.MX8M platform issues > are sorted. AFAICS those things are totally orthogonal. Hi Hans, What do you think about this proposal to split this series ? Get hevc part merged could allow me to continue to add features like scaling lists, compressed reference buffers and 10-bit supports. Regards, Benjamin > > Regards, > Lucas > >>> Regards, >>> Lucas >>> >>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >>>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> >>>> --- >>>> version 9: >>>> - Corrections in commit message >>>> >>>> version 7: >>>> - Add Philipp reviewed-by tag. >>>> - Change syscon phandle name. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> version 5: >>>> - use syscon instead of VPU reset driver. >>>> - if DT doesn't provide syscon keep backward compatibilty by using >>>> 'ctrl' reg-name. >>>> >>>> drivers/staging/media/hantro/hantro.h | 5 +- >>>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 ++++++++++++--------- >>>> 2 files changed, 34 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h >>>> index 6c1b888abe75..37b9ce04bd4e 100644 >>>> --- a/drivers/staging/media/hantro/hantro.h >>>> +++ b/drivers/staging/media/hantro/hantro.h >>>> @@ -13,6 +13,7 @@ >>>> #define HANTRO_H_ >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> #include <linux/platform_device.h> >>>> +#include <linux/regmap.h> >>>> #include <linux/videodev2.h> >>>> #include <linux/wait.h> >>>> #include <linux/clk.h> >>>> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) >>>> * @reg_bases: Mapped addresses of VPU registers. >>>> * @enc_base: Mapped address of VPU encoder register for convenience. >>>> * @dec_base: Mapped address of VPU decoder register for convenience. >>>> - * @ctrl_base: Mapped address of VPU control block. >>>> + * @ctrl_base: Regmap of VPU control block. >>>> * @vpu_mutex: Mutex to synchronize V4L2 calls. >>>> * @irqlock: Spinlock to synchronize access to data structures >>>> * shared with interrupt handlers. >>>> @@ -186,7 +187,7 @@ struct hantro_dev { >>>> void __iomem **reg_bases; >>>> void __iomem *enc_base; >>>> void __iomem *dec_base; >>>> - void __iomem *ctrl_base; >>>> + struct regmap *ctrl_base; >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> struct mutex vpu_mutex; /* video_device lock */ >>>> spinlock_t irqlock; >>>> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c >>>> index c222de075ef4..8d0c3425234b 100644 >>>> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c >>>> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c >>>> @@ -7,6 +7,7 @@ >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> #include <linux/clk.h> >>>> #include <linux/delay.h> >>>> +#include <linux/mfd/syscon.h> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> #include "hantro.h" >>>> #include "hantro_jpeg.h" >>>> @@ -24,30 +25,28 @@ >>>> #define CTRL_G1_PP_FUSE 0x0c >>>> #define CTRL_G2_DEC_FUSE 0x10 >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> +static const struct regmap_config ctrl_regmap_ctrl = { >>>> + .reg_bits = 32, >>>> + .val_bits = 32, >>>> + .reg_stride = 0x14, >>>> +}; >>>> + >>>> static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) >>>> { >>>> - u32 val; >>>> - >>>> /* Assert */ >>>> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >>>> - val &= ~reset_bits; >>>> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >>>> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> udelay(2); >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> /* Release */ >>>> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >>>> - val |= reset_bits; >>>> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >>>> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, >>>> + reset_bits, reset_bits); >>>> } >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) >>>> { >>>> - u32 val; >>>> - >>>> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); >>>> - val |= clock_bits; >>>> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); >>>> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, >>>> + clock_bits, clock_bits); >>>> } >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> static int imx8mq_runtime_resume(struct hantro_dev *vpu) >>>> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) >>>> imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> /* Set values of the fuse registers */ >>>> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); >>>> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); >>>> - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); >>>> + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); >>>> + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); >>>> + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) >>>> { >>>> - vpu->dec_base = vpu->reg_bases[0]; >>>> - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; >>>> + struct device_node *np = vpu->dev->of_node; >>>> + >>>> + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); >>>> + if (IS_ERR(vpu->ctrl_base)) { >>>> + struct resource *res; >>>> + void __iomem *ctrl; >>>> + >>>> + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); >>>> + ctrl = devm_ioremap_resource(vpu->dev, res); >>>> + if (IS_ERR(ctrl)) >>>> + return PTR_ERR(ctrl); >>>> + >>>> + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); >>>> + if (IS_ERR(vpu->ctrl_base)) >>>> + return PTR_ERR(vpu->ctrl_base); >>>> + } >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> return 0; >>>> } >>>> @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { >>>> }; >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; >>>> -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> const struct hantro_variant imx8mq_vpu_variant = { >>>> .dec_fmts = imx8m_vpu_dec_fmts, >>>> @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { >>>> .num_irqs = ARRAY_SIZE(imx8mq_irqs), >>>> .clk_names = imx8mq_clk_names, >>>> .num_clocks = ARRAY_SIZE(imx8mq_clk_names), >>>> - .reg_names = imx8mq_reg_names, >>>> - .num_regs = ARRAY_SIZE(imx8mq_reg_names) >>>> }; >>> > >
On 20/04/2021 11:10, Benjamin Gaignard wrote: > > Le 16/04/2021 à 17:14, Lucas Stach a écrit : >> Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard: >>> Le 16/04/2021 à 12:54, Lucas Stach a écrit : >>>> Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >>>>> In order to be able to share the control hardware block between >>>>> VPUs use a syscon instead a ioremap it in the driver. >>>>> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >>>>> phandle is not found look at 'ctrl' reg-name. >>>>> With the method it becomes useless to provide a list of register >>>>> names so remove it. >>>> Sorry for putting a spoke in the wheel after many iterations of the >>>> series. >>>> >>>> We just discussed a way forward on how to handle the clocks and resets >>>> provided by the blkctl block on i.MX8MM and later and it seems there is >>>> a consensus on trying to provide virtual power domains from a blkctl >>>> driver, controlling clocks and resets for the devices in the power >>>> domain. I would like to avoid introducing yet another way of handling >>>> the blkctl and thus would like to align the i.MX8MQ VPU blkctl with >>>> what we are planning to do on the later chip generations. >>>> >>>> CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this >>>> virtual power domain thing a shot. >>> That could replace the 3 first patches and Dt patche of this series >>> but that will not impact the hevc part, so I wonder if pure hevc patches >>> could be merged anyway ? >>> They are reviewed and don't depend of how the ctrl block is managed. >> I'm not really in a position to give any informed opinion about that >> hvec patches, as I only skimmed them, but I don't see any reason to >> delay patches 04-11 from this series until the i.MX8M platform issues >> are sorted. AFAICS those things are totally orthogonal. > > Hi Hans, > What do you think about this proposal to split this series ? > Get hevc part merged could allow me to continue to add features > like scaling lists, compressed reference buffers and 10-bit supports. Makes sense to me! Regards, Hans
Le 20/04/2021 à 11:16, Hans Verkuil a écrit : > On 20/04/2021 11:10, Benjamin Gaignard wrote: >> Le 16/04/2021 à 17:14, Lucas Stach a écrit : >>> Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard: >>>> Le 16/04/2021 à 12:54, Lucas Stach a écrit : >>>>> Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >>>>>> In order to be able to share the control hardware block between >>>>>> VPUs use a syscon instead a ioremap it in the driver. >>>>>> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >>>>>> phandle is not found look at 'ctrl' reg-name. >>>>>> With the method it becomes useless to provide a list of register >>>>>> names so remove it. >>>>> Sorry for putting a spoke in the wheel after many iterations of the >>>>> series. >>>>> >>>>> We just discussed a way forward on how to handle the clocks and resets >>>>> provided by the blkctl block on i.MX8MM and later and it seems there is >>>>> a consensus on trying to provide virtual power domains from a blkctl >>>>> driver, controlling clocks and resets for the devices in the power >>>>> domain. I would like to avoid introducing yet another way of handling >>>>> the blkctl and thus would like to align the i.MX8MQ VPU blkctl with >>>>> what we are planning to do on the later chip generations. >>>>> >>>>> CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this >>>>> virtual power domain thing a shot. >>>> That could replace the 3 first patches and Dt patche of this series >>>> but that will not impact the hevc part, so I wonder if pure hevc patches >>>> could be merged anyway ? >>>> They are reviewed and don't depend of how the ctrl block is managed. >>> I'm not really in a position to give any informed opinion about that >>> hvec patches, as I only skimmed them, but I don't see any reason to >>> delay patches 04-11 from this series until the i.MX8M platform issues >>> are sorted. AFAICS those things are totally orthogonal. >> Hi Hans, >> What do you think about this proposal to split this series ? >> Get hevc part merged could allow me to continue to add features >> like scaling lists, compressed reference buffers and 10-bit supports. > Makes sense to me! Great ! If the latest version match your expectations how would you like to processed ? Can you merged patches 4 to 12 ? or should I resend them in a new shorted series ? Regards, Benjamin > > Regards, > > Hans >
On 20/04/2021 11:31, Benjamin Gaignard wrote: > > Le 20/04/2021 à 11:16, Hans Verkuil a écrit : >> On 20/04/2021 11:10, Benjamin Gaignard wrote: >>> Le 16/04/2021 à 17:14, Lucas Stach a écrit : >>>> Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard: >>>>> Le 16/04/2021 à 12:54, Lucas Stach a écrit : >>>>>> Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >>>>>>> In order to be able to share the control hardware block between >>>>>>> VPUs use a syscon instead a ioremap it in the driver. >>>>>>> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >>>>>>> phandle is not found look at 'ctrl' reg-name. >>>>>>> With the method it becomes useless to provide a list of register >>>>>>> names so remove it. >>>>>> Sorry for putting a spoke in the wheel after many iterations of the >>>>>> series. >>>>>> >>>>>> We just discussed a way forward on how to handle the clocks and resets >>>>>> provided by the blkctl block on i.MX8MM and later and it seems there is >>>>>> a consensus on trying to provide virtual power domains from a blkctl >>>>>> driver, controlling clocks and resets for the devices in the power >>>>>> domain. I would like to avoid introducing yet another way of handling >>>>>> the blkctl and thus would like to align the i.MX8MQ VPU blkctl with >>>>>> what we are planning to do on the later chip generations. >>>>>> >>>>>> CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this >>>>>> virtual power domain thing a shot. >>>>> That could replace the 3 first patches and Dt patche of this series >>>>> but that will not impact the hevc part, so I wonder if pure hevc patches >>>>> could be merged anyway ? >>>>> They are reviewed and don't depend of how the ctrl block is managed. >>>> I'm not really in a position to give any informed opinion about that >>>> hvec patches, as I only skimmed them, but I don't see any reason to >>>> delay patches 04-11 from this series until the i.MX8M platform issues >>>> are sorted. AFAICS those things are totally orthogonal. >>> Hi Hans, >>> What do you think about this proposal to split this series ? >>> Get hevc part merged could allow me to continue to add features >>> like scaling lists, compressed reference buffers and 10-bit supports. >> Makes sense to me! > > Great ! > If the latest version match your expectations how would you like to processed ? > Can you merged patches 4 to 12 ? or should I resend them in a new shorted series ? A separate patch series would be easier for me. Regards, Hans > > Regards, > Benjamin > >> >> Regards, >> >> Hans >>
Hi Lucas, On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > In order to be able to share the control hardware block between > > VPUs use a syscon instead a ioremap it in the driver. > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > phandle is not found look at 'ctrl' reg-name. > > With the method it becomes useless to provide a list of register > > names so remove it. > > Sorry for putting a spoke in the wheel after many iterations of the > series. > > We just discussed a way forward on how to handle the clocks and resets > provided by the blkctl block on i.MX8MM and later and it seems there is > a consensus on trying to provide virtual power domains from a blkctl > driver, controlling clocks and resets for the devices in the power > domain. I would like to avoid introducing yet another way of handling > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > what we are planning to do on the later chip generations. > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > virtual power domain thing a shot. > It seems the i.MX8MM BLK-CTL series are moving forward: https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 ... but I'm unable to wrap my head around how this affects the devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). Can you clarify that? Thanks, Ezequiel
Hi Ezequiel, Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > Hi Lucas, > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > In order to be able to share the control hardware block between > > > VPUs use a syscon instead a ioremap it in the driver. > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > phandle is not found look at 'ctrl' reg-name. > > > With the method it becomes useless to provide a list of register > > > names so remove it. > > > > Sorry for putting a spoke in the wheel after many iterations of the > > series. > > > > We just discussed a way forward on how to handle the clocks and resets > > provided by the blkctl block on i.MX8MM and later and it seems there is > > a consensus on trying to provide virtual power domains from a blkctl > > driver, controlling clocks and resets for the devices in the power > > domain. I would like to avoid introducing yet another way of handling > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > what we are planning to do on the later chip generations. > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > virtual power domain thing a shot. > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > ... but I'm unable to wrap my head around how this affects the > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > For the i.MX8MQ we want to have the same virtual power-domains provided by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, even though the SoC integration with the blk-ctrl is a little different. > Can you clarify that? > I'm planning on sending some patches adding i.MX8MQ VPU support to the BLK-CTRL driver in the next few days. I guess that should clarify things. :) Regards, Lucas
On Mon, 2021-05-17 at 12:52 +0200, Lucas Stach wrote: > Hi Ezequiel, > > Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > > Hi Lucas, > > > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > > In order to be able to share the control hardware block between > > > > VPUs use a syscon instead a ioremap it in the driver. > > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > > phandle is not found look at 'ctrl' reg-name. > > > > With the method it becomes useless to provide a list of register > > > > names so remove it. > > > > > > Sorry for putting a spoke in the wheel after many iterations of the > > > series. > > > > > > We just discussed a way forward on how to handle the clocks and resets > > > provided by the blkctl block on i.MX8MM and later and it seems there is > > > a consensus on trying to provide virtual power domains from a blkctl > > > driver, controlling clocks and resets for the devices in the power > > > domain. I would like to avoid introducing yet another way of handling > > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > > what we are planning to do on the later chip generations. > > > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > > virtual power domain thing a shot. > > > > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > > > ... but I'm unable to wrap my head around how this affects the > > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > > > > For the i.MX8MQ we want to have the same virtual power-domains provided > by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be > able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, > even though the SoC integration with the blk-ctrl is a little > different. > AFAICS, there's not support for i.MX8MP VPU power domains. I suppose we should make sure we'll be able to cover those as well. Will i.MX8MP need its own driver as well? > > Can you clarify that? > > > I'm planning on sending some patches adding i.MX8MQ VPU support to the > BLK-CTRL driver in the next few days. I guess that should clarify > things. :) > Great. Thanks a lot, Ezequiel
Am Montag, dem 17.05.2021 um 10:23 -0300 schrieb Ezequiel Garcia: > On Mon, 2021-05-17 at 12:52 +0200, Lucas Stach wrote: > > Hi Ezequiel, > > > > Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > > > Hi Lucas, > > > > > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > > > In order to be able to share the control hardware block between > > > > > VPUs use a syscon instead a ioremap it in the driver. > > > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > > > phandle is not found look at 'ctrl' reg-name. > > > > > With the method it becomes useless to provide a list of register > > > > > names so remove it. > > > > > > > > Sorry for putting a spoke in the wheel after many iterations of the > > > > series. > > > > > > > > We just discussed a way forward on how to handle the clocks and resets > > > > provided by the blkctl block on i.MX8MM and later and it seems there is > > > > a consensus on trying to provide virtual power domains from a blkctl > > > > driver, controlling clocks and resets for the devices in the power > > > > domain. I would like to avoid introducing yet another way of handling > > > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > > > what we are planning to do on the later chip generations. > > > > > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > > > virtual power domain thing a shot. > > > > > > > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > > > > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > > > > > ... but I'm unable to wrap my head around how this affects the > > > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > > > > > > > For the i.MX8MQ we want to have the same virtual power-domains provided > > by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be > > able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, > > even though the SoC integration with the blk-ctrl is a little > > different. > > > > AFAICS, there's not support for i.MX8MP VPU power domains. I suppose > we should make sure we'll be able to cover those as well. > > Will i.MX8MP need its own driver as well? > > I haven't looked too closely at the 8MP VPU subsystem yet, but I expect it to be slightly different again so it will need changes to the blk- ctrl driver. But that's the whole point of this virtual power domain exercise: abstract away the SoC specific things in the blk-ctrl driver, so the VPU driver doesn't need to care about them. Regards, Lucas
Hi Lucas, On Mon, 2021-05-17 at 12:52 +0200, Lucas Stach wrote: > Hi Ezequiel, > > Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > > Hi Lucas, > > > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > > In order to be able to share the control hardware block between > > > > VPUs use a syscon instead a ioremap it in the driver. > > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > > phandle is not found look at 'ctrl' reg-name. > > > > With the method it becomes useless to provide a list of register > > > > names so remove it. > > > > > > Sorry for putting a spoke in the wheel after many iterations of the > > > series. > > > > > > We just discussed a way forward on how to handle the clocks and resets > > > provided by the blkctl block on i.MX8MM and later and it seems there is > > > a consensus on trying to provide virtual power domains from a blkctl > > > driver, controlling clocks and resets for the devices in the power > > > domain. I would like to avoid introducing yet another way of handling > > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > > what we are planning to do on the later chip generations. > > > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > > virtual power domain thing a shot. > > > > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > > > ... but I'm unable to wrap my head around how this affects the > > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > > > > For the i.MX8MQ we want to have the same virtual power-domains provided > by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be > able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, > even though the SoC integration with the blk-ctrl is a little > different. > > > Can you clarify that? > > > I'm planning on sending some patches adding i.MX8MQ VPU support to the > BLK-CTRL driver in the next few days. I guess that should clarify > things. :) > As a gentle reminder, Hans sent the i.MX8MQ G2 HEVC support pull request and Benjamin just posted a series adding support for more features. Do you think we could have the blk-ctrl support landing in v5.14? If you work on the patches, and you happen to test the G1 and G2 on i.MX8MM it would be great to add that too. Meanwhile, our next steps would be to improve the HEVC V4L2 uAPI itself. Thanks a lot! Ezequiel
Le 16/04/2021 à 12:54, Lucas Stach a écrit : > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: >> In order to be able to share the control hardware block between >> VPUs use a syscon instead a ioremap it in the driver. >> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' >> phandle is not found look at 'ctrl' reg-name. >> With the method it becomes useless to provide a list of register >> names so remove it. > Sorry for putting a spoke in the wheel after many iterations of the > series. > > We just discussed a way forward on how to handle the clocks and resets > provided by the blkctl block on i.MX8MM and later and it seems there is > a consensus on trying to provide virtual power domains from a blkctl > driver, controlling clocks and resets for the devices in the power > domain. I would like to avoid introducing yet another way of handling > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > what we are planning to do on the later chip generations. > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > virtual power domain thing a shot. Hey guys, I may I have miss them but I haven't see patches about power domain for IMX8MQ VPU control block ? Is it something that you still plan to do ? If not, I can resend my patches where I use syscon. Regards, Benjamin > > Regards, > Lucas > >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> >> --- >> version 9: >> - Corrections in commit message >> >> version 7: >> - Add Philipp reviewed-by tag. >> - Change syscon phandle name. >> >> >> >> >> version 5: >> - use syscon instead of VPU reset driver. >> - if DT doesn't provide syscon keep backward compatibilty by using >> 'ctrl' reg-name. >> >> drivers/staging/media/hantro/hantro.h | 5 +- >> drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 ++++++++++++--------- >> 2 files changed, 34 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h >> index 6c1b888abe75..37b9ce04bd4e 100644 >> --- a/drivers/staging/media/hantro/hantro.h >> +++ b/drivers/staging/media/hantro/hantro.h >> @@ -13,6 +13,7 @@ >> #define HANTRO_H_ >> >> >> >> >> #include <linux/platform_device.h> >> +#include <linux/regmap.h> >> #include <linux/videodev2.h> >> #include <linux/wait.h> >> #include <linux/clk.h> >> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) >> * @reg_bases: Mapped addresses of VPU registers. >> * @enc_base: Mapped address of VPU encoder register for convenience. >> * @dec_base: Mapped address of VPU decoder register for convenience. >> - * @ctrl_base: Mapped address of VPU control block. >> + * @ctrl_base: Regmap of VPU control block. >> * @vpu_mutex: Mutex to synchronize V4L2 calls. >> * @irqlock: Spinlock to synchronize access to data structures >> * shared with interrupt handlers. >> @@ -186,7 +187,7 @@ struct hantro_dev { >> void __iomem **reg_bases; >> void __iomem *enc_base; >> void __iomem *dec_base; >> - void __iomem *ctrl_base; >> + struct regmap *ctrl_base; >> >> >> >> >> struct mutex vpu_mutex; /* video_device lock */ >> spinlock_t irqlock; >> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c >> index c222de075ef4..8d0c3425234b 100644 >> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c >> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c >> @@ -7,6 +7,7 @@ >> >> >> >> >> #include <linux/clk.h> >> #include <linux/delay.h> >> +#include <linux/mfd/syscon.h> >> >> >> >> >> #include "hantro.h" >> #include "hantro_jpeg.h" >> @@ -24,30 +25,28 @@ >> #define CTRL_G1_PP_FUSE 0x0c >> #define CTRL_G2_DEC_FUSE 0x10 >> >> >> >> >> +static const struct regmap_config ctrl_regmap_ctrl = { >> + .reg_bits = 32, >> + .val_bits = 32, >> + .reg_stride = 0x14, >> +}; >> + >> static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) >> { >> - u32 val; >> - >> /* Assert */ >> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >> - val &= ~reset_bits; >> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); >> >> >> >> >> udelay(2); >> >> >> >> >> /* Release */ >> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); >> - val |= reset_bits; >> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); >> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, >> + reset_bits, reset_bits); >> } >> >> >> >> >> static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) >> { >> - u32 val; >> - >> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); >> - val |= clock_bits; >> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); >> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, >> + clock_bits, clock_bits); >> } >> >> >> >> >> static int imx8mq_runtime_resume(struct hantro_dev *vpu) >> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) >> imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); >> >> >> >> >> /* Set values of the fuse registers */ >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); >> - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); >> + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); >> + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); >> + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); >> >> >> >> >> clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); >> >> >> >> >> @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) >> >> >> >> >> static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) >> { >> - vpu->dec_base = vpu->reg_bases[0]; >> - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; >> + struct device_node *np = vpu->dev->of_node; >> + >> + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); >> + if (IS_ERR(vpu->ctrl_base)) { >> + struct resource *res; >> + void __iomem *ctrl; >> + >> + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); >> + ctrl = devm_ioremap_resource(vpu->dev, res); >> + if (IS_ERR(ctrl)) >> + return PTR_ERR(ctrl); >> + >> + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); >> + if (IS_ERR(vpu->ctrl_base)) >> + return PTR_ERR(vpu->ctrl_base); >> + } >> >> >> >> >> return 0; >> } >> @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { >> }; >> >> >> >> >> static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; >> -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; >> >> >> >> >> const struct hantro_variant imx8mq_vpu_variant = { >> .dec_fmts = imx8m_vpu_dec_fmts, >> @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { >> .num_irqs = ARRAY_SIZE(imx8mq_irqs), >> .clk_names = imx8mq_clk_names, >> .num_clocks = ARRAY_SIZE(imx8mq_clk_names), >> - .reg_names = imx8mq_reg_names, >> - .num_regs = ARRAY_SIZE(imx8mq_reg_names) >> }; > >
Hi Benjamin, On Mon, 2021-06-28 at 15:35 +0200, Benjamin Gaignard wrote: > > Le 16/04/2021 à 12:54, Lucas Stach a écrit : > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > In order to be able to share the control hardware block between > > > VPUs use a syscon instead a ioremap it in the driver. > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > phandle is not found look at 'ctrl' reg-name. > > > With the method it becomes useless to provide a list of register > > > names so remove it. > > Sorry for putting a spoke in the wheel after many iterations of the > > series. > > > > We just discussed a way forward on how to handle the clocks and resets > > provided by the blkctl block on i.MX8MM and later and it seems there is > > a consensus on trying to provide virtual power domains from a blkctl > > driver, controlling clocks and resets for the devices in the power > > domain. I would like to avoid introducing yet another way of handling > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > what we are planning to do on the later chip generations. > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > virtual power domain thing a shot. > > Hey guys, > > I may I have miss them but I haven't see patches about power domain for IMX8MQ > VPU control block ? > Is it something that you still plan to do ? > If not, I can resend my patches where I use syscon. > Please see "soc: imx: add i.MX BLK-CTL support" [1] sent by Peng a couple weeks ago. It adds the VPUMIX for i.MX8MM, so it seems the best way forward is to follow that design, extending it for i.MX8MQ. That's still under discussion, but hopefully it will be sorted out for v5.15. Speaking of i.MX8MM, I got a report that the Hantro G1 block mostly work, but needs to be restricted to 1920x1080. If you could add a new compatible and variant for that, maybe we can find someone to test it. [1] https://lore.kernel.org/linux-arm-kernel/7683ab0b-f905-dff1-aa4f-76ad638da568@oss.nxp.com/T/#mf73fe4a13aec0a8e633a14a5d9c2d5609799acb4 Kindly, Ezequiel
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h index 6c1b888abe75..37b9ce04bd4e 100644 --- a/drivers/staging/media/hantro/hantro.h +++ b/drivers/staging/media/hantro/hantro.h @@ -13,6 +13,7 @@ #define HANTRO_H_ #include <linux/platform_device.h> +#include <linux/regmap.h> #include <linux/videodev2.h> #include <linux/wait.h> #include <linux/clk.h> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev) * @reg_bases: Mapped addresses of VPU registers. * @enc_base: Mapped address of VPU encoder register for convenience. * @dec_base: Mapped address of VPU decoder register for convenience. - * @ctrl_base: Mapped address of VPU control block. + * @ctrl_base: Regmap of VPU control block. * @vpu_mutex: Mutex to synchronize V4L2 calls. * @irqlock: Spinlock to synchronize access to data structures * shared with interrupt handlers. @@ -186,7 +187,7 @@ struct hantro_dev { void __iomem **reg_bases; void __iomem *enc_base; void __iomem *dec_base; - void __iomem *ctrl_base; + struct regmap *ctrl_base; struct mutex vpu_mutex; /* video_device lock */ spinlock_t irqlock; diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c index c222de075ef4..8d0c3425234b 100644 --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c @@ -7,6 +7,7 @@ #include <linux/clk.h> #include <linux/delay.h> +#include <linux/mfd/syscon.h> #include "hantro.h" #include "hantro_jpeg.h" @@ -24,30 +25,28 @@ #define CTRL_G1_PP_FUSE 0x0c #define CTRL_G2_DEC_FUSE 0x10 +static const struct regmap_config ctrl_regmap_ctrl = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 0x14, +}; + static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits) { - u32 val; - /* Assert */ - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); - val &= ~reset_bits; - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0); udelay(2); /* Release */ - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET); - val |= reset_bits; - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET); + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, + reset_bits, reset_bits); } static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits) { - u32 val; - - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE); - val |= clock_bits; - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE); + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE, + clock_bits, clock_bits); } static int imx8mq_runtime_resume(struct hantro_dev *vpu) @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu) imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2); /* Set values of the fuse registers */ - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE); - writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE); - writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE); + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff); + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff); + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff); clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks); @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id) static int imx8mq_vpu_hw_init(struct hantro_dev *vpu) { - vpu->dec_base = vpu->reg_bases[0]; - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1]; + struct device_node *np = vpu->dev->of_node; + + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, "nxp,imx8m-vpu-ctrl"); + if (IS_ERR(vpu->ctrl_base)) { + struct resource *res; + void __iomem *ctrl; + + res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, "ctrl"); + ctrl = devm_ioremap_resource(vpu->dev, res); + if (IS_ERR(ctrl)) + return PTR_ERR(ctrl); + + vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl); + if (IS_ERR(vpu->ctrl_base)) + return PTR_ERR(vpu->ctrl_base); + } return 0; } @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = { }; static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" }; -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" }; const struct hantro_variant imx8mq_vpu_variant = { .dec_fmts = imx8m_vpu_dec_fmts, @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = { .num_irqs = ARRAY_SIZE(imx8mq_irqs), .clk_names = imx8mq_clk_names, .num_clocks = ARRAY_SIZE(imx8mq_clk_names), - .reg_names = imx8mq_reg_names, - .num_regs = ARRAY_SIZE(imx8mq_reg_names) };