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

[Lucas Stach]

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