[U-Boot] [PATCH v2] miniarm-rk3288: set isp/vop qos priority level

[Nickey.Yang]

Hi Kever & Simon,


在 2016年12月14日 15:07, Kever Yang 写道:
> Hi Simon,
>
> On 12/12/2016 04:27 AM, Simon Glass wrote:
>> Hi Nickey,
>>
>> On 8 December 2016 at 21:39, Nickey Yang <nickey.yang at rock-chips.com> 
>> wrote:
>>> isp-camera image will be broken when enter dual screen display mode.
>>> We set isp qos high to solve this problem.
>>>
>>> Signed-off-by: Nickey Yang <nickey.yang at rock-chips.com>
>>> ---
>>>   arch/arm/include/asm/arch-rockchip/qos_rk3288.h | 21 
>>> +++++++++++++++++++++
>>>   board/rockchip/miniarm_rk3288/miniarm-rk3288.c  | 21 
>>> +++++++++++++++++++++
>>>   2 files changed, 42 insertions(+)
>>>   create mode 100644 arch/arm/include/asm/arch-rockchip/qos_rk3288.h
>>>
>>> diff --git a/arch/arm/include/asm/arch-rockchip/qos_rk3288.h 
>>> b/arch/arm/include/asm/arch-rockchip/qos_rk3288.h
>>> new file mode 100644
>>> index 0000000..d3d6c3e
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/arch-rockchip/qos_rk3288.h
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + * Copyright 2016 Rockchip Inc.
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0+
>>> + */
>>> +#ifndef _ASM_ARCH_QOS_RK3288_H
>>> +#define _ASM_ARCH_QOS_RK3288_H
>>> +
>>> +/* cpu axi qos priority */
>>> +#define CPU_AXI_QOS_PRIORITY_LEVEL(h, l) \
>>> +       ((((h) & 3) << 2) | ((l) & 3))
>> Can you instead define
>>
>> XXX_SHIFT   2
>> XXX_MASK  (3 << XXX_SHIFT)
>>
>> and then use these in the .c code?
>>
>>> +
>>> +#define CPU_AXI_QOS_PRIORITY    0x08
>>> +
>>> +#define VIO0_VOP_QOS            0xffad0400
>>> +#define VIO1_VOP_QOS            0xffad0000
>>> +#define VIO1_ISP_R_QOS          0xffad0900
>>> +#define VIO1_ISP_W0_QOS         0xffad0100
>>> +#define VIO1_ISP_W1_QOS         0xffad0180
>>> +
>>> +#endif
>>> diff --git a/board/rockchip/miniarm_rk3288/miniarm-rk3288.c 
>>> b/board/rockchip/miniarm_rk3288/miniarm-rk3288.c
>>> index 79541a3..ba0f3a3 100644
>>> --- a/board/rockchip/miniarm_rk3288/miniarm-rk3288.c
>>> +++ b/board/rockchip/miniarm_rk3288/miniarm-rk3288.c
>>> @@ -5,3 +5,24 @@
>>>    */
>>>
>>>   #include <common.h>
>>> +#include <asm/io.h>
>>> +#include <asm/arch/qos_rk3288.h>
>>> +
>>> +int rk_board_late_init(void)
>>> +{
>>> +       /* set isp qos to higher priority */
>>> +       writel(CPU_AXI_QOS_PRIORITY_LEVEL(2, 2),
>>> +              VIO1_ISP_R_QOS + CPU_AXI_QOS_PRIORITY);
>>> +       writel(CPU_AXI_QOS_PRIORITY_LEVEL(2, 2),
>>> +              VIO1_ISP_W0_QOS + CPU_AXI_QOS_PRIORITY);
>>> +       writel(CPU_AXI_QOS_PRIORITY_LEVEL(2, 2),
>>> +              VIO1_ISP_W1_QOS + CPU_AXI_QOS_PRIORITY);
>>> +
>>> +       /* set vop qos to higher priority */
>>> +       writel(CPU_AXI_QOS_PRIORITY_LEVEL(2, 2),
>>> +              VIO0_VOP_QOS + CPU_AXI_QOS_PRIORITY);
>>> +       writel(CPU_AXI_QOS_PRIORITY_LEVEL(2, 2),
>>> +              VIO1_VOP_QOS + CPU_AXI_QOS_PRIORITY);
>> Can you add a register struct for this in 
>> arch/arm/include/asm/arch-rockchip/ ?
>>
>> Also I think it would be best to put this code somewhere in
>> arch/arm/mach-rockchip and call it from your late init routine.
>
> I understand people don't like source cod in hardcode(or nearly) and 
> out of
> any framwork, but there do have some different one time init program for
> different SoC or board, I think it's OK to add then in soc_init() or 
> board_init()
> with appropriate comment.
>
> Back to the case here, setting the ISP NOC niu to higher priority is 
> only needed
> for the miniarm board for the use case in that board. It might have 
> other requirement
> for other board for different use cases, so it's OK to add the code in 
> board file.
> I think you would like to have some API function for this setting then 
> different board can use it
> instead of do SoC setting in board file, right?
> Before other board need this blob of code, would it be more clear to 
> put the code in board file?
>
or in arch/arm/mach-rockchip/rk3288-board.c
+    __weak int rk_qos_init(void)
+{
+    /* do  set  vop  qos level  */
+   }

int board_late_init(void)
  {
         setup_boot_mode();
+       rk_qos_init();

}

in board/rockchip/miniarm_rk3288/miniarm-rk3288.c
+int rk_qos_init(void)
+{
+    /* do  set vop  qos level  */
+    /* do  set isp qos level  */
+}

Whether this is  a better way?

> @Nickey, I'm sure the patch V3 is not what we need. Simon's suggestion 
> could be detail in this:
> - add a rk3288_qos_init() in arch/arm/mach-rockchip/rk3288-board.c
> - call the rk3288_qos_init() in rk_board_late_init() of 
> board/rockchip/miniarm_rk3288/miniarm-rk3288.c
>
> Thanks,
> - Kever
>>
>>> +
>>> +       return 0;
>>> +}
>>> -- 
>>> 1.9.1
>>>
>>>
>> Regards,
>> Simon
>>
>>
>>
>
>
>
>