Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1692354 > unrolled thread

[PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register

Started byMark Yao <mark.yao@rock-chips.com>
First post2017-07-20 04:50 +0200
Last post2017-07-26 08:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register Mark Yao <mark.yao@rock-chips.com> - 2017-07-20 04:50 +0200
    Re: [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register Heiko Stuebner <heiko@sntech.de> - 2017-07-25 23:50 +0200
      Re: [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop  register Mark yao <mark.yao@rock-chips.com> - 2017-07-26 08:20 +0200

#1692354 — [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register

FromMark Yao <mark.yao@rock-chips.com>
Date2017-07-20 04:50 +0200
Subject[PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register
Message-ID<u59Hj-Sh-1@gated-at.bofh.it>
Since the drm atomic framework, only a small part of the vop
register needs sync write, Currently seems only following registers
need sync write:
   cfg_done, standby and interrupt related register.

All ctrl registers are using the sync write method that is
inefficient, hardcode the write_relaxed flags to vop registers,
then can only do synchronize write for those actual needed register.

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++-------
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 42 ++++++++++++++++-------------
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 81164d6..784a2b7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -42,18 +42,12 @@
 #include "rockchip_drm_psr.h"
 #include "rockchip_drm_vop.h"
 
-#define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
-		vop_mask_write(x, off, mask, shift, v, write_mask, true)
-
-#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
-		vop_mask_write(x, off, mask, shift, v, write_mask, false)
-
 #define REG_SET(x, base, reg, v, mode) \
-		__REG_SET_##mode(x, base + reg.offset, \
-				 reg.mask, reg.shift, v, reg.write_mask)
+		vop_mask_write(x, base + reg.offset, reg.mask, reg.shift, \
+			       v, reg.write_mask, reg.relaxed)
 #define REG_SET_MASK(x, base, reg, mask, v, mode) \
-		__REG_SET_##mode(x, base + reg.offset, \
-				 mask, reg.shift, v, reg.write_mask)
+		vop_mask_write(x, base + reg.offset, \
+			       mask, reg.shift, v, reg.write_mask, reg.relaxed)
 
 #define VOP_WIN_SET(x, win, name, v) \
 		REG_SET(x, win->base, win->phy->name, v, RELAXED)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 00e9d79..691dd42 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -20,17 +20,23 @@
 #include "rockchip_drm_vop.h"
 #include "rockchip_vop_reg.h"
 
-#define VOP_REG(off, _mask, s) \
-		{.offset = off, \
+#define _VOP_REG(off, _mask, _shift, _write_mask, _relaxed) \
+		{ \
+		 .offset = off, \
 		 .mask = _mask, \
-		 .shift = s, \
-		 .write_mask = false,}
+		 .shift = _shift, \
+		 .write_mask = _write_mask, \
+		 .relaxed = _relaxed, \
+		}
 
-#define VOP_REG_MASK(off, _mask, s) \
-		{.offset = off, \
-		 .mask = _mask, \
-		 .shift = s, \
-		 .write_mask = true,}
+#define VOP_REG(off, _mask, _shift) \
+		_VOP_REG(off, _mask, _shift, false, true)
+
+#define VOP_REG_SYNC(off, _mask, _shift) \
+		_VOP_REG(off, _mask, _shift, false, false)
+
+#define VOP_REG_MASK_SYNC(off, _mask, _shift) \
+		_VOP_REG(off, _mask, _shift, true, false)
 
 static const uint32_t formats_win_full[] = {
 	DRM_FORMAT_XRGB8888,
@@ -116,7 +122,7 @@
 };
 
 static const struct vop_ctrl rk3036_ctrl_data = {
-	.standby = VOP_REG(RK3036_SYS_CTRL, 0x1, 30),
+	.standby = VOP_REG_SYNC(RK3036_SYS_CTRL, 0x1, 30),
 	.out_mode = VOP_REG(RK3036_DSP_CTRL0, 0xf, 0),
 	.pin_pol = VOP_REG(RK3036_DSP_CTRL0, 0xf, 4),
 	.htotal_pw = VOP_REG(RK3036_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
@@ -124,7 +130,7 @@
 	.vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
 	.vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
-	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
+	.cfg_done = VOP_REG_SYNC(RK3036_REG_CFG_DONE, 0x1, 0),
 };
 
 static const struct vop_data rk3036_vop = {
@@ -200,7 +206,7 @@
 };
 
 static const struct vop_ctrl rk3288_ctrl_data = {
-	.standby = VOP_REG(RK3288_SYS_CTRL, 0x1, 22),
+	.standby = VOP_REG_SYNC(RK3288_SYS_CTRL, 0x1, 22),
 	.gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23),
 	.mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20),
 	.rgb_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 12),
@@ -221,7 +227,7 @@
 	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
 	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
 	.global_regdone_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 11),
