[U-Boot] [PATCH 2/5] imx: mx6qp add AQoS setting for IPU

[Stefano Babic]

Hi Peng,

On 08/08/2017 10:21, Peng Fan wrote:
> Bypass QoS for IPU and increase bankwidth threshold
> for PRE to get better performance for video.
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> Cc: Stefano Babic <sbabic at denx.de>
> Cc: Fabio Estevam <fabio.estevam at nxp.com>
> ---
>  arch/arm/include/asm/arch-mx6/imx-regs.h | 47 ++++++++++++++++++++++++++++++++
>  arch/arm/mach-imx/mx6/clock.c            | 37 +++++++++++++++++++++++--
>  2 files changed, 82 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
> index 86e2670..5468d59 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -74,6 +74,8 @@
>  #define GPV0_BASE_ADDR                  0x00B00000
>  #define GPV1_BASE_ADDR                  0x00C00000
>  
> +#define MF2_SCHED_BASE_ADDR		0x00BB0000
> +
>  #define AIPS1_ARB_BASE_ADDR             0x02000000
>  #define AIPS1_ARB_END_ADDR              0x020FFFFF
>  #define AIPS2_ARB_BASE_ADDR             0x02100000
> @@ -985,5 +987,50 @@ struct pwm_regs {
>  	u32	pr;
>  	u32	cnr;
>  };
> +
> +/* For i.MX6DQP */
> +#define AQOS_G2D_ID		0
> +#define AQOS_G3DPRIM_READ_ID	1
> +#define AQOS_G3DPRIM_WRITE_ID	2
> +#define AQOS_G3DSEC_READ_ID	3
> +#define AQOS_G3DSEC_WRITE_ID	4
> +#define AQOS_MPCORE0_ID		5
> +#define AQOS_MPCORE1_ID		6
> +#define AQOS_IPU1_ID		7
> +#define AQOS_IPU2_ID		8
> +#define AQOS_OPENVG_ID		9
> +#define AQOS_FAST1FARIC_ID	10
> +#define AQOS_PRE0_ID		11
> +#define AQOS_PRE1_ID		12
> +#define AQOS_PRE2_ID		13
> +#define AQOS_PRE3_ID		14
> +
> +struct aqos_regs {
> +	u32 coreid;
> +	u32 revid;
> +	u32 priority;
> +	u32 mode;
> +	u32 bandwidth;
> +	u32 saturation;
> +	u32 extcontrol;
> +	u32 reserved[25];
> +};
> +
> +struct sched_ctrl_regs {
> +	u32 coreid;
> +	u32 revid;
> +	u32 ddrconf;
> +	u32 ddrtiming;
> +	u32 ddrmode;
> +	u32 readlatency;
> +	u32 res0[4];
> +	u32 ipu1;
> +	u32 ipu2;
> +	u32 res1[2];
> +	u32 active;
> +	u32 res2[49];
> +	struct aqos_regs aqos[0];
> +};
> +
>  #endif /* __ASSEMBLER__*/
>  #endif /* __ASM_ARCH_MX6_IMX_REGS_H__ */
> diff --git a/arch/arm/mach-imx/mx6/clock.c b/arch/arm/mach-imx/mx6/clock.c
> index 1f2739e..73250aa 100644
> --- a/arch/arm/mach-imx/mx6/clock.c
> +++ b/arch/arm/mach-imx/mx6/clock.c
> @@ -1297,7 +1297,33 @@ int do_mx6_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	return 0;
>  }
>  
> -#ifndef CONFIG_MX6SX
> +static void pre_misc_setting(void)
> +{
> +	struct sched_ctrl_regs *regs =
> +		(struct sched_ctrl_regs *)MF2_SCHED_BASE_ADDR;
> +
> +	/* Bypass IPU1 QoS generator */
> +	writel(0x00000002, &regs->aqos[AQOS_IPU1_ID].mode);
> +	/* Bypass IPU2 QoS generator */
> +	writel(0x00000002, &regs->aqos[AQOS_IPU2_ID].mode);
> +	/* Bandwidth THR for of PRE0 */
> +	writel(0x00000200, &regs->aqos[AQOS_PRE0_ID].bandwidth);
> +	/* Bandwidth THR for of PRE1 */
> +	writel(0x00000200, &regs->aqos[AQOS_PRE1_ID].bandwidth);
> +	/* Bandwidth THR for of PRE2 */
> +	writel(0x00000200, &regs->aqos[AQOS_PRE2_ID].bandwidth);
> +	/* Bandwidth THR for of PRE3 */
> +	writel(0x00000200, &regs->aqos[AQOS_PRE3_ID].bandwidth);
> +	/* Saturation THR for of PRE0 */
> +	writel(0x00000010, &regs->aqos[AQOS_PRE0_ID].saturation);
> +	/* Saturation THR for of PRE1 */
> +	writel(0x00000010, &regs->aqos[AQOS_PRE1_ID].saturation);
> +	/* Saturation THR for of PRE2 */
> +	writel(0x00000010, &regs->aqos[AQOS_PRE2_ID].saturation);
> +	/* Saturation THR for of PRE */
> +	writel(0x00000010, &regs->aqos[AQOS_PRE3_ID].saturation);
> +}

Maybe can you add #define for all these constants ?

> +
>  void enable_ipu_clock(void)
>  {
>  	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
> @@ -1309,9 +1335,16 @@ void enable_ipu_clock(void)
>  	if (is_mx6dqp()) {
>  		setbits_le32(&mxc_ccm->CCGR6, MXC_CCM_CCGR6_PRG_CLK0_MASK);
>  		setbits_le32(&mxc_ccm->CCGR3, MXC_CCM_CCGR3_IPU2_IPU_MASK);
> +
> +		/*
> +		 * Since CONFIG_VIDEO_IPUV3 is always set in
> +		 * mx6sabre_common.h and this misc setting is
> +		 * a must for mx6qp, this position is ok to for
> +		 * such settings.
> +		 */

But you are in arch code and you cannot take this assumptions.

> +		pre_misc_setting();
>  	}
>  }
> -#endif
>  
>  #if defined(CONFIG_MX6Q) || defined(CONFIG_MX6D) || defined(CONFIG_MX6DL) || \
>  	defined(CONFIG_MX6S)
> 

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================