linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
@ 2022-12-20 17:53 Lucas Stach
  2022-12-21  8:33 ` Marco Felsch
  2023-01-01  5:26 ` Shawn Guo
  0 siblings, 2 replies; 6+ messages in thread
From: Lucas Stach @ 2022-12-20 17:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	patchwork-lst, Lukas F . Hartmann

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(-)

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;
 }
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
  2022-12-20 17:53 [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level Lucas Stach
@ 2022-12-21  8:33 ` Marco Felsch
  2022-12-21  8:43   ` Lucas Stach
  2023-01-01  5:26 ` Shawn Guo
  1 sibling, 1 reply; 6+ messages in thread
From: Marco Felsch @ 2022-12-21  8:33 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Shawn Guo, linux-arm-kernel, Lukas F . Hartmann, NXP Linux Team,
	Pengutronix Kernel Team, patchwork-lst

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
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
  2022-12-21  8:33 ` Marco Felsch
@ 2022-12-21  8:43   ` Lucas Stach
  2022-12-21  8:50     ` Marco Felsch
  0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2022-12-21  8:43 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Shawn Guo, linux-arm-kernel, Lukas F . Hartmann, NXP Linux Team,
	Pengutronix Kernel Team, patchwork-lst

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
> > 
> > 
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
  2022-12-21  8:43   ` Lucas Stach
@ 2022-12-21  8:50     ` Marco Felsch
  0 siblings, 0 replies; 6+ messages in thread
From: Marco Felsch @ 2022-12-21  8:50 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Shawn Guo, linux-arm-kernel, Lukas F . Hartmann, NXP Linux Team,
	Pengutronix Kernel Team, patchwork-lst

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
> > > 
> > > 
> > > 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
  2022-12-20 17:53 [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level Lucas Stach
  2022-12-21  8:33 ` Marco Felsch
@ 2023-01-01  5:26 ` Shawn Guo
  1 sibling, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2023-01-01  5:26 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Pengutronix Kernel Team, NXP Linux Team, linux-arm-kernel,
	patchwork-lst, Lukas F . Hartmann

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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level
@ 2023-03-31  9:37 Peng Fan (OSS)
  0 siblings, 0 replies; 6+ messages in thread
From: Peng Fan (OSS) @ 2023-03-31  9:37 UTC (permalink / raw)
  To: shawnguo, s.hauer
  Cc: kernel, festevam, linux-imx, linux-kernel, linux-arm-kernel,
	Peng Fan

From: Peng Fan <peng.fan@nxp.com>

The ISI block could signal a panic condition to use hurry level
priority. When the bandwidth is low for ISI, the hurry level priority
could be used to avoid FIFO overflow. The NXP downstream ATF/Linux
Kernel sets the hurry level as 7.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V1:
 If anyone has setup to help test, that would be appreciated.

 drivers/soc/imx/imx8m-blk-ctrl.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 7c4045068804..eb13829e5dc1 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -690,6 +690,10 @@ static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = {
 #define LCDIF_ARCACHE_CTRL	0x4c
 #define  LCDIF_1_RD_HURRY	GENMASK(15, 13)
 #define  LCDIF_0_RD_HURRY	GENMASK(12, 10)
+#define ISI_CACHE_CTRL		0x50
+#define ISI_V_WR_HURRY		GENMASK(28, 26)
+#define ISI_U_WR_HURRY		GENMASK(25, 23)
+#define ISI_Y_WR_HURRY		GENMASK(22, 20)
 
 static int imx8mp_media_power_notifier(struct notifier_block *nb,
 				unsigned long action, void *data)
@@ -720,6 +724,15 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb,
 		regmap_set_bits(bc->regmap, LCDIF_ARCACHE_CTRL,
 				FIELD_PREP(LCDIF_1_RD_HURRY, 7) |
 				FIELD_PREP(LCDIF_0_RD_HURRY, 7));
+
+		/*
+		 * Set panic write hurry level for ISI interfaces to
+		 * maximum priority to avoid bandwidth issue.
+		 */
+		regmap_set_bits(bc->regmap, ISI_CACHE_CTRL,
+				FIELD_PREP(ISI_V_WR_HURRY, 7) |
+				FIELD_PREP(ISI_U_WR_HURRY, 7) |
+				FIELD_PREP(ISI_Y_WR_HURRY, 7));
 	}
 
 	return NOTIFY_OK;
-- 
2.37.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-31  9:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-20 17:53 [PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level Lucas Stach
2022-12-21  8:33 ` Marco Felsch
2022-12-21  8:43   ` Lucas Stach
2022-12-21  8:50     ` Marco Felsch
2023-01-01  5:26 ` Shawn Guo
  -- strict thread matches above, loose matches on Subject: below --
2023-03-31  9:37 Peng Fan (OSS)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).