[PATCH] soc: imx: imx8m-blk-ctrl: set LCDIF panic read hurry level

[Marco Felsch]

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 at pengutronix.de>
> > 
> > lgtm apart a minor nit, please see below.
> > 
> > Reviewed-by: Marco Felsch <m.felsch at 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 at 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
> > > 
> > > 
> > > 
> 
>