Skip to contentSkip to conversation

miniarm-rk3288: set isp/vop qos priority level

PATCH Superseded Nickey Yang 3 participants 5 messages Dec 9, 2016 – Dec 17, 2016

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 miniarm-rk3288: set isp/vop qos priority level

isp-camera image will be broken when enter dual screen display mode. We set isp qos high to solve this problem.

---
 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
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +21 -0
new file mode 100644
index 0000000..d3d6c3e
@@ -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))
+
+#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
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +21 -0
index 79541a3..ba0f3a3 100644
@@ -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);
+
+	return 0;
+}
Signature · 1 line
1.9.1
#1 lore raw

PATCH v2 1/1 miniarm-rk3288: set isp/vop qos priority level

Hi Nickey,

Quoted 17 lines · Nickey Yang

isp-camera image will be broken when enter dual screen display mode. We set isp qos high to solve this problem.

---
 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
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +1 -0
new file mode 100644
index 0000000..d3d6c3e
@@ -0,0 +1,21 @@
+/*

arch/arm/include/asm/arch-rockchip/qos_rk3288.h line 11

Quoted 10 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+ * 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?

Quoted 26 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+
+#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
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +8 -0
index 79541a3..ba0f3a3 100644
@@ -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);

board/rockchip/miniarm_rk3288/miniarm-rk3288.c line 25

Quoted 10 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +10 -0
+       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.

Quoted 7 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +3 -1
+
+       return 0;
+}
--

1.9.1

Regards,
Simon

#2 lore raw

PATCH v2 1/1 miniarm-rk3288: set isp/vop qos priority level

Hi Simon,

Quoted 75 lines · Simon Glass

Hi Nickey,

Quoted 17 lines · Nickey Yang

isp-camera image will be broken when enter dual screen display mode. We set isp qos high to solve this problem.

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

arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +1 -0
new file mode 100644
index 0000000..d3d6c3e
@@ -0,0 +1,21 @@
+/*

arch/arm/include/asm/arch-rockchip/qos_rk3288.h line 11

Quoted 10 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+ * 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?

Quoted 26 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+
+#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
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +8 -0
index 79541a3..ba0f3a3 100644
@@ -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);

board/rockchip/miniarm_rk3288/miniarm-rk3288.c line 25

Quoted 10 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +10 -0
+       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/ ?

Quoted 2 lines

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?

@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

Quoted 13 lines
Quoted 8 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +3 -1
+
+       return 0;
+}
--

1.9.1

Regards,
Simon

#3 lore raw

PATCH v2 1/1 miniarm-rk3288: set isp/vop qos priority level

Hi Kever & Simon,

? 2016?12?14? 15:07, Kever Yang ??:

Quoted 94 lines

Hi Simon,

Quoted 81 lines · Simon Glass

Hi Nickey,

Quoted 20 lines · Nickey Yang

isp-camera image will be broken when enter dual screen display mode. We set isp qos high to solve this problem.

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

arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +1 -0
new file mode 100644
index 0000000..d3d6c3e
@@ -0,0 +1,21 @@
+/*

arch/arm/include/asm/arch-rockchip/qos_rk3288.h line 11

Quoted 10 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+ * 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?

Quoted 27 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+
+#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
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +8 -0
index 79541a3..ba0f3a3 100644
@@ -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);

board/rockchip/miniarm_rk3288/miniarm-rk3288.c line 25

Quoted 10 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +10 -0
+       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/ ?

Quoted 2 lines

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.

Quoted 12 lines

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?

Quoted 25 lines

@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

Quoted 13 lines
Quoted 8 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +3 -0
+
+       return 0;
+}
Signature · 1 line
1.9.1

Regards,
Simon

#4 lore raw

PATCH v2 1/1 miniarm-rk3288: set isp/vop qos priority level

Hi,

Quoted 137 lines · Nickey.Yang

Hi Kever & Simon,

? 2016?12?14? 15:07, Kever Yang ??:

Quoted 100 lines

Hi Simon,

Quoted 85 lines · Simon Glass

Hi Nickey,

Quoted 21 lines · Nickey Yang

isp-camera image will be broken when enter dual screen display mode. We set isp qos high to solve this problem.

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

arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +1 -0
new file mode 100644
index 0000000..d3d6c3e
@@ -0,0 +1,21 @@
+/*

arch/arm/include/asm/arch-rockchip/qos_rk3288.h line 11

Quoted 10 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+ * 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?

Quoted 27 lines
arch/arm/include/asm/arch-rockchip/qos_rk3288.h new file +10 -0
+
+#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
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +8 -0
index 79541a3..ba0f3a3 100644
@@ -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);

board/rockchip/miniarm_rk3288/miniarm-rk3288.c line 25

Quoted 10 lines
board/rockchip/miniarm_rk3288/miniarm-rk3288.c +10 -0
+       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/ ?

Quoted 2 lines

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.

Quoted 12 lines

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

board/rockchip/miniarm_rk3288/miniarm-rk3288.c +4 -0
+    __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

(quoted diff) +5 -0
+int rk_qos_init(void)
+{
+    /* do  set vop  qos level  */
+    /* do  set isp qos level  */
+}
Quoted 8 lines

Whether this is a better way?

Quoted 5 lines

@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

How about something like:

if (!fdt_node_check_compatible(gd-fdt_blob, 0, "rockchip,rk3288-miniarm"))
rk_qos_init()

Then it doesn't need to be weak.

Regards,
Simon