| Message ID | 20221220175353.1884333-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level | expand |
On 22-12-20, Lucas Stach wrote: > When the LCDIF block signals a panic condition due to the display FIFO > falling below the threshold, the priority at the NoC level is boosted > to the value set in the LCDIF_ARCACHE_CTRL register of i.MX8MP mediamix > blk-ctrl. Same as all other blk-ctrl registers this register is reset > when the domain is powered down. Initialize the panic hurry levels for > both LCIF interfaces to the maximium priority (same as downstream TF-A > and proven to work with the other priorities set in the interconnect > driver) when coming back from power down. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> lgtm apart a minor nit, please see below. Reviewed-by: Marco Felsch <m.felsch@pengutronix.de> > --- > drivers/soc/imx/imx8m-blk-ctrl.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > index 00879615a701..b93b296c47be 100644 > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > @@ -4,6 +4,7 @@ > * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de> > */ > > +#include <linux/bitfield.h> > #include <linux/device.h> > #include <linux/interconnect.h> > #include <linux/module.h> > @@ -649,6 +650,10 @@ static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { > .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), > }; > > +#define LCDIF_ARCACHE_CTRL 0x4c > +#define LCDIF_1_RD_HURRY GENMASK(15, 13) > +#define LCDIF_0_RD_HURRY GENMASK(12, 10) ^ Was this extra space intended? Regards, Marco > + > static int imx8mp_media_power_notifier(struct notifier_block *nb, > unsigned long action, void *data) > { > @@ -662,14 +667,24 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb, > 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) > + if (action == GENPD_NOTIFY_ON) { > + /* > + * 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. > + */ > udelay(5); > > + /* > + * Set panic read hurry level for both LCDIF interfaces to > + * maximum priority to minimize chances of display FIFO > + * underflow. > + */ > + regmap_set_bits(bc->regmap, LCDIF_ARCACHE_CTRL, > + FIELD_PREP(LCDIF_1_RD_HURRY, 7) | > + FIELD_PREP(LCDIF_0_RD_HURRY, 7)); > + } > + > return NOTIFY_OK; > } > > -- > 2.30.2 > > >
Am Mittwoch, dem 21.12.2022 um 09:33 +0100 schrieb Marco Felsch: > On 22-12-20, Lucas Stach wrote: > > When the LCDIF block signals a panic condition due to the display FIFO > > falling below the threshold, the priority at the NoC level is boosted > > to the value set in the LCDIF_ARCACHE_CTRL register of i.MX8MP mediamix > > blk-ctrl. Same as all other blk-ctrl registers this register is reset > > when the domain is powered down. Initialize the panic hurry levels for > > both LCIF interfaces to the maximium priority (same as downstream TF-A > > and proven to work with the other priorities set in the interconnect > > driver) when coming back from power down. > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > lgtm apart a minor nit, please see below. > > Reviewed-by: Marco Felsch <m.felsch@pengutronix.de> > > > --- > > drivers/soc/imx/imx8m-blk-ctrl.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > > index 00879615a701..b93b296c47be 100644 > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > > @@ -4,6 +4,7 @@ > > * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de> > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/device.h> > > #include <linux/interconnect.h> > > #include <linux/module.h> > > @@ -649,6 +650,10 @@ static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { > > .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), > > }; > > > > +#define LCDIF_ARCACHE_CTRL 0x4c > > +#define LCDIF_1_RD_HURRY GENMASK(15, 13) > > +#define LCDIF_0_RD_HURRY GENMASK(12, 10) > ^ > Was this extra space intended? > Yes, I'm using this to separate register offset from register content defines. If you look closely this style is present in lots of drivers authored by me and others, who found it to be a good idea. ;) Regards, Lucas > Regards, > Marco > > > + > > static int imx8mp_media_power_notifier(struct notifier_block *nb, > > unsigned long action, void *data) > > { > > @@ -662,14 +667,24 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb, > > 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) > > + if (action == GENPD_NOTIFY_ON) { > > + /* > > + * 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. > > + */ > > udelay(5); > > > > + /* > > + * Set panic read hurry level for both LCDIF interfaces to > > + * maximum priority to minimize chances of display FIFO > > + * underflow. > > + */ > > + regmap_set_bits(bc->regmap, LCDIF_ARCACHE_CTRL, > > + FIELD_PREP(LCDIF_1_RD_HURRY, 7) | > > + FIELD_PREP(LCDIF_0_RD_HURRY, 7)); > > + } > > + > > return NOTIFY_OK; > > } > > > > -- > > 2.30.2 > > > > > >
On 22-12-21, Lucas Stach wrote: > Am Mittwoch, dem 21.12.2022 um 09:33 +0100 schrieb Marco Felsch: > > On 22-12-20, Lucas Stach wrote: > > > When the LCDIF block signals a panic condition due to the display FIFO > > > falling below the threshold, the priority at the NoC level is boosted > > > to the value set in the LCDIF_ARCACHE_CTRL register of i.MX8MP mediamix > > > blk-ctrl. Same as all other blk-ctrl registers this register is reset > > > when the domain is powered down. Initialize the panic hurry levels for > > > both LCIF interfaces to the maximium priority (same as downstream TF-A > > > and proven to work with the other priorities set in the interconnect > > > driver) when coming back from power down. > > > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > > > lgtm apart a minor nit, please see below. > > > > Reviewed-by: Marco Felsch <m.felsch@pengutronix.de> > > > > > --- > > > drivers/soc/imx/imx8m-blk-ctrl.c | 27 +++++++++++++++++++++------ > > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c > > > index 00879615a701..b93b296c47be 100644 > > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c > > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c > > > @@ -4,6 +4,7 @@ > > > * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de> > > > */ > > > > > > +#include <linux/bitfield.h> > > > #include <linux/device.h> > > > #include <linux/interconnect.h> > > > #include <linux/module.h> > > > @@ -649,6 +650,10 @@ static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { > > > .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), > > > }; > > > > > > +#define LCDIF_ARCACHE_CTRL 0x4c > > > +#define LCDIF_1_RD_HURRY GENMASK(15, 13) > > > +#define LCDIF_0_RD_HURRY GENMASK(12, 10) > > ^ > > Was this extra space intended? > > > Yes, I'm using this to separate register offset from register content > defines. Fine for me :) I know this style and I use it here and there as well but with more indention, therefore I asked. Regards, Marco > If you look closely this style is present in lots of drivers > authored by me and others, who found it to be a good idea. ;) > > Regards, > Lucas > > > Regards, > > Marco > > > > > + > > > static int imx8mp_media_power_notifier(struct notifier_block *nb, > > > unsigned long action, void *data) > > > { > > > @@ -662,14 +667,24 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb, > > > 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) > > > + if (action == GENPD_NOTIFY_ON) { > > > + /* > > > + * 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. > > > + */ > > > udelay(5); > > > > > > + /* > > > + * Set panic read hurry level for both LCDIF interfaces to > > > + * maximum priority to minimize chances of display FIFO > > > + * underflow. > > > + */ > > > + regmap_set_bits(bc->regmap, LCDIF_ARCACHE_CTRL, > > > + FIELD_PREP(LCDIF_1_RD_HURRY, 7) | > > > + FIELD_PREP(LCDIF_0_RD_HURRY, 7)); > > > + } > > > + > > > return NOTIFY_OK; > > > } > > > > > > -- > > > 2.30.2 > > > > > > > > > > >
On Tue, Dec 20, 2022 at 06:53:53PM +0100, Lucas Stach wrote: > When the LCDIF block signals a panic condition due to the display FIFO > falling below the threshold, the priority at the NoC level is boosted > to the value set in the LCDIF_ARCACHE_CTRL register of i.MX8MP mediamix > blk-ctrl. Same as all other blk-ctrl registers this register is reset > when the domain is powered down. Initialize the panic hurry levels for > both LCIF interfaces to the maximium priority (same as downstream TF-A > and proven to work with the other priorities set in the interconnect > driver) when coming back from power down. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Applied, thanks!
diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c index 00879615a701..b93b296c47be 100644 --- a/drivers/soc/imx/imx8m-blk-ctrl.c +++ b/drivers/soc/imx/imx8m-blk-ctrl.c @@ -4,6 +4,7 @@ * Copyright 2021 Pengutronix, Lucas Stach <kernel@pengutronix.de> */ +#include <linux/bitfield.h> #include <linux/device.h> #include <linux/interconnect.h> #include <linux/module.h> @@ -649,6 +650,10 @@ static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = { .num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data), }; +#define LCDIF_ARCACHE_CTRL 0x4c +#define LCDIF_1_RD_HURRY GENMASK(15, 13) +#define LCDIF_0_RD_HURRY GENMASK(12, 10) + static int imx8mp_media_power_notifier(struct notifier_block *nb, unsigned long action, void *data) { @@ -662,14 +667,24 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb, 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) + if (action == GENPD_NOTIFY_ON) { + /* + * 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. + */ udelay(5); + /* + * Set panic read hurry level for both LCDIF interfaces to + * maximum priority to minimize chances of display FIFO + * underflow. + */ + regmap_set_bits(bc->regmap, LCDIF_ARCACHE_CTRL, + FIELD_PREP(LCDIF_1_RD_HURRY, 7) | + FIELD_PREP(LCDIF_0_RD_HURRY, 7)); + } + return NOTIFY_OK; }
When the LCDIF block signals a panic condition due to the display FIFO falling below the threshold, the priority at the NoC level is boosted to the value set in the LCDIF_ARCACHE_CTRL register of i.MX8MP mediamix blk-ctrl. Same as all other blk-ctrl registers this register is reset when the domain is powered down. Initialize the panic hurry levels for both LCIF interfaces to the maximium priority (same as downstream TF-A and proven to work with the other priorities set in the interconnect driver) when coming back from power down. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/soc/imx/imx8m-blk-ctrl.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)