-	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
+	.cfg_done = VOP_REG_SYNC(RK3288_REG_CFG_DONE, 0x1, 0),
 };
 
 /*
@@ -265,7 +271,7 @@
 };
 
 static const struct vop_ctrl rk3399_ctrl_data = {
-	.standby = VOP_REG(RK3399_SYS_CTRL, 0x1, 22),
+	.standby = VOP_REG_SYNC(RK3399_SYS_CTRL, 0x1, 22),
 	.gate_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 23),
 	.dp_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 11),
 	.rgb_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 12),
@@ -289,7 +295,7 @@
 	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
 	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
 	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
-	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
+	.cfg_done = VOP_REG_MASK_SYNC(RK3399_REG_CFG_DONE, 0x1, 0),
 };
 
 static const int rk3399_vop_intrs[] = {
@@ -305,9 +311,9 @@
 static const struct vop_intr rk3399_vop_intr = {
 	.intrs = rk3399_vop_intrs,
 	.nintrs = ARRAY_SIZE(rk3399_vop_intrs),
-	.status = VOP_REG_MASK(RK3399_INTR_STATUS0, 0xffff, 0),
-	.enable = VOP_REG_MASK(RK3399_INTR_EN0, 0xffff, 0),
-	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
+	.status = VOP_REG_MASK_SYNC(RK3399_INTR_STATUS0, 0xffff, 0),
+	.enable = VOP_REG_MASK_SYNC(RK3399_INTR_EN0, 0xffff, 0),
+	.clear = VOP_REG_MASK_SYNC(RK3399_INTR_CLEAR0, 0xffff, 0),
 };
 
 static const struct vop_data rk3399_vop_big = {
-- 
1.9.1

[toc] | [next] | [standalone]

#1696608

FromHeiko Stuebner <heiko@sntech.de>
Date2017-07-25 23:50 +0200
Message-ID<u7fSi-8jG-21@gated-at.bofh.it>
In reply to#1692354
Hi Mark,

Am Donnerstag, 20. Juli 2017, 10:43:27 CEST schrieb Mark Yao:
> Since the drm atomic framework, only a small part of the vop
> register needs sync write, Currently seems only following registers
> need sync write:
>    cfg_done, standby and interrupt related register.
> 
> All ctrl registers are using the sync write method that is
> inefficient, hardcode the write_relaxed flags to vop registers,
> then can only do synchronize write for those actual needed register.
> 
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++-------
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 42 ++++++++++++++++-------------
>  2 files changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 81164d6..784a2b7 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -42,18 +42,12 @@
>  #include "rockchip_drm_psr.h"
>  #include "rockchip_drm_vop.h"
>  
> -#define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
> -		vop_mask_write(x, off, mask, shift, v, write_mask, true)
> -
> -#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
> -		vop_mask_write(x, off, mask, shift, v, write_mask, false)
> -
>  #define REG_SET(x, base, reg, v, mode) \
> -		__REG_SET_##mode(x, base + reg.offset, \
> -				 reg.mask, reg.shift, v, reg.write_mask)
> +		vop_mask_write(x, base + reg.offset, reg.mask, reg.shift, \
> +			       v, reg.write_mask, reg.relaxed)
>  #define REG_SET_MASK(x, base, reg, mask, v, mode) \
> -		__REG_SET_##mode(x, base + reg.offset, \
> -				 mask, reg.shift, v, reg.write_mask)
> +		vop_mask_write(x, base + reg.offset, \
> +			       mask, reg.shift, v, reg.write_mask, reg.relaxed)

you only introduce the relaxed element of struct vop_reg in patch4.
So using it here produces a compile error

../drivers/gpu/drm/rockchip/rockchip_drm_vop.c: In function ‘vop_cfg_done’:
../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:47:33: error: ‘const struct vop_reg’ has no member named ‘relaxed’
           v, reg.write_mask, reg.relaxed)
                                 ^
../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:59:3: note: in expansion of macro ‘REG_SET’
   REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
   ^~~~~~~
../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:201:2: note: in expansion of macro ‘VOP_CTRL_SET’
  VOP_CTRL_SET(vop, cfg_done, 1);
  ^~~~~~~~~~~~

when only patches 1+2 are applied. So the relaxed field addition should
definitly move into this patch to not break bisectability.


Heiko

[toc] | [prev] | [next] | [standalone]

#1696808 — Re: [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register

FromMark yao <mark.yao@rock-chips.com>
Date2017-07-26 08:20 +0200
SubjectRe: [PATCH v5 2/7] drm/rockchip: vop: move write_relaxed flags to vop register
Message-ID<u7nPR-56r-17@gated-at.bofh.it>
In reply to#1696608
On 2017年07月26日 05:47, Heiko Stuebner wrote:
> Hi Mark,
>
> Am Donnerstag, 20. Juli 2017, 10:43:27 CEST schrieb Mark Yao:
>> Since the drm atomic framework, only a small part of the vop
>> register needs sync write, Currently seems only following registers
>> need sync write:
>>     cfg_done, standby and interrupt related register.
>>
>> All ctrl registers are using the sync write method that is
>> inefficient, hardcode the write_relaxed flags to vop registers,
>> then can only do synchronize write for those actual needed register.
>>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++-------
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 42 ++++++++++++++++-------------
>>   2 files changed, 28 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index 81164d6..784a2b7 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -42,18 +42,12 @@
>>   #include "rockchip_drm_psr.h"
>>   #include "rockchip_drm_vop.h"
>>   
>> -#define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
>> -		vop_mask_write(x, off, mask, shift, v, write_mask, true)
>> -
>> -#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
>> -		vop_mask_write(x, off, mask, shift, v, write_mask, false)
>> -
>>   #define REG_SET(x, base, reg, v, mode) \
>> -		__REG_SET_##mode(x, base + reg.offset, \
>> -				 reg.mask, reg.shift, v, reg.write_mask)
>> +		vop_mask_write(x, base + reg.offset, reg.mask, reg.shift, \
>> +			       v, reg.write_mask, reg.relaxed)
>>   #define REG_SET_MASK(x, base, reg, mask, v, mode) \
>> -		__REG_SET_##mode(x, base + reg.offset, \
>> -				 mask, reg.shift, v, reg.write_mask)
>> +		vop_mask_write(x, base + reg.offset, \
>> +			       mask, reg.shift, v, reg.write_mask, reg.relaxed)
> you only introduce the relaxed element of struct vop_reg in patch4.
> So using it here produces a compile error
>
> ../drivers/gpu/drm/rockchip/rockchip_drm_vop.c: In function ‘vop_cfg_done’:
> ../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:47:33: error: ‘const struct vop_reg’ has no member named ‘relaxed’
>             v, reg.write_mask, reg.relaxed)
>                                   ^
> ../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:59:3: note: in expansion of macro ‘REG_SET’
>     REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
>     ^~~~~~~
> ../drivers/gpu/drm/rockchip/rockchip_drm_vop.c:201:2: note: in expansion of macro ‘VOP_CTRL_SET’
>    VOP_CTRL_SET(vop, cfg_done, 1);
>    ^~~~~~~~~~~~
>
> when only patches 1+2 are applied. So the relaxed field addition should
> definitly move into this patch to not break bisectability.
>
>
> Heiko
>
>
>
>
Hi Heiko

Thanks for the test, will fix it at next version.

-- 
Ｍark Yao

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web