[U-Boot] [PATCH v2 6/8] imx: mx6qp Enable PRG clock and AQoS setting for IPU

[Stefano Babic <sbabic@denx.de>]

Hi Peng,

On 11/06/2015 12:30, Peng Fan wrote:
> The i.MX6DQP has a PRG module, need to enable its clock for using IPU.
> 
> Bypass QoS for IPU and increase bankwidth threshold for PRE to get
> better performance for video.
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Signed-off-by: Brown Oliver <B37094@freescale.com>
> Signed-off-by: Ye.Li <B37916@freescale.com>
> ---
> 
> Changes v2:
>  1. runtime check
>  2. introduce ipu qos settings for better performance
> 
>  arch/arm/cpu/armv7/mx6/clock.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
> index 0d862b2..7106df0 100644
> --- a/arch/arm/cpu/armv7/mx6/clock.c
> +++ b/arch/arm/cpu/armv7/mx6/clock.c
> @@ -862,6 +862,30 @@ int do_mx6_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  }
>  
>  #ifndef CONFIG_MX6SX
> +static void ipu_qos_setting(void)
> +{
> +	/* Bypass IPU1 QoS generator */
> +	writel(0x00000002, 0x00bb048c);
> +	/* Bypass IPU2 QoS generator */
> +	writel(0x00000002, 0x00bb050c);
> +	/* Bandwidth THR for of PRE0 */
> +	writel(0x00000200, 0x00bb0690);
> +	/* Bandwidth THR for of PRE1 */
> +	writel(0x00000200, 0x00bb0710);
> +	/* Bandwidth THR for of PRE2 */
> +	writel(0x00000200, 0x00bb0790);
> +	/* Bandwidth THR for of PRE3 */
> +	writel(0x00000200, 0x00bb0810);
> +	/* Saturation THR for of PRE0 */
> +	writel(0x00000010, 0x00bb0694);
> +	/* Saturation THR for of PRE1 */
> +	writel(0x00000010, 0x00bb0714);
> +	/* Saturation THR for of PRE2 */
> +	writel(0x00000010, 0x00bb0794);
> +	/* Saturation THR for of PRE */
> +	writel(0x00000010, 0x00bb0814);
> +}

Sorry, but this is very difficult to read. I guess you want to set the
QOS for IPU, but then without accessing to the GPR in the iomux. Anyway,
you are accessing directly with offset and value to internal registers.
Please add structures to access these registers and/or functions if the
offset must be computed

> +
>  void enable_ipu_clock(void)
>  {
>  	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
> @@ -869,7 +893,22 @@ void enable_ipu_clock(void)
>  	reg = readl(&mxc_ccm->CCGR3);
>  	reg |= MXC_CCM_CCGR3_IPU1_IPU_MASK;
>  	writel(reg, &mxc_ccm->CCGR3);
> +
> +	if (is_mx6dqp()) {
> +#define MXC_CCM_CCGR6_PRG_CLK0_MASK (3 << 24)

I will suggest, as this as other ones are new defines, that you put all
of them where they belong (crm_regs.h), surrounding with comments
explaining that are defined for the QuadPlus.

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@denx.de
=====================================================================