[PATCH v2 0/5] drm/rockchip: add all full framework vop support

[PATCH v2 0/5] drm/rockchip: add all full framework vop support

[Mark Yao]

These patches try to make all current rockchip full framework vop works
on drm, The newer vop design always have some different to the old one,
So we add a register verify mechanism to distinguish those register, then
the registers table can be reused.

And people can easy to know the different for the vops from register table.

Tested on rk3399 evb board.

Changes in v2:
- rename rk322x to rk3228(Heiko St?bner)
- correct some vop registers define

Mark Yao (5):
  drm/rockchip: vop: get rid of register init table
  drm/rockchip: vop: support verify registers with vop version
  drm/rockchip: vop: move line_flag_num to interrupt registers
  drm/rockchip: vop: add a series of vop support
  dt-bindings: display: fill Documents for series of vop

 .../bindings/display/rockchip/rockchip-vop.txt     |   4 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        |  74 +-
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h        |  32 +-
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c        | 263 +++---
 drivers/gpu/drm/rockchip/rockchip_vop_reg.h        | 905 ++++++++++++++++-----
 5 files changed, 956 insertions(+), 322 deletions(-)

-- 
1.9.1

[PATCH v2 1/5] drm/rockchip: vop: get rid of register init table

[Mark Yao]

Register init table use un-document define, it is unreadable,
And sometimes we only want to update tiny bits, init table
method is not friendly, it's diffcult to reuse for difference
chips.

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  6 ++--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 10 ++-----
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 43 +++--------------------------
 3 files changed, 10 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 45589d6..7a5f809 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1393,7 +1393,6 @@ static void vop_destroy_crtc(struct vop *vop)
 static int vop_initial(struct vop *vop)
 {
 	const struct vop_data *vop_data = vop->data;
-	const struct vop_reg_data *init_table = vop_data->init_table;
 	struct reset_control *ahb_rst;
 	int i, ret;
 
@@ -1453,13 +1452,14 @@ static int vop_initial(struct vop *vop)
 
 	memcpy(vop->regsbak, vop->regs, vop->len);
 
-	for (i = 0; i < vop_data->table_size; i++)
-		vop_writel(vop, init_table[i].offset, init_table[i].value);
+	VOP_CTRL_SET(vop, global_regdone_en, 1);
+	VOP_CTRL_SET(vop, dsp_blank, 0);
 
 	for (i = 0; i < vop_data->win_size; i++) {
 		const struct vop_win_data *win = &vop_data->win[i];
 
 		VOP_WIN_SET(vop, win, enable, 0);
+		VOP_WIN_SET(vop, win, gate, 1);
 	}
 
 	vop_cfg_done(vop);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 9979fd0..084d3b2 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -24,11 +24,6 @@ enum vop_data_format {
 	VOP_FMT_YUV444SP,
 };
 
-struct vop_reg_data {
-	uint32_t offset;
-	uint32_t value;
-};
-
 struct vop_reg {
 	uint32_t offset;
 	uint32_t shift;
@@ -46,6 +41,7 @@ struct vop_ctrl {
 	struct vop_reg hdmi_en;
 	struct vop_reg mipi_en;
 	struct vop_reg dp_en;
+	struct vop_reg dsp_blank;
 	struct vop_reg out_mode;
 	struct vop_reg dither_down;
 	struct vop_reg dither_up;
@@ -65,6 +61,7 @@ struct vop_ctrl {
 
 	struct vop_reg line_flag_num[2];
 
+	struct vop_reg global_regdone_en;
 	struct vop_reg cfg_done;
 };
 
@@ -115,6 +112,7 @@ struct vop_win_phy {
 	uint32_t nformats;
 
 	struct vop_reg enable;
+	struct vop_reg gate;
 	struct vop_reg format;
 	struct vop_reg rb_swap;
 	struct vop_reg act_info;
@@ -136,8 +134,6 @@ struct vop_win_data {
 };
 
 struct vop_data {
-	const struct vop_reg_data *init_table;
-	unsigned int table_size;
 	const struct vop_ctrl *ctrl;
 	const struct vop_intr *intr;
 	const struct vop_win_data *win;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index bafd698..00e9d79 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -127,13 +127,7 @@
 	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
 };
 
-static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
-	{RK3036_DSP_CTRL1, 0x00000000},
-};
-
 static const struct vop_data rk3036_vop = {
-	.init_table = rk3036_vop_init_reg_table,
-	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
 	.ctrl = &rk3036_ctrl_data,
 	.intr = &rk3036_intr,
 	.win = rk3036_vop_win_data,
@@ -193,7 +187,8 @@
 static const struct vop_win_phy rk3288_win23_data = {
 	.data_formats = formats_win_lite,
 	.nformats = ARRAY_SIZE(formats_win_lite),
-	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
+	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
+	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
 	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
 	.rb_swap = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 12),
 	.dsp_info = VOP_REG(RK3288_WIN2_DSP_INFO0, 0x0fff0fff, 0),
@@ -215,6 +210,7 @@
 	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
 	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
 	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
+	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
 	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
 	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
 	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
@@ -224,22 +220,10 @@
 	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.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),
 };
 
-static const struct vop_reg_data rk3288_init_reg_table[] = {
-	{RK3288_SYS_CTRL, 0x00c00000},
-	{RK3288_DSP_CTRL0, 0x00000000},
-	{RK3288_WIN0_CTRL0, 0x00000080},
-	{RK3288_WIN1_CTRL0, 0x00000080},
-	/* TODO: Win2/3 support multiple area function, but we haven't found
-	 * a suitable way to use it yet, so let's just use them as other windows
-	 * with only area 0 enabled.
-	 */
-	{RK3288_WIN2_CTRL0, 0x00000010},
-	{RK3288_WIN3_CTRL0, 0x00000010},
-};
-
 /*
  * Note: rk3288 has a dedicated 'cursor' window, however, that window requires
  * special support to get alpha blending working.  For now, just use overlay
@@ -273,8 +257,6 @@
 };
 
 static const struct vop_data rk3288_vop = {
-	.init_table = rk3288_init_reg_table,
-	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
 	.feature = VOP_FEATURE_OUTPUT_RGB10,
 	.intr = &rk3288_vop_intr,
 	.ctrl = &rk3288_ctrl_data,
@@ -328,22 +310,7 @@
 	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
 };
 
-static const struct vop_reg_data rk3399_init_reg_table[] = {
-	{RK3399_SYS_CTRL, 0x2000f800},
-	{RK3399_DSP_CTRL0, 0x00000000},
-	{RK3399_WIN0_CTRL0, 0x00000080},
-	{RK3399_WIN1_CTRL0, 0x00000080},
-	/* TODO: Win2/3 support multiple area function, but we haven't found
-	 * a suitable way to use it yet, so let's just use them as other windows
-	 * with only area 0 enabled.
-	 */
-	{RK3399_WIN2_CTRL0, 0x00000010},
-	{RK3399_WIN3_CTRL0, 0x00000010},
-};
-
 static const struct vop_data rk3399_vop_big = {
-	.init_table = rk3399_init_reg_table,
-	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
 	.feature = VOP_FEATURE_OUTPUT_RGB10,
 	.intr = &rk3399_vop_intr,
 	.ctrl = &rk3399_ctrl_data,
@@ -362,8 +329,6 @@
 };
 
 static const struct vop_data rk3399_vop_lit = {
-	.init_table = rk3399_init_reg_table,
-	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
 	.intr = &rk3399_vop_intr,
 	.ctrl = &rk3399_ctrl_data,
 	/*
-- 
1.9.1

[PATCH v2 1/5] drm/rockchip: vop: get rid of register init table

[Sean Paul]

On Wed, Jul 12, 2017 at 10:03:27AM +0800, Mark Yao wrote:
> Register init table use un-document define, it is unreadable,
> And sometimes we only want to update tiny bits, init table
> method is not friendly, it's diffcult to reuse for difference
> chips.

While I'm happy to see the init_table removed, it seems like the new code is not
equivalent to the old (ie: some register writes have been dropped). How did you
ensure that you're not breaking existing boards that depend on the deleted register
initialization?

Sean

> 
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  6 ++--
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 10 ++-----
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 43 +++--------------------------
>  3 files changed, 10 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 45589d6..7a5f809 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1393,7 +1393,6 @@ static void vop_destroy_crtc(struct vop *vop)
>  static int vop_initial(struct vop *vop)
>  {
>  	const struct vop_data *vop_data = vop->data;
> -	const struct vop_reg_data *init_table = vop_data->init_table;
>  	struct reset_control *ahb_rst;
>  	int i, ret;
>  
> @@ -1453,13 +1452,14 @@ static int vop_initial(struct vop *vop)
>  
>  	memcpy(vop->regsbak, vop->regs, vop->len);
>  
> -	for (i = 0; i < vop_data->table_size; i++)
> -		vop_writel(vop, init_table[i].offset, init_table[i].value);
> +	VOP_CTRL_SET(vop, global_regdone_en, 1);
> +	VOP_CTRL_SET(vop, dsp_blank, 0);
>  
>  	for (i = 0; i < vop_data->win_size; i++) {
>  		const struct vop_win_data *win = &vop_data->win[i];
>  
>  		VOP_WIN_SET(vop, win, enable, 0);
> +		VOP_WIN_SET(vop, win, gate, 1);
>  	}
>  
>  	vop_cfg_done(vop);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 9979fd0..084d3b2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -24,11 +24,6 @@ enum vop_data_format {
>  	VOP_FMT_YUV444SP,
>  };
>  
> -struct vop_reg_data {
> -	uint32_t offset;
> -	uint32_t value;
> -};
> -
>  struct vop_reg {
>  	uint32_t offset;
>  	uint32_t shift;
> @@ -46,6 +41,7 @@ struct vop_ctrl {
>  	struct vop_reg hdmi_en;
>  	struct vop_reg mipi_en;
>  	struct vop_reg dp_en;
> +	struct vop_reg dsp_blank;
>  	struct vop_reg out_mode;
>  	struct vop_reg dither_down;
>  	struct vop_reg dither_up;
> @@ -65,6 +61,7 @@ struct vop_ctrl {
>  
>  	struct vop_reg line_flag_num[2];
>  
> +	struct vop_reg global_regdone_en;
>  	struct vop_reg cfg_done;
>  };
>  
> @@ -115,6 +112,7 @@ struct vop_win_phy {
>  	uint32_t nformats;
>  
>  	struct vop_reg enable;
> +	struct vop_reg gate;
>  	struct vop_reg format;
>  	struct vop_reg rb_swap;
>  	struct vop_reg act_info;
> @@ -136,8 +134,6 @@ struct vop_win_data {
>  };
>  
>  struct vop_data {
> -	const struct vop_reg_data *init_table;
> -	unsigned int table_size;
>  	const struct vop_ctrl *ctrl;
>  	const struct vop_intr *intr;
>  	const struct vop_win_data *win;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index bafd698..00e9d79 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -127,13 +127,7 @@
>  	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>  };
>  
> -static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
> -	{RK3036_DSP_CTRL1, 0x00000000},
> -};
> -
>  static const struct vop_data rk3036_vop = {
> -	.init_table = rk3036_vop_init_reg_table,
> -	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
>  	.ctrl = &rk3036_ctrl_data,
>  	.intr = &rk3036_intr,
>  	.win = rk3036_vop_win_data,
> @@ -193,7 +187,8 @@
>  static const struct vop_win_phy rk3288_win23_data = {
>  	.data_formats = formats_win_lite,
>  	.nformats = ARRAY_SIZE(formats_win_lite),
> -	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
> +	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
> +	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>  	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
>  	.rb_swap = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 12),
>  	.dsp_info = VOP_REG(RK3288_WIN2_DSP_INFO0, 0x0fff0fff, 0),
> @@ -215,6 +210,7 @@
>  	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
>  	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
>  	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
> +	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
>  	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
>  	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
>  	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
> @@ -224,22 +220,10 @@
>  	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.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),
>  };
>  
> -static const struct vop_reg_data rk3288_init_reg_table[] = {
> -	{RK3288_SYS_CTRL, 0x00c00000},
> -	{RK3288_DSP_CTRL0, 0x00000000},
> -	{RK3288_WIN0_CTRL0, 0x00000080},
> -	{RK3288_WIN1_CTRL0, 0x00000080},
> -	/* TODO: Win2/3 support multiple area function, but we haven't found
> -	 * a suitable way to use it yet, so let's just use them as other windows
> -	 * with only area 0 enabled.
> -	 */
> -	{RK3288_WIN2_CTRL0, 0x00000010},
> -	{RK3288_WIN3_CTRL0, 0x00000010},
> -};
> -
>  /*
>   * Note: rk3288 has a dedicated 'cursor' window, however, that window requires
>   * special support to get alpha blending working.  For now, just use overlay
> @@ -273,8 +257,6 @@
>  };
>  
>  static const struct vop_data rk3288_vop = {
> -	.init_table = rk3288_init_reg_table,
> -	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
>  	.feature = VOP_FEATURE_OUTPUT_RGB10,
>  	.intr = &rk3288_vop_intr,
>  	.ctrl = &rk3288_ctrl_data,
> @@ -328,22 +310,7 @@
>  	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
>  };
>  
> -static const struct vop_reg_data rk3399_init_reg_table[] = {
> -	{RK3399_SYS_CTRL, 0x2000f800},
> -	{RK3399_DSP_CTRL0, 0x00000000},
> -	{RK3399_WIN0_CTRL0, 0x00000080},
> -	{RK3399_WIN1_CTRL0, 0x00000080},
> -	/* TODO: Win2/3 support multiple area function, but we haven't found
> -	 * a suitable way to use it yet, so let's just use them as other windows
> -	 * with only area 0 enabled.
> -	 */
> -	{RK3399_WIN2_CTRL0, 0x00000010},
> -	{RK3399_WIN3_CTRL0, 0x00000010},
> -};
> -
>  static const struct vop_data rk3399_vop_big = {
> -	.init_table = rk3399_init_reg_table,
> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>  	.feature = VOP_FEATURE_OUTPUT_RGB10,
>  	.intr = &rk3399_vop_intr,
>  	.ctrl = &rk3399_ctrl_data,
> @@ -362,8 +329,6 @@
>  };
>  
>  static const struct vop_data rk3399_vop_lit = {
> -	.init_table = rk3399_init_reg_table,
> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>  	.intr = &rk3399_vop_intr,
>  	.ctrl = &rk3399_ctrl_data,
>  	/*
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 1/5] drm/rockchip: vop: get rid of register init table

[Mark yao]

On 2017?07?13? 00:47, Sean Paul wrote:
> On Wed, Jul 12, 2017 at 10:03:27AM +0800, Mark Yao wrote:
>> Register init table use un-document define, it is unreadable,
>> And sometimes we only want to update tiny bits, init table
>> method is not friendly, it's diffcult to reuse for difference
>> chips.
> While I'm happy to see the init_table removed, it seems like the new code is not
> equivalent to the old (ie: some register writes have been dropped). How did you
> ensure that you're not breaking existing boards that depend on the deleted register
> initialization?
>
> Sean

All the existing boards works fine on my internal kernel board with the init_table removed,
We had tested all the existing boards, so it's no problem to removed init_table

>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  6 ++--
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 10 ++-----
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 43 +++--------------------------
>>   3 files changed, 10 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index 45589d6..7a5f809 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -1393,7 +1393,6 @@ static void vop_destroy_crtc(struct vop *vop)
>>   static int vop_initial(struct vop *vop)
>>   {
>>   	const struct vop_data *vop_data = vop->data;
>> -	const struct vop_reg_data *init_table = vop_data->init_table;
>>   	struct reset_control *ahb_rst;
>>   	int i, ret;
>>   
>> @@ -1453,13 +1452,14 @@ static int vop_initial(struct vop *vop)
>>   
>>   	memcpy(vop->regsbak, vop->regs, vop->len);
>>   
>> -	for (i = 0; i < vop_data->table_size; i++)
>> -		vop_writel(vop, init_table[i].offset, init_table[i].value);
>> +	VOP_CTRL_SET(vop, global_regdone_en, 1);
>> +	VOP_CTRL_SET(vop, dsp_blank, 0);
>>   
>>   	for (i = 0; i < vop_data->win_size; i++) {
>>   		const struct vop_win_data *win = &vop_data->win[i];
>>   
>>   		VOP_WIN_SET(vop, win, enable, 0);
>> +		VOP_WIN_SET(vop, win, gate, 1);
>>   	}
>>   
>>   	vop_cfg_done(vop);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index 9979fd0..084d3b2 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -24,11 +24,6 @@ enum vop_data_format {
>>   	VOP_FMT_YUV444SP,
>>   };
>>   
>> -struct vop_reg_data {
>> -	uint32_t offset;
>> -	uint32_t value;
>> -};
>> -
>>   struct vop_reg {
>>   	uint32_t offset;
>>   	uint32_t shift;
>> @@ -46,6 +41,7 @@ struct vop_ctrl {
>>   	struct vop_reg hdmi_en;
>>   	struct vop_reg mipi_en;
>>   	struct vop_reg dp_en;
>> +	struct vop_reg dsp_blank;
>>   	struct vop_reg out_mode;
>>   	struct vop_reg dither_down;
>>   	struct vop_reg dither_up;
>> @@ -65,6 +61,7 @@ struct vop_ctrl {
>>   
>>   	struct vop_reg line_flag_num[2];
>>   
>> +	struct vop_reg global_regdone_en;
>>   	struct vop_reg cfg_done;
>>   };
>>   
>> @@ -115,6 +112,7 @@ struct vop_win_phy {
>>   	uint32_t nformats;
>>   
>>   	struct vop_reg enable;
>> +	struct vop_reg gate;
>>   	struct vop_reg format;
>>   	struct vop_reg rb_swap;
>>   	struct vop_reg act_info;
>> @@ -136,8 +134,6 @@ struct vop_win_data {
>>   };
>>   
>>   struct vop_data {
>> -	const struct vop_reg_data *init_table;
>> -	unsigned int table_size;
>>   	const struct vop_ctrl *ctrl;
>>   	const struct vop_intr *intr;
>>   	const struct vop_win_data *win;
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index bafd698..00e9d79 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -127,13 +127,7 @@
>>   	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>>   };
>>   
>> -static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
>> -	{RK3036_DSP_CTRL1, 0x00000000},
>> -};
>> -
>>   static const struct vop_data rk3036_vop = {
>> -	.init_table = rk3036_vop_init_reg_table,
>> -	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
>>   	.ctrl = &rk3036_ctrl_data,
>>   	.intr = &rk3036_intr,
>>   	.win = rk3036_vop_win_data,
>> @@ -193,7 +187,8 @@
>>   static const struct vop_win_phy rk3288_win23_data = {
>>   	.data_formats = formats_win_lite,
>>   	.nformats = ARRAY_SIZE(formats_win_lite),
>> -	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>> +	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
>> +	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>>   	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
>>   	.rb_swap = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 12),
>>   	.dsp_info = VOP_REG(RK3288_WIN2_DSP_INFO0, 0x0fff0fff, 0),
>> @@ -215,6 +210,7 @@
>>   	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
>>   	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
>>   	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
>> +	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
>>   	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
>>   	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
>>   	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
>> @@ -224,22 +220,10 @@
>>   	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>   	.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),
>>   };
>>   
>> -static const struct vop_reg_data rk3288_init_reg_table[] = {
>> -	{RK3288_SYS_CTRL, 0x00c00000},
>> -	{RK3288_DSP_CTRL0, 0x00000000},
>> -	{RK3288_WIN0_CTRL0, 0x00000080},
>> -	{RK3288_WIN1_CTRL0, 0x00000080},
>> -	/* TODO: Win2/3 support multiple area function, but we haven't found
>> -	 * a suitable way to use it yet, so let's just use them as other windows
>> -	 * with only area 0 enabled.
>> -	 */
>> -	{RK3288_WIN2_CTRL0, 0x00000010},
>> -	{RK3288_WIN3_CTRL0, 0x00000010},
>> -};
>> -
>>   /*
>>    * Note: rk3288 has a dedicated 'cursor' window, however, that window requires
>>    * special support to get alpha blending working.  For now, just use overlay
>> @@ -273,8 +257,6 @@
>>   };
>>   
>>   static const struct vop_data rk3288_vop = {
>> -	.init_table = rk3288_init_reg_table,
>> -	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
>>   	.feature = VOP_FEATURE_OUTPUT_RGB10,
>>   	.intr = &rk3288_vop_intr,
>>   	.ctrl = &rk3288_ctrl_data,
>> @@ -328,22 +310,7 @@
>>   	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
>>   };
>>   
>> -static const struct vop_reg_data rk3399_init_reg_table[] = {
>> -	{RK3399_SYS_CTRL, 0x2000f800},
>> -	{RK3399_DSP_CTRL0, 0x00000000},
>> -	{RK3399_WIN0_CTRL0, 0x00000080},
>> -	{RK3399_WIN1_CTRL0, 0x00000080},
>> -	/* TODO: Win2/3 support multiple area function, but we haven't found
>> -	 * a suitable way to use it yet, so let's just use them as other windows
>> -	 * with only area 0 enabled.
>> -	 */
>> -	{RK3399_WIN2_CTRL0, 0x00000010},
>> -	{RK3399_WIN3_CTRL0, 0x00000010},
>> -};
>> -
>>   static const struct vop_data rk3399_vop_big = {
>> -	.init_table = rk3399_init_reg_table,
>> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>>   	.feature = VOP_FEATURE_OUTPUT_RGB10,
>>   	.intr = &rk3399_vop_intr,
>>   	.ctrl = &rk3399_ctrl_data,
>> @@ -362,8 +329,6 @@
>>   };
>>   
>>   static const struct vop_data rk3399_vop_lit = {
>> -	.init_table = rk3399_init_reg_table,
>> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>>   	.intr = &rk3399_vop_intr,
>>   	.ctrl = &rk3399_ctrl_data,
>>   	/*
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


-- 
?ark Yao

[PATCH v2 1/5] drm/rockchip: vop: get rid of register init table

[Sean Paul]

On Thu, Jul 13, 2017 at 09:33:45AM +0800, Mark yao wrote:
> On 2017?07?13? 00:47, Sean Paul wrote:
> > On Wed, Jul 12, 2017 at 10:03:27AM +0800, Mark Yao wrote:
> > > Register init table use un-document define, it is unreadable,
> > > And sometimes we only want to update tiny bits, init table
> > > method is not friendly, it's diffcult to reuse for difference
> > > chips.
> > While I'm happy to see the init_table removed, it seems like the new code is not
> > equivalent to the old (ie: some register writes have been dropped). How did you
> > ensure that you're not breaking existing boards that depend on the deleted register
> > initialization?
> > 
> > Sean
> 
> All the existing boards works fine on my internal kernel board with the init_table removed,
> We had tested all the existing boards, so it's no problem to removed init_table
> 

That begs the question... why was it introduced if it's not necessary?

Sean

> > 
> > > Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> > > ---
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  6 ++--
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 10 ++-----
> > >   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 43 +++--------------------------
> > >   3 files changed, 10 insertions(+), 49 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > index 45589d6..7a5f809 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > @@ -1393,7 +1393,6 @@ static void vop_destroy_crtc(struct vop *vop)
> > >   static int vop_initial(struct vop *vop)
> > >   {
> > >   	const struct vop_data *vop_data = vop->data;
> > > -	const struct vop_reg_data *init_table = vop_data->init_table;
> > >   	struct reset_control *ahb_rst;
> > >   	int i, ret;
> > > @@ -1453,13 +1452,14 @@ static int vop_initial(struct vop *vop)
> > >   	memcpy(vop->regsbak, vop->regs, vop->len);
> > > -	for (i = 0; i < vop_data->table_size; i++)
> > > -		vop_writel(vop, init_table[i].offset, init_table[i].value);
> > > +	VOP_CTRL_SET(vop, global_regdone_en, 1);
> > > +	VOP_CTRL_SET(vop, dsp_blank, 0);
> > >   	for (i = 0; i < vop_data->win_size; i++) {
> > >   		const struct vop_win_data *win = &vop_data->win[i];
> > >   		VOP_WIN_SET(vop, win, enable, 0);
> > > +		VOP_WIN_SET(vop, win, gate, 1);
> > >   	}
> > >   	vop_cfg_done(vop);
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > index 9979fd0..084d3b2 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > @@ -24,11 +24,6 @@ enum vop_data_format {
> > >   	VOP_FMT_YUV444SP,
> > >   };
> > > -struct vop_reg_data {
> > > -	uint32_t offset;
> > > -	uint32_t value;
> > > -};
> > > -
> > >   struct vop_reg {
> > >   	uint32_t offset;
> > >   	uint32_t shift;
> > > @@ -46,6 +41,7 @@ struct vop_ctrl {
> > >   	struct vop_reg hdmi_en;
> > >   	struct vop_reg mipi_en;
> > >   	struct vop_reg dp_en;
> > > +	struct vop_reg dsp_blank;
> > >   	struct vop_reg out_mode;
> > >   	struct vop_reg dither_down;
> > >   	struct vop_reg dither_up;
> > > @@ -65,6 +61,7 @@ struct vop_ctrl {
> > >   	struct vop_reg line_flag_num[2];
> > > +	struct vop_reg global_regdone_en;
> > >   	struct vop_reg cfg_done;
> > >   };
> > > @@ -115,6 +112,7 @@ struct vop_win_phy {
> > >   	uint32_t nformats;
> > >   	struct vop_reg enable;
> > > +	struct vop_reg gate;
> > >   	struct vop_reg format;
> > >   	struct vop_reg rb_swap;
> > >   	struct vop_reg act_info;
> > > @@ -136,8 +134,6 @@ struct vop_win_data {
> > >   };
> > >   struct vop_data {
> > > -	const struct vop_reg_data *init_table;
> > > -	unsigned int table_size;
> > >   	const struct vop_ctrl *ctrl;
> > >   	const struct vop_intr *intr;
> > >   	const struct vop_win_data *win;
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > index bafd698..00e9d79 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > @@ -127,13 +127,7 @@
> > >   	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
> > >   };
> > > -static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
> > > -	{RK3036_DSP_CTRL1, 0x00000000},
> > > -};
> > > -
> > >   static const struct vop_data rk3036_vop = {
> > > -	.init_table = rk3036_vop_init_reg_table,
> > > -	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
> > >   	.ctrl = &rk3036_ctrl_data,
> > >   	.intr = &rk3036_intr,
> > >   	.win = rk3036_vop_win_data,
> > > @@ -193,7 +187,8 @@
> > >   static const struct vop_win_phy rk3288_win23_data = {
> > >   	.data_formats = formats_win_lite,
> > >   	.nformats = ARRAY_SIZE(formats_win_lite),
> > > -	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
> > > +	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
> > > +	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
> > >   	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
> > >   	.rb_swap = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 12),
> > >   	.dsp_info = VOP_REG(RK3288_WIN2_DSP_INFO0, 0x0fff0fff, 0),
> > > @@ -215,6 +210,7 @@
> > >   	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
> > >   	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
> > >   	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
> > > +	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
> > >   	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
> > >   	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
> > >   	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
> > > @@ -224,22 +220,10 @@
> > >   	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
> > >   	.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),
> > >   };
> > > -static const struct vop_reg_data rk3288_init_reg_table[] = {
> > > -	{RK3288_SYS_CTRL, 0x00c00000},
> > > -	{RK3288_DSP_CTRL0, 0x00000000},
> > > -	{RK3288_WIN0_CTRL0, 0x00000080},
> > > -	{RK3288_WIN1_CTRL0, 0x00000080},
> > > -	/* TODO: Win2/3 support multiple area function, but we haven't found
> > > -	 * a suitable way to use it yet, so let's just use them as other windows
> > > -	 * with only area 0 enabled.
> > > -	 */
> > > -	{RK3288_WIN2_CTRL0, 0x00000010},
> > > -	{RK3288_WIN3_CTRL0, 0x00000010},
> > > -};
> > > -
> > >   /*
> > >    * Note: rk3288 has a dedicated 'cursor' window, however, that window requires
> > >    * special support to get alpha blending working.  For now, just use overlay
> > > @@ -273,8 +257,6 @@
> > >   };
> > >   static const struct vop_data rk3288_vop = {
> > > -	.init_table = rk3288_init_reg_table,
> > > -	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
> > >   	.feature = VOP_FEATURE_OUTPUT_RGB10,
> > >   	.intr = &rk3288_vop_intr,
> > >   	.ctrl = &rk3288_ctrl_data,
> > > @@ -328,22 +310,7 @@
> > >   	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
> > >   };
> > > -static const struct vop_reg_data rk3399_init_reg_table[] = {
> > > -	{RK3399_SYS_CTRL, 0x2000f800},
> > > -	{RK3399_DSP_CTRL0, 0x00000000},
> > > -	{RK3399_WIN0_CTRL0, 0x00000080},
> > > -	{RK3399_WIN1_CTRL0, 0x00000080},
> > > -	/* TODO: Win2/3 support multiple area function, but we haven't found
> > > -	 * a suitable way to use it yet, so let's just use them as other windows
> > > -	 * with only area 0 enabled.
> > > -	 */
> > > -	{RK3399_WIN2_CTRL0, 0x00000010},
> > > -	{RK3399_WIN3_CTRL0, 0x00000010},
> > > -};
> > > -
> > >   static const struct vop_data rk3399_vop_big = {
> > > -	.init_table = rk3399_init_reg_table,
> > > -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
> > >   	.feature = VOP_FEATURE_OUTPUT_RGB10,
> > >   	.intr = &rk3399_vop_intr,
> > >   	.ctrl = &rk3399_ctrl_data,
> > > @@ -362,8 +329,6 @@
> > >   };
> > >   static const struct vop_data rk3399_vop_lit = {
> > > -	.init_table = rk3399_init_reg_table,
> > > -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
> > >   	.intr = &rk3399_vop_intr,
> > >   	.ctrl = &rk3399_ctrl_data,
> > >   	/*
> > > -- 
> > > 1.9.1
> > > 
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> -- 
> ?ark Yao
> 
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 1/5] drm/rockchip: vop: get rid of register init table

[Mark yao]

On 2017?07?14? 04:29, Sean Paul wrote:
> On Thu, Jul 13, 2017 at 09:33:45AM +0800, Mark yao wrote:
>> On 2017?07?13? 00:47, Sean Paul wrote:
>>> On Wed, Jul 12, 2017 at 10:03:27AM +0800, Mark Yao wrote:
>>>> Register init table use un-document define, it is unreadable,
>>>> And sometimes we only want to update tiny bits, init table
>>>> method is not friendly, it's diffcult to reuse for difference
>>>> chips.
>>> While I'm happy to see the init_table removed, it seems like the new code is not
>>> equivalent to the old (ie: some register writes have been dropped). How did you
>>> ensure that you're not breaking existing boards that depend on the deleted register
>>> initialization?
>>>
>>> Sean
>> All the existing boards works fine on my internal kernel board with the init_table removed,
>> We had tested all the existing boards, so it's no problem to removed init_table
>>
> That begs the question... why was it introduced if it's not necessary?
Hi Sean

Some registers need to be initialized when vop power on, global_config_done, blank, win gate, etc.
Previously init_table was introduced to handle the initialization of these registers.

So, when we remove the init_table mechanism, we need to re-join the initialization of these registers,
this job is already done in this patch.

I think the title "get rid of" caused some doubts, it's not just remove init_table mechanism,
to be exact, direct setting needed register instead of handling with init_table.

I will make it cleaner next version.

Thanks.

>
> Sean
>
>>>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>>>> ---
>>>>    drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  6 ++--
>>>>    drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 10 ++-----
>>>>    drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 43 +++--------------------------
>>>>    3 files changed, 10 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> index 45589d6..7a5f809 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>>>> @@ -1393,7 +1393,6 @@ static void vop_destroy_crtc(struct vop *vop)
>>>>    static int vop_initial(struct vop *vop)
>>>>    {
>>>>    	const struct vop_data *vop_data = vop->data;
>>>> -	const struct vop_reg_data *init_table = vop_data->init_table;
>>>>    	struct reset_control *ahb_rst;
>>>>    	int i, ret;
>>>> @@ -1453,13 +1452,14 @@ static int vop_initial(struct vop *vop)
>>>>    	memcpy(vop->regsbak, vop->regs, vop->len);
>>>> -	for (i = 0; i < vop_data->table_size; i++)
>>>> -		vop_writel(vop, init_table[i].offset, init_table[i].value);
>>>> +	VOP_CTRL_SET(vop, global_regdone_en, 1);
>>>> +	VOP_CTRL_SET(vop, dsp_blank, 0);
>>>>    	for (i = 0; i < vop_data->win_size; i++) {
>>>>    		const struct vop_win_data *win = &vop_data->win[i];
>>>>    		VOP_WIN_SET(vop, win, enable, 0);
>>>> +		VOP_WIN_SET(vop, win, gate, 1);
>>>>    	}
>>>>    	vop_cfg_done(vop);
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>>>> index 9979fd0..084d3b2 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>>>> @@ -24,11 +24,6 @@ enum vop_data_format {
>>>>    	VOP_FMT_YUV444SP,
>>>>    };
>>>> -struct vop_reg_data {
>>>> -	uint32_t offset;
>>>> -	uint32_t value;
>>>> -};
>>>> -
>>>>    struct vop_reg {
>>>>    	uint32_t offset;
>>>>    	uint32_t shift;
>>>> @@ -46,6 +41,7 @@ struct vop_ctrl {
>>>>    	struct vop_reg hdmi_en;
>>>>    	struct vop_reg mipi_en;
>>>>    	struct vop_reg dp_en;
>>>> +	struct vop_reg dsp_blank;
>>>>    	struct vop_reg out_mode;
>>>>    	struct vop_reg dither_down;
>>>>    	struct vop_reg dither_up;
>>>> @@ -65,6 +61,7 @@ struct vop_ctrl {
>>>>    	struct vop_reg line_flag_num[2];
>>>> +	struct vop_reg global_regdone_en;
>>>>    	struct vop_reg cfg_done;
>>>>    };
>>>> @@ -115,6 +112,7 @@ struct vop_win_phy {
>>>>    	uint32_t nformats;
>>>>    	struct vop_reg enable;
>>>> +	struct vop_reg gate;
>>>>    	struct vop_reg format;
>>>>    	struct vop_reg rb_swap;
>>>>    	struct vop_reg act_info;
>>>> @@ -136,8 +134,6 @@ struct vop_win_data {
>>>>    };
>>>>    struct vop_data {
>>>> -	const struct vop_reg_data *init_table;
>>>> -	unsigned int table_size;
>>>>    	const struct vop_ctrl *ctrl;
>>>>    	const struct vop_intr *intr;
>>>>    	const struct vop_win_data *win;
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>>>> index bafd698..00e9d79 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>>>> @@ -127,13 +127,7 @@
>>>>    	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>>>>    };
>>>> -static const struct vop_reg_data rk3036_vop_init_reg_table[] = {
>>>> -	{RK3036_DSP_CTRL1, 0x00000000},
>>>> -};
>>>> -
>>>>    static const struct vop_data rk3036_vop = {
>>>> -	.init_table = rk3036_vop_init_reg_table,
>>>> -	.table_size = ARRAY_SIZE(rk3036_vop_init_reg_table),
>>>>    	.ctrl = &rk3036_ctrl_data,
>>>>    	.intr = &rk3036_intr,
>>>>    	.win = rk3036_vop_win_data,
>>>> @@ -193,7 +187,8 @@
>>>>    static const struct vop_win_phy rk3288_win23_data = {
>>>>    	.data_formats = formats_win_lite,
>>>>    	.nformats = ARRAY_SIZE(formats_win_lite),
>>>> -	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>>>> +	.enable = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 4),
>>>> +	.gate = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 0),
>>>>    	.format = VOP_REG(RK3288_WIN2_CTRL0, 0x7, 1),
>>>>    	.rb_swap = VOP_REG(RK3288_WIN2_CTRL0, 0x1, 12),
>>>>    	.dsp_info = VOP_REG(RK3288_WIN2_DSP_INFO0, 0x0fff0fff, 0),
>>>> @@ -215,6 +210,7 @@
>>>>    	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
>>>>    	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
>>>>    	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
>>>> +	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
>>>>    	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
>>>>    	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
>>>>    	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
>>>> @@ -224,22 +220,10 @@
>>>>    	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>>>    	.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),
>>>>    };
>>>> -static const struct vop_reg_data rk3288_init_reg_table[] = {
>>>> -	{RK3288_SYS_CTRL, 0x00c00000},
>>>> -	{RK3288_DSP_CTRL0, 0x00000000},
>>>> -	{RK3288_WIN0_CTRL0, 0x00000080},
>>>> -	{RK3288_WIN1_CTRL0, 0x00000080},
>>>> -	/* TODO: Win2/3 support multiple area function, but we haven't found
>>>> -	 * a suitable way to use it yet, so let's just use them as other windows
>>>> -	 * with only area 0 enabled.
>>>> -	 */
>>>> -	{RK3288_WIN2_CTRL0, 0x00000010},
>>>> -	{RK3288_WIN3_CTRL0, 0x00000010},
>>>> -};
>>>> -
>>>>    /*
>>>>     * Note: rk3288 has a dedicated 'cursor' window, however, that window requires
>>>>     * special support to get alpha blending working.  For now, just use overlay
>>>> @@ -273,8 +257,6 @@
>>>>    };
>>>>    static const struct vop_data rk3288_vop = {
>>>> -	.init_table = rk3288_init_reg_table,
>>>> -	.table_size = ARRAY_SIZE(rk3288_init_reg_table),
>>>>    	.feature = VOP_FEATURE_OUTPUT_RGB10,
>>>>    	.intr = &rk3288_vop_intr,
>>>>    	.ctrl = &rk3288_ctrl_data,
>>>> @@ -328,22 +310,7 @@
>>>>    	.clear = VOP_REG_MASK(RK3399_INTR_CLEAR0, 0xffff, 0),
>>>>    };
>>>> -static const struct vop_reg_data rk3399_init_reg_table[] = {
>>>> -	{RK3399_SYS_CTRL, 0x2000f800},
>>>> -	{RK3399_DSP_CTRL0, 0x00000000},
>>>> -	{RK3399_WIN0_CTRL0, 0x00000080},
>>>> -	{RK3399_WIN1_CTRL0, 0x00000080},
>>>> -	/* TODO: Win2/3 support multiple area function, but we haven't found
>>>> -	 * a suitable way to use it yet, so let's just use them as other windows
>>>> -	 * with only area 0 enabled.
>>>> -	 */
>>>> -	{RK3399_WIN2_CTRL0, 0x00000010},
>>>> -	{RK3399_WIN3_CTRL0, 0x00000010},
>>>> -};
>>>> -
>>>>    static const struct vop_data rk3399_vop_big = {
>>>> -	.init_table = rk3399_init_reg_table,
>>>> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>>>>    	.feature = VOP_FEATURE_OUTPUT_RGB10,
>>>>    	.intr = &rk3399_vop_intr,
>>>>    	.ctrl = &rk3399_ctrl_data,
>>>> @@ -362,8 +329,6 @@
>>>>    };
>>>>    static const struct vop_data rk3399_vop_lit = {
>>>> -	.init_table = rk3399_init_reg_table,
>>>> -	.table_size = ARRAY_SIZE(rk3399_init_reg_table),
>>>>    	.intr = &rk3399_vop_intr,
>>>>    	.ctrl = &rk3399_ctrl_data,
>>>>    	/*
>>>> -- 
>>>> 1.9.1
>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>> -- 
>> ?ark Yao
>>
>>


-- 
?ark Yao

[PATCH v2 2/5] drm/rockchip: vop: support verify registers with vop version

[Mark Yao]

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 66 +++++++++++++++++++++--------
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 18 ++++++--
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 20 ++++++---
 3 files changed, 77 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 7a5f809..a9180fd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -42,33 +42,60 @@
 #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 VOP_REG_SUPPORT(vop, reg) \
+		(!reg.major || (reg.major == VOP_MAJOR(vop->data->version) && \
+		reg.begin_minor <= VOP_MINOR(vop->data->version) && \
+		reg.end_minor >= VOP_MINOR(vop->data->version) && \
+		reg.mask))
 
-#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
-		vop_mask_write(x, off, mask, shift, v, write_mask, false)
+#define VOP_WIN_SUPPORT(vop, win, name) \
+		VOP_REG_SUPPORT(vop, win->phy->name)
 
-#define REG_SET(x, base, reg, v, mode) \
-		__REG_SET_##mode(x, base + reg.offset, \
-				 reg.mask, reg.shift, v, reg.write_mask)
-#define REG_SET_MASK(x, base, reg, mask, v, mode) \
-		__REG_SET_##mode(x, base + reg.offset, \
-				 mask, reg.shift, v, reg.write_mask)
+#define VOP_CTRL_SUPPORT(vop, name) \
+		VOP_REG_SUPPORT(vop, vop->data->ctrl->name)
+
+#define VOP_INTR_SUPPORT(vop, name) \
+		VOP_REG_SUPPORT(vop, vop->data->intr->name)
+
+#define __REG_SET(x, off, mask, shift, v, write_mask, relaxed) \
+		vop_mask_write(x, off, mask, shift, v, write_mask, relaxed)
+
+#define _REG_SET(vop, name, off, reg, mask, v, relaxed) \
+	do { \
+		if (VOP_REG_SUPPORT(vop, reg)) \
+			__REG_SET(vop, off + reg.offset, mask, reg.shift, \
+				  v, reg.write_mask, relaxed); \
+		else \
+			dev_dbg(vop->dev, "Warning: not support "#name"\n"); \
+	} while (0)
+
+#define REG_SET(x, name, off, reg, v, relaxed) \
+		_REG_SET(x, name, off, reg, reg.mask, v, relaxed)
+#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \
+		_REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed)
 
 #define VOP_WIN_SET(x, win, name, v) \
-		REG_SET(x, win->base, win->phy->name, v, RELAXED)
+		REG_SET(x, name, win->base, win->phy->name, v, true)
+#define VOP_WIN_SET_EXT(x, win, ext, name, v) \
+		REG_SET(x, name, 0, win->ext->name, v, true)
 #define VOP_SCL_SET(x, win, name, v) \
-		REG_SET(x, win->base, win->phy->scl->name, v, RELAXED)
+		REG_SET(x, name, win->base, win->phy->scl->name, v, true)
 #define VOP_SCL_SET_EXT(x, win, name, v) \
-		REG_SET(x, win->base, win->phy->scl->ext->name, v, RELAXED)
+		REG_SET(x, name, win->base, win->phy->scl->ext->name, v, true)
+
 #define VOP_CTRL_SET(x, name, v) \
-		REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
+		REG_SET(x, name, 0, (x)->data->ctrl->name, v, false)
 
 #define VOP_INTR_GET(vop, name) \
 		vop_read_reg(vop, 0, &vop->data->ctrl->name)
 
-#define VOP_INTR_SET(vop, name, mask, v) \
-		REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v, NORMAL)
+#define VOP_INTR_SET(vop, name, v) \
+		REG_SET(vop, name, 0, vop->data->intr->name, \
+			v, false)
+#define VOP_INTR_SET_MASK(vop, name, mask, v) \
+		REG_SET_MASK(vop, name, 0, vop->data->intr->name, \
+			     mask, v, false)
+
 #define VOP_INTR_SET_TYPE(vop, name, type, v) \
 	do { \
 		int i, reg = 0, mask = 0; \
@@ -78,13 +105,16 @@
 				mask |= 1 << i; \
 			} \
 		} \
-		VOP_INTR_SET(vop, name, mask, reg); \
+		VOP_INTR_SET_MASK(vop, name, mask, reg); \
 	} while (0)
 #define VOP_INTR_GET_TYPE(vop, name, type) \
 		vop_get_intr_type(vop, &vop->data->intr->name, type)
 
+#define VOP_CTRL_GET(x, name) \
+		vop_read_reg(x, 0, &vop->data->ctrl->name)
+
 #define VOP_WIN_GET(x, win, name) \
-		vop_read_reg(x, win->base, &win->phy->name)
+		vop_read_reg(x, win->offset, win->phy->name)
 
 #define VOP_WIN_GET_YRGBADDR(vop, win) \
 		vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 084d3b2..e4de890 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -15,6 +15,14 @@
 #ifndef _ROCKCHIP_DRM_VOP_H
 #define _ROCKCHIP_DRM_VOP_H
 
+/*
+ * major: IP major vertion, used for IP structure
+ * minor: big feature change under same structure
+ */
+#define VOP_VERSION(major, minor)	((major) << 8 | (minor))
+#define VOP_MAJOR(version)		((version) >> 8)
+#define VOP_MINOR(version)		((version) & 0xff)
+
 enum vop_data_format {
 	VOP_FMT_ARGB8888 = 0,
 	VOP_FMT_RGB888,
@@ -25,10 +33,13 @@ enum vop_data_format {
 };
 
 struct vop_reg {
-	uint32_t offset;
-	uint32_t shift;
 	uint32_t mask;
-	bool write_mask;
+	uint32_t offset:12;
+	uint32_t shift:5;
+	uint32_t begin_minor:4;
+	uint32_t end_minor:4;
+	uint32_t major:3;
+	uint32_t write_mask:1;
 };
 
 struct vop_ctrl {
@@ -134,6 +145,7 @@ struct vop_win_data {
 };
 
 struct vop_data {
+	uint32_t version;
 	const struct vop_ctrl *ctrl;
 	const struct vop_intr *intr;
 	const struct vop_win_data *win;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 00e9d79..7744603 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -20,17 +20,25 @@
 #include "rockchip_drm_vop.h"
 #include "rockchip_vop_reg.h"
 
-#define VOP_REG(off, _mask, s) \
+#define VOP_REG_VER_MASK(off, _mask, s, _write_mask, _major, \
+			 _begin_minor, _end_minor) \
 		{.offset = off, \
 		 .mask = _mask, \
 		 .shift = s, \
-		 .write_mask = false,}
+		 .write_mask = _write_mask, \
+		 .major = _major, \
+		 .begin_minor = _begin_minor, \
+		 .end_minor = _end_minor,}
+
+#define VOP_REG_VER(off, _mask, s, _major, _begin_minor, _end_minor) \
+		VOP_REG_VER_MASK(off, _mask, s, false, \
+				 _major, _begin_minor, _end_minor)
+
+#define VOP_REG(off, _mask, s) \
+		VOP_REG_VER(off, _mask, s, 0, 0, -1)
 
 #define VOP_REG_MASK(off, _mask, s) \
-		{.offset = off, \
-		 .mask = _mask, \
-		 .shift = s, \
-		 .write_mask = true,}
+		VOP_REG_VER_MASK(off, _mask, s, true, 0, 0, -1)
 
 static const uint32_t formats_win_full[] = {
 	DRM_FORMAT_XRGB8888,
-- 
1.9.1

[PATCH v2 2/5] drm/rockchip: vop: support verify registers with vop version

[Sean Paul]

On Wed, Jul 12, 2017 at 10:03:38AM +0800, Mark Yao wrote:

Please add a commit message describing *what* and *why* you are making the
change.

> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 66 +++++++++++++++++++++--------
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 18 ++++++--
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 20 ++++++---
>  3 files changed, 77 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 7a5f809..a9180fd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -42,33 +42,60 @@
>  #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 VOP_REG_SUPPORT(vop, reg) \
> +		(!reg.major || (reg.major == VOP_MAJOR(vop->data->version) && \
> +		reg.begin_minor <= VOP_MINOR(vop->data->version) && \
> +		reg.end_minor >= VOP_MINOR(vop->data->version) && \
> +		reg.mask))

This would be better suited as a static inline function. As would many of the
macros below.

>  
> -#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
> -		vop_mask_write(x, off, mask, shift, v, write_mask, false)
> +#define VOP_WIN_SUPPORT(vop, win, name) \
> +		VOP_REG_SUPPORT(vop, win->phy->name)
>  
> -#define REG_SET(x, base, reg, v, mode) \
> -		__REG_SET_##mode(x, base + reg.offset, \
> -				 reg.mask, reg.shift, v, reg.write_mask)
> -#define REG_SET_MASK(x, base, reg, mask, v, mode) \
> -		__REG_SET_##mode(x, base + reg.offset, \
> -				 mask, reg.shift, v, reg.write_mask)
> +#define VOP_CTRL_SUPPORT(vop, name) \
> +		VOP_REG_SUPPORT(vop, vop->data->ctrl->name)
> +
> +#define VOP_INTR_SUPPORT(vop, name) \
> +		VOP_REG_SUPPORT(vop, vop->data->intr->name)
> +
> +#define __REG_SET(x, off, mask, shift, v, write_mask, relaxed) \
> +		vop_mask_write(x, off, mask, shift, v, write_mask, relaxed)

There's really no point to this, just call vop_mask_write directly.

> +
> +#define _REG_SET(vop, name, off, reg, mask, v, relaxed) \
> +	do { \
> +		if (VOP_REG_SUPPORT(vop, reg)) \
> +			__REG_SET(vop, off + reg.offset, mask, reg.shift, \

s/mask/reg.mask & mask/

> +				  v, reg.write_mask, relaxed); \
> +		else \
> +			dev_dbg(vop->dev, "Warning: not support "#name"\n"); \
> +	} while (0)


Also better as static inline, IMO.

> +
> +#define REG_SET(x, name, off, reg, v, relaxed) \
> +		_REG_SET(x, name, off, reg, reg.mask, v, relaxed)

s/reg.mask/~0/

> +#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \
> +		_REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed)

s/reg.mask &//

Also, these can become static inline functions as well.

>  
>  #define VOP_WIN_SET(x, win, name, v) \
> -		REG_SET(x, win->base, win->phy->name, v, RELAXED)
> +		REG_SET(x, name, win->base, win->phy->name, v, true)
> +#define VOP_WIN_SET_EXT(x, win, ext, name, v) \
> +		REG_SET(x, name, 0, win->ext->name, v, true)
>  #define VOP_SCL_SET(x, win, name, v) \
> -		REG_SET(x, win->base, win->phy->scl->name, v, RELAXED)
> +		REG_SET(x, name, win->base, win->phy->scl->name, v, true)
>  #define VOP_SCL_SET_EXT(x, win, name, v) \
> -		REG_SET(x, win->base, win->phy->scl->ext->name, v, RELAXED)
> +		REG_SET(x, name, win->base, win->phy->scl->ext->name, v, true)
> +
>  #define VOP_CTRL_SET(x, name, v) \
> -		REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
> +		REG_SET(x, name, 0, (x)->data->ctrl->name, v, false)
>  
>  #define VOP_INTR_GET(vop, name) \
>  		vop_read_reg(vop, 0, &vop->data->ctrl->name)
>  
> -#define VOP_INTR_SET(vop, name, mask, v) \
> -		REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v, NORMAL)
> +#define VOP_INTR_SET(vop, name, v) \
> +		REG_SET(vop, name, 0, vop->data->intr->name, \
> +			v, false)
> +#define VOP_INTR_SET_MASK(vop, name, mask, v) \
> +		REG_SET_MASK(vop, name, 0, vop->data->intr->name, \
> +			     mask, v, false)
> +
>  #define VOP_INTR_SET_TYPE(vop, name, type, v) \
>  	do { \
>  		int i, reg = 0, mask = 0; \
> @@ -78,13 +105,16 @@
>  				mask |= 1 << i; \
>  			} \
>  		} \
> -		VOP_INTR_SET(vop, name, mask, reg); \
> +		VOP_INTR_SET_MASK(vop, name, mask, reg); \
>  	} while (0)
>  #define VOP_INTR_GET_TYPE(vop, name, type) \
>  		vop_get_intr_type(vop, &vop->data->intr->name, type)
>  
> +#define VOP_CTRL_GET(x, name) \
> +		vop_read_reg(x, 0, &vop->data->ctrl->name)
> +
>  #define VOP_WIN_GET(x, win, name) \
> -		vop_read_reg(x, win->base, &win->phy->name)
> +		vop_read_reg(x, win->offset, win->phy->name)
>  
>  #define VOP_WIN_GET_YRGBADDR(vop, win) \
>  		vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 084d3b2..e4de890 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -15,6 +15,14 @@
>  #ifndef _ROCKCHIP_DRM_VOP_H
>  #define _ROCKCHIP_DRM_VOP_H
>  
> +/*
> + * major: IP major vertion, used for IP structure

s/vertion/version

> + * minor: big feature change under same structure
> + */
> +#define VOP_VERSION(major, minor)	((major) << 8 | (minor))
> +#define VOP_MAJOR(version)		((version) >> 8)
> +#define VOP_MINOR(version)		((version) & 0xff)
> +
>  enum vop_data_format {
>  	VOP_FMT_ARGB8888 = 0,
>  	VOP_FMT_RGB888,
> @@ -25,10 +33,13 @@ enum vop_data_format {
>  };
>  
>  struct vop_reg {
> -	uint32_t offset;
> -	uint32_t shift;
>  	uint32_t mask;
> -	bool write_mask;
> +	uint32_t offset:12;
> +	uint32_t shift:5;
> +	uint32_t begin_minor:4;
> +	uint32_t end_minor:4;
> +	uint32_t major:3;
> +	uint32_t write_mask:1;

Why are you packing this?

>  };
>  
>  struct vop_ctrl {
> @@ -134,6 +145,7 @@ struct vop_win_data {
>  };
>  
>  struct vop_data {
> +	uint32_t version;
>  	const struct vop_ctrl *ctrl;
>  	const struct vop_intr *intr;
>  	const struct vop_win_data *win;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 00e9d79..7744603 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -20,17 +20,25 @@
>  #include "rockchip_drm_vop.h"
>  #include "rockchip_vop_reg.h"
>  
> -#define VOP_REG(off, _mask, s) \
> +#define VOP_REG_VER_MASK(off, _mask, s, _write_mask, _major, \
> +			 _begin_minor, _end_minor) \
>  		{.offset = off, \
>  		 .mask = _mask, \
>  		 .shift = s, \
> -		 .write_mask = false,}
> +		 .write_mask = _write_mask, \
> +		 .major = _major, \
> +		 .begin_minor = _begin_minor, \
> +		 .end_minor = _end_minor,}
> +
> +#define VOP_REG_VER(off, _mask, s, _major, _begin_minor, _end_minor) \
> +		VOP_REG_VER_MASK(off, _mask, s, false, \
> +				 _major, _begin_minor, _end_minor)
> +
> +#define VOP_REG(off, _mask, s) \
> +		VOP_REG_VER(off, _mask, s, 0, 0, -1)
>  
>  #define VOP_REG_MASK(off, _mask, s) \
> -		{.offset = off, \
> -		 .mask = _mask, \
> -		 .shift = s, \
> -		 .write_mask = true,}
> +		VOP_REG_VER_MASK(off, _mask, s, true, 0, 0, -1)
>  
>  static const uint32_t formats_win_full[] = {
>  	DRM_FORMAT_XRGB8888,
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 2/5] drm/rockchip: vop: support verify registers with vop version

[Mark yao]

On 2017?07?13? 01:53, Sean Paul wrote:
> On Wed, Jul 12, 2017 at 10:03:38AM +0800, Mark Yao wrote:
>
> Please add a commit message describing *what* and *why* you are making the
> change.

Got it, I will fix it at next version.

Thanks.
>
>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 66 +++++++++++++++++++++--------
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 18 ++++++--
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 20 ++++++---
>>   3 files changed, 77 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index 7a5f809..a9180fd 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -42,33 +42,60 @@
>>   #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 VOP_REG_SUPPORT(vop, reg) \
>> +		(!reg.major || (reg.major == VOP_MAJOR(vop->data->version) && \
>> +		reg.begin_minor <= VOP_MINOR(vop->data->version) && \
>> +		reg.end_minor >= VOP_MINOR(vop->data->version) && \
>> +		reg.mask))
> This would be better suited as a static inline function. As would many of the
> macros below.

Got it, will fix it at next version.
>
>>   
>> -#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
>> -		vop_mask_write(x, off, mask, shift, v, write_mask, false)
>> +#define VOP_WIN_SUPPORT(vop, win, name) \
>> +		VOP_REG_SUPPORT(vop, win->phy->name)
>>   
>> -#define REG_SET(x, base, reg, v, mode) \
>> -		__REG_SET_##mode(x, base + reg.offset, \
>> -				 reg.mask, reg.shift, v, reg.write_mask)
>> -#define REG_SET_MASK(x, base, reg, mask, v, mode) \
>> -		__REG_SET_##mode(x, base + reg.offset, \
>> -				 mask, reg.shift, v, reg.write_mask)
>> +#define VOP_CTRL_SUPPORT(vop, name) \
>> +		VOP_REG_SUPPORT(vop, vop->data->ctrl->name)
>> +
>> +#define VOP_INTR_SUPPORT(vop, name) \
>> +		VOP_REG_SUPPORT(vop, vop->data->intr->name)
>> +
>> +#define __REG_SET(x, off, mask, shift, v, write_mask, relaxed) \
>> +		vop_mask_write(x, off, mask, shift, v, write_mask, relaxed)
> There's really no point to this, just call vop_mask_write directly.

Got it, will fix it at next version.
>
>> +
>> +#define _REG_SET(vop, name, off, reg, mask, v, relaxed) \
>> +	do { \
>> +		if (VOP_REG_SUPPORT(vop, reg)) \
>> +			__REG_SET(vop, off + reg.offset, mask, reg.shift, \
> s/mask/reg.mask & mask/

Got it, will fix it at next version.
>
>> +				  v, reg.write_mask, relaxed); \
>> +		else \
>> +			dev_dbg(vop->dev, "Warning: not support "#name"\n"); \
>> +	} while (0)
>
> Also better as static inline, IMO.

Good idea, I will try it.

>
>> +
>> +#define REG_SET(x, name, off, reg, v, relaxed) \
>> +		_REG_SET(x, name, off, reg, reg.mask, v, relaxed)
> s/reg.mask/~0/

Got it, will fix it at next version.
>
>> +#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \
>> +		_REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed)
> s/reg.mask &//
>
> Also, these can become static inline functions as well.

Got it, will fix it at next version.
>
>>   
>>   #define VOP_WIN_SET(x, win, name, v) \
>> -		REG_SET(x, win->base, win->phy->name, v, RELAXED)
>> +		REG_SET(x, name, win->base, win->phy->name, v, true)
>> +#define VOP_WIN_SET_EXT(x, win, ext, name, v) \
>> +		REG_SET(x, name, 0, win->ext->name, v, true)
>>   #define VOP_SCL_SET(x, win, name, v) \
>> -		REG_SET(x, win->base, win->phy->scl->name, v, RELAXED)
>> +		REG_SET(x, name, win->base, win->phy->scl->name, v, true)
>>   #define VOP_SCL_SET_EXT(x, win, name, v) \
>> -		REG_SET(x, win->base, win->phy->scl->ext->name, v, RELAXED)
>> +		REG_SET(x, name, win->base, win->phy->scl->ext->name, v, true)
>> +
>>   #define VOP_CTRL_SET(x, name, v) \
>> -		REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
>> +		REG_SET(x, name, 0, (x)->data->ctrl->name, v, false)
>>   
>>   #define VOP_INTR_GET(vop, name) \
>>   		vop_read_reg(vop, 0, &vop->data->ctrl->name)
>>   
>> -#define VOP_INTR_SET(vop, name, mask, v) \
>> -		REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v, NORMAL)
>> +#define VOP_INTR_SET(vop, name, v) \
>> +		REG_SET(vop, name, 0, vop->data->intr->name, \
>> +			v, false)
>> +#define VOP_INTR_SET_MASK(vop, name, mask, v) \
>> +		REG_SET_MASK(vop, name, 0, vop->data->intr->name, \
>> +			     mask, v, false)
>> +
>>   #define VOP_INTR_SET_TYPE(vop, name, type, v) \
>>   	do { \
>>   		int i, reg = 0, mask = 0; \
>> @@ -78,13 +105,16 @@
>>   				mask |= 1 << i; \
>>   			} \
>>   		} \
>> -		VOP_INTR_SET(vop, name, mask, reg); \
>> +		VOP_INTR_SET_MASK(vop, name, mask, reg); \
>>   	} while (0)
>>   #define VOP_INTR_GET_TYPE(vop, name, type) \
>>   		vop_get_intr_type(vop, &vop->data->intr->name, type)
>>   
>> +#define VOP_CTRL_GET(x, name) \
>> +		vop_read_reg(x, 0, &vop->data->ctrl->name)
>> +
>>   #define VOP_WIN_GET(x, win, name) \
>> -		vop_read_reg(x, win->base, &win->phy->name)
>> +		vop_read_reg(x, win->offset, win->phy->name)
>>   
>>   #define VOP_WIN_GET_YRGBADDR(vop, win) \
>>   		vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index 084d3b2..e4de890 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -15,6 +15,14 @@
>>   #ifndef _ROCKCHIP_DRM_VOP_H
>>   #define _ROCKCHIP_DRM_VOP_H
>>   
>> +/*
>> + * major: IP major vertion, used for IP structure
> s/vertion/version

Got it, will fix it at next version.
Thanks.

>
>> + * minor: big feature change under same structure
>> + */
>> +#define VOP_VERSION(major, minor)	((major) << 8 | (minor))
>> +#define VOP_MAJOR(version)		((version) >> 8)
>> +#define VOP_MINOR(version)		((version) & 0xff)
>> +
>>   enum vop_data_format {
>>   	VOP_FMT_ARGB8888 = 0,
>>   	VOP_FMT_RGB888,
>> @@ -25,10 +33,13 @@ enum vop_data_format {
>>   };
>>   
>>   struct vop_reg {
>> -	uint32_t offset;
>> -	uint32_t shift;
>>   	uint32_t mask;
>> -	bool write_mask;
>> +	uint32_t offset:12;
>> +	uint32_t shift:5;
>> +	uint32_t begin_minor:4;
>> +	uint32_t end_minor:4;
>> +	uint32_t major:3;
>> +	uint32_t write_mask:1;
> Why are you packing this?

make struct vop_reg not too big, jus u64 size, struct vop_reg use a lot on register define, packing it would reduce register table size.

>
>>   };
>>   
>>   struct vop_ctrl {
>> @@ -134,6 +145,7 @@ struct vop_win_data {
>>   };
>>   
>>   struct vop_data {
>> +	uint32_t version;
>>   	const struct vop_ctrl *ctrl;
>>   	const struct vop_intr *intr;
>>   	const struct vop_win_data *win;
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 00e9d79..7744603 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -20,17 +20,25 @@
>>   #include "rockchip_drm_vop.h"
>>   #include "rockchip_vop_reg.h"
>>   
>> -#define VOP_REG(off, _mask, s) \
>> +#define VOP_REG_VER_MASK(off, _mask, s, _write_mask, _major, \
>> +			 _begin_minor, _end_minor) \
>>   		{.offset = off, \
>>   		 .mask = _mask, \
>>   		 .shift = s, \
>> -		 .write_mask = false,}
>> +		 .write_mask = _write_mask, \
>> +		 .major = _major, \
>> +		 .begin_minor = _begin_minor, \
>> +		 .end_minor = _end_minor,}
>> +
>> +#define VOP_REG_VER(off, _mask, s, _major, _begin_minor, _end_minor) \
>> +		VOP_REG_VER_MASK(off, _mask, s, false, \
>> +				 _major, _begin_minor, _end_minor)
>> +
>> +#define VOP_REG(off, _mask, s) \
>> +		VOP_REG_VER(off, _mask, s, 0, 0, -1)
>>   
>>   #define VOP_REG_MASK(off, _mask, s) \
>> -		{.offset = off, \
>> -		 .mask = _mask, \
>> -		 .shift = s, \
>> -		 .write_mask = true,}
>> +		VOP_REG_VER_MASK(off, _mask, s, true, 0, 0, -1)
>>   
>>   static const uint32_t formats_win_full[] = {
>>   	DRM_FORMAT_XRGB8888,
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel


-- 
?ark Yao

[PATCH v2 2/5] drm/rockchip: vop: support verify registers with vop version

[Sean Paul]

On Thu, Jul 13, 2017 at 09:45:12AM +0800, Mark yao wrote:
> On 2017?07?13? 01:53, Sean Paul wrote:
> > On Wed, Jul 12, 2017 at 10:03:38AM +0800, Mark Yao wrote:
> > 
> > Please add a commit message describing *what* and *why* you are making the
> > change.
> 
> Got it, I will fix it at next version.
> 
> Thanks.
> > 
> > > Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> > > ---
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 66 +++++++++++++++++++++--------
> > >   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 18 ++++++--
> > >   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 20 ++++++---
> > >   3 files changed, 77 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > index 7a5f809..a9180fd 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > > @@ -42,33 +42,60 @@
> > >   #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 VOP_REG_SUPPORT(vop, reg) \
> > > +		(!reg.major || (reg.major == VOP_MAJOR(vop->data->version) && \
> > > +		reg.begin_minor <= VOP_MINOR(vop->data->version) && \
> > > +		reg.end_minor >= VOP_MINOR(vop->data->version) && \
> > > +		reg.mask))
> > This would be better suited as a static inline function. As would many of the
> > macros below.
> 
> Got it, will fix it at next version.
> > 
> > > -#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
> > > -		vop_mask_write(x, off, mask, shift, v, write_mask, false)
> > > +#define VOP_WIN_SUPPORT(vop, win, name) \
> > > +		VOP_REG_SUPPORT(vop, win->phy->name)
> > > -#define REG_SET(x, base, reg, v, mode) \
> > > -		__REG_SET_##mode(x, base + reg.offset, \
> > > -				 reg.mask, reg.shift, v, reg.write_mask)
> > > -#define REG_SET_MASK(x, base, reg, mask, v, mode) \
> > > -		__REG_SET_##mode(x, base + reg.offset, \
> > > -				 mask, reg.shift, v, reg.write_mask)
> > > +#define VOP_CTRL_SUPPORT(vop, name) \
> > > +		VOP_REG_SUPPORT(vop, vop->data->ctrl->name)
> > > +
> > > +#define VOP_INTR_SUPPORT(vop, name) \
> > > +		VOP_REG_SUPPORT(vop, vop->data->intr->name)
> > > +
> > > +#define __REG_SET(x, off, mask, shift, v, write_mask, relaxed) \
> > > +		vop_mask_write(x, off, mask, shift, v, write_mask, relaxed)
> > There's really no point to this, just call vop_mask_write directly.
> 
> Got it, will fix it at next version.
> > 
> > > +
> > > +#define _REG_SET(vop, name, off, reg, mask, v, relaxed) \
> > > +	do { \
> > > +		if (VOP_REG_SUPPORT(vop, reg)) \
> > > +			__REG_SET(vop, off + reg.offset, mask, reg.shift, \
> > s/mask/reg.mask & mask/
> 
> Got it, will fix it at next version.
> > 
> > > +				  v, reg.write_mask, relaxed); \
> > > +		else \
> > > +			dev_dbg(vop->dev, "Warning: not support "#name"\n"); \
> > > +	} while (0)
> > 
> > Also better as static inline, IMO.
> 
> Good idea, I will try it.
> 
> > 
> > > +
> > > +#define REG_SET(x, name, off, reg, v, relaxed) \
> > > +		_REG_SET(x, name, off, reg, reg.mask, v, relaxed)
> > s/reg.mask/~0/
> 
> Got it, will fix it at next version.
> > 
> > > +#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \
> > > +		_REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed)
> > s/reg.mask &//
> > 
> > Also, these can become static inline functions as well.
> 
> Got it, will fix it at next version.
> > 
> > >   #define VOP_WIN_SET(x, win, name, v) \
> > > -		REG_SET(x, win->base, win->phy->name, v, RELAXED)
> > > +		REG_SET(x, name, win->base, win->phy->name, v, true)
> > > +#define VOP_WIN_SET_EXT(x, win, ext, name, v) \
> > > +		REG_SET(x, name, 0, win->ext->name, v, true)
> > >   #define VOP_SCL_SET(x, win, name, v) \
> > > -		REG_SET(x, win->base, win->phy->scl->name, v, RELAXED)
> > > +		REG_SET(x, name, win->base, win->phy->scl->name, v, true)
> > >   #define VOP_SCL_SET_EXT(x, win, name, v) \
> > > -		REG_SET(x, win->base, win->phy->scl->ext->name, v, RELAXED)
> > > +		REG_SET(x, name, win->base, win->phy->scl->ext->name, v, true)
> > > +
> > >   #define VOP_CTRL_SET(x, name, v) \
> > > -		REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
> > > +		REG_SET(x, name, 0, (x)->data->ctrl->name, v, false)
> > >   #define VOP_INTR_GET(vop, name) \
> > >   		vop_read_reg(vop, 0, &vop->data->ctrl->name)
> > > -#define VOP_INTR_SET(vop, name, mask, v) \
> > > -		REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v, NORMAL)
> > > +#define VOP_INTR_SET(vop, name, v) \
> > > +		REG_SET(vop, name, 0, vop->data->intr->name, \
> > > +			v, false)
> > > +#define VOP_INTR_SET_MASK(vop, name, mask, v) \
> > > +		REG_SET_MASK(vop, name, 0, vop->data->intr->name, \
> > > +			     mask, v, false)
> > > +
> > >   #define VOP_INTR_SET_TYPE(vop, name, type, v) \
> > >   	do { \
> > >   		int i, reg = 0, mask = 0; \
> > > @@ -78,13 +105,16 @@
> > >   				mask |= 1 << i; \
> > >   			} \
> > >   		} \
> > > -		VOP_INTR_SET(vop, name, mask, reg); \
> > > +		VOP_INTR_SET_MASK(vop, name, mask, reg); \
> > >   	} while (0)
> > >   #define VOP_INTR_GET_TYPE(vop, name, type) \
> > >   		vop_get_intr_type(vop, &vop->data->intr->name, type)
> > > +#define VOP_CTRL_GET(x, name) \
> > > +		vop_read_reg(x, 0, &vop->data->ctrl->name)
> > > +
> > >   #define VOP_WIN_GET(x, win, name) \
> > > -		vop_read_reg(x, win->base, &win->phy->name)
> > > +		vop_read_reg(x, win->offset, win->phy->name)
> > >   #define VOP_WIN_GET_YRGBADDR(vop, win) \
> > >   		vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > index 084d3b2..e4de890 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > > @@ -15,6 +15,14 @@
> > >   #ifndef _ROCKCHIP_DRM_VOP_H
> > >   #define _ROCKCHIP_DRM_VOP_H
> > > +/*
> > > + * major: IP major vertion, used for IP structure
> > s/vertion/version
> 
> Got it, will fix it at next version.
> Thanks.
> 
> > 
> > > + * minor: big feature change under same structure
> > > + */
> > > +#define VOP_VERSION(major, minor)	((major) << 8 | (minor))
> > > +#define VOP_MAJOR(version)		((version) >> 8)
> > > +#define VOP_MINOR(version)		((version) & 0xff)
> > > +
> > >   enum vop_data_format {
> > >   	VOP_FMT_ARGB8888 = 0,
> > >   	VOP_FMT_RGB888,
> > > @@ -25,10 +33,13 @@ enum vop_data_format {
> > >   };
> > >   struct vop_reg {
> > > -	uint32_t offset;
> > > -	uint32_t shift;
> > >   	uint32_t mask;
> > > -	bool write_mask;
> > > +	uint32_t offset:12;
> > > +	uint32_t shift:5;
> > > +	uint32_t begin_minor:4;
> > > +	uint32_t end_minor:4;
> > > +	uint32_t major:3;
> > > +	uint32_t write_mask:1;
> > Why are you packing this?
> 
> make struct vop_reg not too big, jus u64 size, struct vop_reg use a lot on register define, packing it would reduce register table size.
> 

Ehhh, let's not get too fancy. If you want to save space, you can use uint8_t
and uint16_t. Packing like this makes it look like you're sending the struct
elsewhere, which is just confusing.

Sean

> > 
> > >   };
> > >   struct vop_ctrl {
> > > @@ -134,6 +145,7 @@ struct vop_win_data {
> > >   };
> > >   struct vop_data {
> > > +	uint32_t version;
> > >   	const struct vop_ctrl *ctrl;
> > >   	const struct vop_intr *intr;
> > >   	const struct vop_win_data *win;
> > > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > index 00e9d79..7744603 100644
> > > --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> > > @@ -20,17 +20,25 @@
> > >   #include "rockchip_drm_vop.h"
> > >   #include "rockchip_vop_reg.h"
> > > -#define VOP_REG(off, _mask, s) \
> > > +#define VOP_REG_VER_MASK(off, _mask, s, _write_mask, _major, \
> > > +			 _begin_minor, _end_minor) \
> > >   		{.offset = off, \
> > >   		 .mask = _mask, \
> > >   		 .shift = s, \
> > > -		 .write_mask = false,}
> > > +		 .write_mask = _write_mask, \
> > > +		 .major = _major, \
> > > +		 .begin_minor = _begin_minor, \
> > > +		 .end_minor = _end_minor,}
> > > +
> > > +#define VOP_REG_VER(off, _mask, s, _major, _begin_minor, _end_minor) \
> > > +		VOP_REG_VER_MASK(off, _mask, s, false, \
> > > +				 _major, _begin_minor, _end_minor)
> > > +
> > > +#define VOP_REG(off, _mask, s) \
> > > +		VOP_REG_VER(off, _mask, s, 0, 0, -1)
> > >   #define VOP_REG_MASK(off, _mask, s) \
> > > -		{.offset = off, \
> > > -		 .mask = _mask, \
> > > -		 .shift = s, \
> > > -		 .write_mask = true,}
> > > +		VOP_REG_VER_MASK(off, _mask, s, true, 0, 0, -1)
> > >   static const uint32_t formats_win_full[] = {
> > >   	DRM_FORMAT_XRGB8888,
> > > -- 
> > > 1.9.1
> > > 
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> -- 
> ?ark Yao
> 
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 3/5] drm/rockchip: vop: move line_flag_num to interrupt registers

[Mark Yao]

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

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a9180fd..be208ee 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -507,7 +507,7 @@ static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
 
 	spin_lock_irqsave(&vop->irq_lock, flags);
 
-	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
+	VOP_INTR_SET(vop, line_flag_num[0], line_num);
 	VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
 	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index e4de890..f64685e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -70,8 +70,6 @@ struct vop_ctrl {
 	struct vop_reg hpost_st_end;
 	struct vop_reg vpost_st_end;
 
-	struct vop_reg line_flag_num[2];
-
 	struct vop_reg global_regdone_en;
 	struct vop_reg cfg_done;
 };
@@ -79,6 +77,8 @@ struct vop_ctrl {
 struct vop_intr {
 	const int *intrs;
 	uint32_t nintrs;
+
+	struct vop_reg line_flag_num[2];
 	struct vop_reg enable;
 	struct vop_reg clear;
 	struct vop_reg status;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 7744603..159cedf 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -118,6 +118,7 @@
 static const struct vop_intr rk3036_intr = {
 	.intrs = rk3036_vop_intrs,
 	.nintrs = ARRAY_SIZE(rk3036_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
 	.status = VOP_REG(RK3036_INT_STATUS, 0xf, 0),
 	.enable = VOP_REG(RK3036_INT_STATUS, 0xf, 4),
 	.clear = VOP_REG(RK3036_INT_STATUS, 0xf, 8),
@@ -131,7 +132,6 @@
 	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
 	.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),
 };
 
@@ -227,7 +227,6 @@
 	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.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),
 };
@@ -259,6 +258,7 @@
 static const struct vop_intr rk3288_vop_intr = {
 	.intrs = rk3288_vop_intrs,
 	.nintrs = ARRAY_SIZE(rk3288_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
 	.status = VOP_REG(RK3288_INTR_CTRL0, 0xf, 0),
 	.enable = VOP_REG(RK3288_INTR_CTRL0, 0xf, 4),
 	.clear = VOP_REG(RK3288_INTR_CTRL0, 0xf, 8),
@@ -295,8 +295,6 @@
 	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.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),
 };
 
@@ -313,6 +311,8 @@
 static const struct vop_intr rk3399_vop_intr = {
 	.intrs = rk3399_vop_intrs,
 	.nintrs = ARRAY_SIZE(rk3399_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
 	.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),
-- 
1.9.1

[PATCH v2 3/5] drm/rockchip: vop: move line_flag_num to interrupt registers

[Sean Paul]

On Wed, Jul 12, 2017 at 10:03:46AM +0800, Mark Yao wrote:

Again, please add commit message describing the what and why of this change.

You can also add:

Reviewed-by: Sean Paul <seanpaul@chromium.org>


> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 ++--
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 8 ++++----
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index a9180fd..be208ee 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -507,7 +507,7 @@ static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
>  
>  	spin_lock_irqsave(&vop->irq_lock, flags);
>  
> -	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
> +	VOP_INTR_SET(vop, line_flag_num[0], line_num);
>  	VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
>  	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
>  
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index e4de890..f64685e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -70,8 +70,6 @@ struct vop_ctrl {
>  	struct vop_reg hpost_st_end;
>  	struct vop_reg vpost_st_end;
>  
> -	struct vop_reg line_flag_num[2];
> -
>  	struct vop_reg global_regdone_en;
>  	struct vop_reg cfg_done;
>  };
> @@ -79,6 +77,8 @@ struct vop_ctrl {
>  struct vop_intr {
>  	const int *intrs;
>  	uint32_t nintrs;
> +
> +	struct vop_reg line_flag_num[2];
>  	struct vop_reg enable;
>  	struct vop_reg clear;
>  	struct vop_reg status;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 7744603..159cedf 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -118,6 +118,7 @@
>  static const struct vop_intr rk3036_intr = {
>  	.intrs = rk3036_vop_intrs,
>  	.nintrs = ARRAY_SIZE(rk3036_vop_intrs),
> +	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>  	.status = VOP_REG(RK3036_INT_STATUS, 0xf, 0),
>  	.enable = VOP_REG(RK3036_INT_STATUS, 0xf, 4),
>  	.clear = VOP_REG(RK3036_INT_STATUS, 0xf, 8),
> @@ -131,7 +132,6 @@
>  	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>  	.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),
>  };
>  
> @@ -227,7 +227,6 @@
>  	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>  	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.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),
>  };
> @@ -259,6 +258,7 @@
>  static const struct vop_intr rk3288_vop_intr = {
>  	.intrs = rk3288_vop_intrs,
>  	.nintrs = ARRAY_SIZE(rk3288_vop_intrs),
> +	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>  	.status = VOP_REG(RK3288_INTR_CTRL0, 0xf, 0),
>  	.enable = VOP_REG(RK3288_INTR_CTRL0, 0xf, 4),
>  	.clear = VOP_REG(RK3288_INTR_CTRL0, 0xf, 8),
> @@ -295,8 +295,6 @@
>  	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>  	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.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),
>  };
>  
> @@ -313,6 +311,8 @@
>  static const struct vop_intr rk3399_vop_intr = {
>  	.intrs = rk3399_vop_intrs,
>  	.nintrs = ARRAY_SIZE(rk3399_vop_intrs),
> +	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
> +	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>  	.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),
> -- 
> 1.9.1
> 
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 3/5] drm/rockchip: vop: move line_flag_num to interrupt registers

[Mark yao]

On 2017?07?13? 01:54, Sean Paul wrote:
> On Wed, Jul 12, 2017 at 10:03:46AM +0800, Mark Yao wrote:
>
> Again, please add commit message describing the what and why of this change.
>
> You can also add:
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>

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

>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 4 ++--
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 8 ++++----
>>   3 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index a9180fd..be208ee 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -507,7 +507,7 @@ static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
>>   
>>   	spin_lock_irqsave(&vop->irq_lock, flags);
>>   
>> -	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
>> +	VOP_INTR_SET(vop, line_flag_num[0], line_num);
>>   	VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1);
>>   	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
>>   
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index e4de890..f64685e 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -70,8 +70,6 @@ struct vop_ctrl {
>>   	struct vop_reg hpost_st_end;
>>   	struct vop_reg vpost_st_end;
>>   
>> -	struct vop_reg line_flag_num[2];
>> -
>>   	struct vop_reg global_regdone_en;
>>   	struct vop_reg cfg_done;
>>   };
>> @@ -79,6 +77,8 @@ struct vop_ctrl {
>>   struct vop_intr {
>>   	const int *intrs;
>>   	uint32_t nintrs;
>> +
>> +	struct vop_reg line_flag_num[2];
>>   	struct vop_reg enable;
>>   	struct vop_reg clear;
>>   	struct vop_reg status;
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 7744603..159cedf 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -118,6 +118,7 @@
>>   static const struct vop_intr rk3036_intr = {
>>   	.intrs = rk3036_vop_intrs,
>>   	.nintrs = ARRAY_SIZE(rk3036_vop_intrs),
>> +	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>>   	.status = VOP_REG(RK3036_INT_STATUS, 0xf, 0),
>>   	.enable = VOP_REG(RK3036_INT_STATUS, 0xf, 4),
>>   	.clear = VOP_REG(RK3036_INT_STATUS, 0xf, 8),
>> @@ -131,7 +132,6 @@
>>   	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>>   	.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),
>>   };
>>   
>> @@ -227,7 +227,6 @@
>>   	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>   	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>   	.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),
>>   };
>> @@ -259,6 +258,7 @@
>>   static const struct vop_intr rk3288_vop_intr = {
>>   	.intrs = rk3288_vop_intrs,
>>   	.nintrs = ARRAY_SIZE(rk3288_vop_intrs),
>> +	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>>   	.status = VOP_REG(RK3288_INTR_CTRL0, 0xf, 0),
>>   	.enable = VOP_REG(RK3288_INTR_CTRL0, 0xf, 4),
>>   	.clear = VOP_REG(RK3288_INTR_CTRL0, 0xf, 8),
>> @@ -295,8 +295,6 @@
>>   	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>   	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>   	.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),
>>   };
>>   
>> @@ -313,6 +311,8 @@
>>   static const struct vop_intr rk3399_vop_intr = {
>>   	.intrs = rk3399_vop_intrs,
>>   	.nintrs = ARRAY_SIZE(rk3399_vop_intrs),
>> +	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
>> +	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>>   	.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),
>> -- 
>> 1.9.1
>>
>>


-- 
?ark Yao

[PATCH v2 4/5] drm/rockchip: vop: add a series of vop support

[Mark Yao]

Vop Full framework now has following vops:
IP version    chipname
  3.1           rk3288
  3.2           rk3368
  3.4           rk3366
  3.5           rk3399 big
  3.6           rk3399 lit
  3.7           rk3228
  3.8           rk3328

The above IP version is from H/W define, some of vop support get
the IP version from VERSION_INFO register, some are not.
hardcode the IP version for each vop to identify them.

major version: used for IP structure, Vop full framework is 3,
               vop little framework is 2.
minor version: on same structure, newer design vop will bigger
               then old one.

Changes in v2:
- rename rk322x to rk3228(Heiko St?bner)
- correct some vop registers define

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 202 +++++--
 drivers/gpu/drm/rockchip/rockchip_vop_reg.h | 905 ++++++++++++++++++++++------
 2 files changed, 863 insertions(+), 244 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 159cedf..b33483c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -211,6 +211,7 @@
 	.standby = VOP_REG(RK3288_SYS_CTRL, 0x1, 22),
 	.gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23),
 	.mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20),
+	.dp_en = VOP_REG_VER(RK3399_SYS_CTRL, 0x1, 11, 3, 5, -1),
 	.rgb_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 12),
 	.hdmi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 13),
 	.edp_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 14),
@@ -220,14 +221,19 @@
 	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
 	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
 	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
-	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
+	.pin_pol = VOP_REG_VER(RK3288_DSP_CTRL0, 0xf, 4, 3, 0, 1),
+	.dp_pin_pol = VOP_REG_VER(RK3399_DSP_CTRL1, 0xf, 16, 3, 5, -1),
+	.rgb_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 16, 3, 2, -1),
+	.hdmi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 20, 3, 2, -1),
+	.edp_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 24, 3, 2, -1),
+	.mipi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 28, 3, 2, -1),
 	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
 	.hact_st_end = VOP_REG(RK3288_DSP_HACT_ST_END, 0x1fff1fff, 0),
 	.vtotal_pw = VOP_REG(RK3288_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
 	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
-	.global_regdone_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 11),
+	.global_regdone_en = VOP_REG_VER(RK3288_SYS_CTRL, 0x1, 11, 3, 2, -1),
 	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -265,6 +271,7 @@
 };
 
 static const struct vop_data rk3288_vop = {
+	.version = VOP_VERSION(3, 1),
 	.feature = VOP_FEATURE_OUTPUT_RGB10,
 	.intr = &rk3288_vop_intr,
 	.ctrl = &rk3288_ctrl_data,
@@ -272,33 +279,7 @@
 	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
 };
 
-static const struct vop_ctrl rk3399_ctrl_data = {
-	.standby = VOP_REG(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),
-	.hdmi_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 13),
-	.edp_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 14),
-	.mipi_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 15),
-	.dither_down = VOP_REG(RK3399_DSP_CTRL1, 0xf, 1),
-	.dither_up = VOP_REG(RK3399_DSP_CTRL1, 0x1, 6),
-	.data_blank = VOP_REG(RK3399_DSP_CTRL0, 0x1, 19),
-	.out_mode = VOP_REG(RK3399_DSP_CTRL0, 0xf, 0),
-	.rgb_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
-	.dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16),
-	.hdmi_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 20),
-	.edp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 24),
-	.mipi_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 28),
-	.htotal_pw = VOP_REG(RK3399_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
-	.hact_st_end = VOP_REG(RK3399_DSP_HACT_ST_END, 0x1fff1fff, 0),
-	.vtotal_pw = VOP_REG(RK3399_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
-	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
-	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
-	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
-	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
-};
-
-static const int rk3399_vop_intrs[] = {
+static const int rk3368_vop_intrs[] = {
 	FS_INTR,
 	0, 0,
 	LINE_FLAG_INTR,
@@ -308,54 +289,173 @@
 	DSP_HOLD_VALID_INTR,
 };
 
-static const struct vop_intr rk3399_vop_intr = {
-	.intrs = rk3399_vop_intrs,
-	.nintrs = ARRAY_SIZE(rk3399_vop_intrs),
-	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
-	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
-	.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),
+static const struct vop_intr rk3368_vop_intr = {
+	.intrs = rk3368_vop_intrs,
+	.nintrs = ARRAY_SIZE(rk3368_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3368_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3368_LINE_FLAG, 0xffff, 16),
+	.status = VOP_REG_MASK(RK3368_INTR_STATUS, 0x3fff, 0),
+	.enable = VOP_REG_MASK(RK3368_INTR_EN, 0x3fff, 0),
+	.clear = VOP_REG_MASK(RK3368_INTR_CLEAR, 0x3fff, 0),
+};
+
+static const struct vop_win_phy rk3368_win23_data = {
+	.data_formats = formats_win_lite,
+	.nformats = ARRAY_SIZE(formats_win_lite),
+	.gate = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 0),
+	.enable = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 4),
+	.format = VOP_REG(RK3368_WIN2_CTRL0, 0x3, 5),
+	.rb_swap = VOP_REG(RK3368_WIN2_CTRL0, 0x1, 20),
+	.dsp_info = VOP_REG(RK3368_WIN2_DSP_INFO0, 0x0fff0fff, 0),
+	.dsp_st = VOP_REG(RK3368_WIN2_DSP_ST0, 0x1fff1fff, 0),
+	.yrgb_mst = VOP_REG(RK3368_WIN2_MST0, 0xffffffff, 0),
+	.yrgb_vir = VOP_REG(RK3368_WIN2_VIR0_1, 0x1fff, 0),
+	.src_alpha_ctl = VOP_REG(RK3368_WIN2_SRC_ALPHA_CTRL, 0xff, 0),
+	.dst_alpha_ctl = VOP_REG(RK3368_WIN2_DST_ALPHA_CTRL, 0xff, 0),
+};
+
+static const struct vop_win_data rk3368_vop_win_data[] = {
+	{ .base = 0x00, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_PRIMARY },
+	{ .base = 0x40, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_OVERLAY },
+	{ .base = 0x00, .phy = &rk3368_win23_data,
+	  .type = DRM_PLANE_TYPE_OVERLAY },
+	{ .base = 0x50, .phy = &rk3368_win23_data,
+	  .type = DRM_PLANE_TYPE_CURSOR },
+};
+
+static const struct vop_data rk3368_vop = {
+	.version = VOP_VERSION(3, 2),
+	.intr = &rk3368_vop_intr,
+	.ctrl = &rk3288_ctrl_data,
+	.win = rk3368_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+};
+
+static const struct vop_intr rk3366_vop_intr = {
+	.intrs = rk3368_vop_intrs,
+	.nintrs = ARRAY_SIZE(rk3368_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3366_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3366_LINE_FLAG, 0xffff, 16),
+	.status = VOP_REG_MASK(RK3366_INTR_STATUS0, 0xffff, 0),
+	.enable = VOP_REG_MASK(RK3366_INTR_EN0, 0xffff, 0),
+	.clear = VOP_REG_MASK(RK3366_INTR_CLEAR0, 0xffff, 0),
+};
+
+static const struct vop_data rk3366_vop = {
+	.version = VOP_VERSION(3, 4),
+	.intr = &rk3366_vop_intr,
+	.ctrl = &rk3288_ctrl_data,
+	.win = rk3368_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
 };
 
 static const struct vop_data rk3399_vop_big = {
+	.version = VOP_VERSION(3, 5),
 	.feature = VOP_FEATURE_OUTPUT_RGB10,
-	.intr = &rk3399_vop_intr,
-	.ctrl = &rk3399_ctrl_data,
-	/*
-	 * rk3399 vop big windows register layout is same as rk3288.
-	 */
-	.win = rk3288_vop_win_data,
-	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
+	.intr = &rk3366_vop_intr,
+	.ctrl = &rk3288_ctrl_data,
+	.win = rk3368_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
 };
 
 static const struct vop_win_data rk3399_vop_lit_win_data[] = {
 	{ .base = 0x00, .phy = &rk3288_win01_data,
 	  .type = DRM_PLANE_TYPE_PRIMARY },
-	{ .base = 0x00, .phy = &rk3288_win23_data,
+	{ .base = 0x00, .phy = &rk3368_win23_data,
 	  .type = DRM_PLANE_TYPE_CURSOR},
 };
 
 static const struct vop_data rk3399_vop_lit = {
-	.intr = &rk3399_vop_intr,
-	.ctrl = &rk3399_ctrl_data,
-	/*
-	 * rk3399 vop lit windows register layout is same as rk3288,
-	 * but cut off the win1 and win3 windows.
-	 */
+	.version = VOP_VERSION(3, 6),
+	.intr = &rk3366_vop_intr,
+	.ctrl = &rk3288_ctrl_data,
 	.win = rk3399_vop_lit_win_data,
 	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
 };
 
+static const struct vop_data rk3228_vop = {
+	.version = VOP_VERSION(3, 7),
+	.feature = VOP_FEATURE_OUTPUT_RGB10,
+	.intr = &rk3366_vop_intr,
+	.ctrl = &rk3288_ctrl_data,
+	.win = rk3368_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+};
+
+static const struct vop_ctrl rk3328_ctrl_data = {
+	.standby = VOP_REG(RK3328_SYS_CTRL, 0x1, 22),
+	.htotal_pw = VOP_REG(RK3328_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
+	.hact_st_end = VOP_REG(RK3328_DSP_HACT_ST_END, 0x1fff1fff, 0),
+	.vtotal_pw = VOP_REG(RK3328_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
+	.vact_st_end = VOP_REG(RK3328_DSP_VACT_ST_END, 0x1fff1fff, 0),
+	.hpost_st_end = VOP_REG(RK3328_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
+	.vpost_st_end = VOP_REG(RK3328_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
+	.global_regdone_en = VOP_REG(RK3328_SYS_CTRL, 0x1, 11),
+	.rgb_en = VOP_REG(RK3328_SYS_CTRL, 0x1, 12),
+	.hdmi_en = VOP_REG(RK3328_SYS_CTRL, 0x1, 13),
+	.edp_en = VOP_REG(RK3328_SYS_CTRL, 0x1, 14),
+	.mipi_en = VOP_REG(RK3328_SYS_CTRL, 0x1, 15),
+	.rgb_pin_pol = VOP_REG(RK3328_DSP_CTRL1, 0xf, 16),
+	.hdmi_pin_pol = VOP_REG(RK3328_DSP_CTRL1, 0xf, 20),
+	.edp_pin_pol = VOP_REG(RK3328_DSP_CTRL1, 0xf, 24),
+	.mipi_pin_pol = VOP_REG(RK3328_DSP_CTRL1, 0xf, 28),
+
+	.dither_down = VOP_REG(RK3328_DSP_CTRL1, 0xf, 1),
+	.dither_up = VOP_REG(RK3328_DSP_CTRL1, 0x1, 6),
+
+	.dsp_blank = VOP_REG(RK3328_DSP_CTRL0, 0x3, 18),
+	.out_mode = VOP_REG(RK3328_DSP_CTRL0, 0xf, 0),
+
+	.cfg_done = VOP_REG(RK3328_REG_CFG_DONE, 0x1, 0),
+};
+
+static const struct vop_intr rk3328_vop_intr = {
+	.intrs = rk3368_vop_intrs,
+	.nintrs = ARRAY_SIZE(rk3368_vop_intrs),
+	.line_flag_num[0] = VOP_REG(RK3328_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3328_LINE_FLAG, 0xffff, 16),
+	.status = VOP_REG_MASK(RK3328_INTR_STATUS0, 0xffff, 0),
+	.enable = VOP_REG_MASK(RK3328_INTR_EN0, 0xffff, 0),
+	.clear = VOP_REG_MASK(RK3328_INTR_CLEAR0, 0xffff, 0),
+};
+
+static const struct vop_win_data rk3328_vop_win_data[] = {
+	{ .base = 0xd0, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_PRIMARY },
+	{ .base = 0x1d0, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_OVERLAY },
+	{ .base = 0x2d0, .phy = &rk3288_win01_data,
+	  .type = DRM_PLANE_TYPE_CURSOR },
+};
+
+static const struct vop_data rk3328_vop = {
+	.version = VOP_VERSION(3, 8),
+	.feature = VOP_FEATURE_OUTPUT_RGB10,
+	.intr = &rk3328_vop_intr,
+	.ctrl = &rk3328_ctrl_data,
+	.win = rk3328_vop_win_data,
+	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
+};
+
 static const struct of_device_id vop_driver_dt_match[] = {
 	{ .compatible = "rockchip,rk3036-vop",
 	  .data = &rk3036_vop },
 	{ .compatible = "rockchip,rk3288-vop",
 	  .data = &rk3288_vop },
+	{ .compatible = "rockchip,rk3368-vop",
+	  .data = &rk3368_vop },
+	{ .compatible = "rockchip,rk3366-vop",
+	  .data = &rk3366_vop },
 	{ .compatible = "rockchip,rk3399-vop-big",
 	  .data = &rk3399_vop_big },
 	{ .compatible = "rockchip,rk3399-vop-lit",
 	  .data = &rk3399_vop_lit },
+	{ .compatible = "rockchip,rk3228-vop",
+	  .data = &rk3228_vop },
+	{ .compatible = "rockchip,rk3328-vop",
+	  .data = &rk3328_vop },
 	{},
 };
 MODULE_DEVICE_TABLE(of, vop_driver_dt_match);
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.h b/drivers/gpu/drm/rockchip/rockchip_vop_reg.h
index cd19726..4a4799f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.h
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.h
@@ -41,6 +41,7 @@
 #define RK3288_WIN0_SRC_ALPHA_CTRL		0x0060
 #define RK3288_WIN0_DST_ALPHA_CTRL		0x0064
 #define RK3288_WIN0_FADING_CTRL			0x0068
+#define RK3288_WIN0_CTRL2			0x006c
 
 /* win1 register */
 #define RK3288_WIN1_CTRL0			0x0070
@@ -122,6 +123,717 @@
 #define RK3288_DSP_VACT_ST_END_F1		0x019c
 /* register definition end */
 
+/* rk3368 register definition */
+#define RK3368_REG_CFG_DONE			0x0000
+#define RK3368_VERSION_INFO			0x0004
+#define RK3368_SYS_CTRL				0x0008
+#define RK3368_SYS_CTRL1			0x000c
+#define RK3368_DSP_CTRL0			0x0010
+#define RK3368_DSP_CTRL1			0x0014
+#define RK3368_DSP_BG				0x0018
+#define RK3368_MCU_CTRL				0x001c
+#define RK3368_LINE_FLAG			0x0020
+#define RK3368_INTR_EN				0x0024
+#define RK3368_INTR_CLEAR			0x0028
+#define RK3368_INTR_STATUS			0x002c
+#define RK3368_WIN0_CTRL0			0x0030
+#define RK3368_WIN0_CTRL1			0x0034
+#define RK3368_WIN0_COLOR_KEY			0x0038
+#define RK3368_WIN0_VIR				0x003c
+#define RK3368_WIN0_YRGB_MST			0x0040
+#define RK3368_WIN0_CBR_MST			0x0044
+#define RK3368_WIN0_ACT_INFO			0x0048
+#define RK3368_WIN0_DSP_INFO			0x004c
+#define RK3368_WIN0_DSP_ST			0x0050
+#define RK3368_WIN0_SCL_FACTOR_YRGB		0x0054
+#define RK3368_WIN0_SCL_FACTOR_CBR		0x0058
+#define RK3368_WIN0_SCL_OFFSET			0x005c
+#define RK3368_WIN0_SRC_ALPHA_CTRL		0x0060
+#define RK3368_WIN0_DST_ALPHA_CTRL		0x0064
+#define RK3368_WIN0_FADING_CTRL			0x0068
+#define RK3368_WIN0_CTRL2			0x006c
+#define RK3368_WIN1_CTRL0			0x0070
+#define RK3368_WIN1_CTRL1			0x0074
+#define RK3368_WIN1_COLOR_KEY			0x0078
+#define RK3368_WIN1_VIR				0x007c
+#define RK3368_WIN1_YRGB_MST			0x0080
+#define RK3368_WIN1_CBR_MST			0x0084
+#define RK3368_WIN1_ACT_INFO			0x0088
+#define RK3368_WIN1_DSP_INFO			0x008c
+#define RK3368_WIN1_DSP_ST			0x0090
+#define RK3368_WIN1_SCL_FACTOR_YRGB		0x0094
+#define RK3368_WIN1_SCL_FACTOR_CBR		0x0098
+#define RK3368_WIN1_SCL_OFFSET			0x009c
+#define RK3368_WIN1_SRC_ALPHA_CTRL		0x00a0
+#define RK3368_WIN1_DST_ALPHA_CTRL		0x00a4
+#define RK3368_WIN1_FADING_CTRL			0x00a8
+#define RK3368_WIN1_CTRL2			0x00ac
+#define RK3368_WIN2_CTRL0			0x00b0
+#define RK3368_WIN2_CTRL1			0x00b4
+#define RK3368_WIN2_VIR0_1			0x00b8
+#define RK3368_WIN2_VIR2_3			0x00bc
+#define RK3368_WIN2_MST0			0x00c0
+#define RK3368_WIN2_DSP_INFO0			0x00c4
+#define RK3368_WIN2_DSP_ST0			0x00c8
+#define RK3368_WIN2_COLOR_KEY			0x00cc
+#define RK3368_WIN2_MST1			0x00d0
+#define RK3368_WIN2_DSP_INFO1			0x00d4
+#define RK3368_WIN2_DSP_ST1			0x00d8
+#define RK3368_WIN2_SRC_ALPHA_CTRL		0x00dc
+#define RK3368_WIN2_MST2			0x00e0
+#define RK3368_WIN2_DSP_INFO2			0x00e4
+#define RK3368_WIN2_DSP_ST2			0x00e8
+#define RK3368_WIN2_DST_ALPHA_CTRL		0x00ec
+#define RK3368_WIN2_MST3			0x00f0
+#define RK3368_WIN2_DSP_INFO3			0x00f4
+#define RK3368_WIN2_DSP_ST3			0x00f8
+#define RK3368_WIN2_FADING_CTRL			0x00fc
+#define RK3368_WIN3_CTRL0			0x0100
+#define RK3368_WIN3_CTRL1			0x0104
+#define RK3368_WIN3_VIR0_1			0x0108
+#define RK3368_WIN3_VIR2_3			0x010c
+#define RK3368_WIN3_MST0			0x0110
+#define RK3368_WIN3_DSP_INFO0			0x0114
+#define RK3368_WIN3_DSP_ST0			0x0118
+#define RK3368_WIN3_COLOR_KEY			0x011c
+#define RK3368_WIN3_MST1			0x0120
+#define RK3368_WIN3_DSP_INFO1			0x0124
+#define RK3368_WIN3_DSP_ST1			0x0128
+#define RK3368_WIN3_SRC_ALPHA_CTRL		0x012c
+#define RK3368_WIN3_MST2			0x0130
+#define RK3368_WIN3_DSP_INFO2			0x0134
+#define RK3368_WIN3_DSP_ST2			0x0138
+#define RK3368_WIN3_DST_ALPHA_CTRL		0x013c
+#define RK3368_WIN3_MST3			0x0140
+#define RK3368_WIN3_DSP_INFO3			0x0144
+#define RK3368_WIN3_DSP_ST3			0x0148
+#define RK3368_WIN3_FADING_CTRL			0x014c
+#define RK3368_HWC_CTRL0			0x0150
+#define RK3368_HWC_CTRL1			0x0154
+#define RK3368_HWC_MST				0x0158
+#define RK3368_HWC_DSP_ST			0x015c
+#define RK3368_HWC_SRC_ALPHA_CTRL		0x0160
+#define RK3368_HWC_DST_ALPHA_CTRL		0x0164
+#define RK3368_HWC_FADING_CTRL			0x0168
+#define RK3368_HWC_RESERVED1			0x016c
+#define RK3368_POST_DSP_HACT_INFO		0x0170
+#define RK3368_POST_DSP_VACT_INFO		0x0174
+#define RK3368_POST_SCL_FACTOR_YRGB		0x0178
+#define RK3368_POST_RESERVED			0x017c
+#define RK3368_POST_SCL_CTRL			0x0180
+#define RK3368_POST_DSP_VACT_INFO_F1		0x0184
+#define RK3368_DSP_HTOTAL_HS_END		0x0188
+#define RK3368_DSP_HACT_ST_END			0x018c
+#define RK3368_DSP_VTOTAL_VS_END		0x0190
+#define RK3368_DSP_VACT_ST_END			0x0194
+#define RK3368_DSP_VS_ST_END_F1			0x0198
+#define RK3368_DSP_VACT_ST_END_F1		0x019c
+#define RK3368_PWM_CTRL				0x01a0
+#define RK3368_PWM_PERIOD_HPR			0x01a4
+#define RK3368_PWM_DUTY_LPR			0x01a8
+#define RK3368_PWM_CNT				0x01ac
+#define RK3368_BCSH_COLOR_BAR			0x01b0
+#define RK3368_BCSH_BCS				0x01b4
+#define RK3368_BCSH_H				0x01b8
+#define RK3368_BCSH_CTRL			0x01bc
+#define RK3368_CABC_CTRL0			0x01c0
+#define RK3368_CABC_CTRL1			0x01c4
+#define RK3368_CABC_CTRL2			0x01c8
+#define RK3368_CABC_CTRL3			0x01cc
+#define RK3368_CABC_GAUSS_LINE0_0		0x01d0
+#define RK3368_CABC_GAUSS_LINE0_1		0x01d4
+#define RK3368_CABC_GAUSS_LINE1_0		0x01d8
+#define RK3368_CABC_GAUSS_LINE1_1		0x01dc
+#define RK3368_CABC_GAUSS_LINE2_0		0x01e0
+#define RK3368_CABC_GAUSS_LINE2_1		0x01e4
+#define RK3368_FRC_LOWER01_0			0x01e8
+#define RK3368_FRC_LOWER01_1			0x01ec
+#define RK3368_FRC_LOWER10_0			0x01f0
+#define RK3368_FRC_LOWER10_1			0x01f4
+#define RK3368_FRC_LOWER11_0			0x01f8
+#define RK3368_FRC_LOWER11_1			0x01fc
+#define RK3368_IFBDC_CTRL			0x0200
+#define RK3368_IFBDC_TILES_NUM			0x0204
+#define RK3368_IFBDC_FRAME_RST_CYCLE		0x0208
+#define RK3368_IFBDC_BASE_ADDR			0x020c
+#define RK3368_IFBDC_MB_SIZE			0x0210
+#define RK3368_IFBDC_CMP_INDEX_INIT		0x0214
+#define RK3368_IFBDC_VIR			0x0220
+#define RK3368_IFBDC_DEBUG0			0x0230
+#define RK3368_IFBDC_DEBUG1			0x0234
+#define RK3368_LATENCY_CTRL0			0x0250
+#define RK3368_RD_MAX_LATENCY_NUM0		0x0254
+#define RK3368_RD_LATENCY_THR_NUM0		0x0258
+#define RK3368_RD_LATENCY_SAMP_NUM0		0x025c
+#define RK3368_WIN0_DSP_BG			0x0260
+#define RK3368_WIN1_DSP_BG			0x0264
+#define RK3368_WIN2_DSP_BG			0x0268
+#define RK3368_WIN3_DSP_BG			0x026c
+#define RK3368_SCAN_LINE_NUM			0x0270
+#define RK3368_CABC_DEBUG0			0x0274
+#define RK3368_CABC_DEBUG1			0x0278
+#define RK3368_CABC_DEBUG2			0x027c
+#define RK3368_DBG_REG_000			0x0280
+#define RK3368_DBG_REG_001			0x0284
+#define RK3368_DBG_REG_002			0x0288
+#define RK3368_DBG_REG_003			0x028c
+#define RK3368_DBG_REG_004			0x0290
+#define RK3368_DBG_REG_005			0x0294
+#define RK3368_DBG_REG_006			0x0298
+#define RK3368_DBG_REG_007			0x029c
+#define RK3368_DBG_REG_008			0x02a0
+#define RK3368_DBG_REG_016			0x02c0
+#define RK3368_DBG_REG_017			0x02c4
+#define RK3368_DBG_REG_018			0x02c8
+#define RK3368_DBG_REG_019			0x02cc
+#define RK3368_DBG_REG_020			0x02d0
+#define RK3368_DBG_REG_021			0x02d4
+#define RK3368_DBG_REG_022			0x02d8
+#define RK3368_DBG_REG_023			0x02dc
+#define RK3368_DBG_REG_028			0x02f0
+#define RK3368_MMU_DTE_ADDR			0x0300
+#define RK3368_MMU_STATUS			0x0304
+#define RK3368_MMU_COMMAND			0x0308
+#define RK3368_MMU_PAGE_FAULT_ADDR		0x030c
+#define RK3368_MMU_ZAP_ONE_LINE			0x0310
+#define RK3368_MMU_INT_RAWSTAT			0x0314
+#define RK3368_MMU_INT_CLEAR			0x0318
+#define RK3368_MMU_INT_MASK			0x031c
+#define RK3368_MMU_INT_STATUS			0x0320
+#define RK3368_MMU_AUTO_GATING			0x0324
+#define RK3368_WIN2_LUT_ADDR			0x0400
+#define RK3368_WIN3_LUT_ADDR			0x0800
+#define RK3368_HWC_LUT_ADDR			0x0c00
+#define RK3368_GAMMA_LUT_ADDR			0x1000
+#define RK3368_CABC_GAMMA_LUT_ADDR		0x1800
+#define RK3368_MCU_BYPASS_WPORT			0x2200
+#define RK3368_MCU_BYPASS_RPORT			0x2300
+/* rk3368 register definition end */
+
+#define RK3366_REG_CFG_DONE			0x0000
+#define RK3366_VERSION_INFO			0x0004
+#define RK3366_SYS_CTRL				0x0008
+#define RK3366_SYS_CTRL1			0x000c
+#define RK3366_DSP_CTRL0			0x0010
+#define RK3366_DSP_CTRL1			0x0014
+#define RK3366_DSP_BG				0x0018
+#define RK3366_MCU_CTRL				0x001c
+#define RK3366_WB_CTRL0				0x0020
+#define RK3366_WB_CTRL1				0x0024
+#define RK3366_WB_YRGB_MST			0x0028
+#define RK3366_WB_CBR_MST			0x002c
+#define RK3366_WIN0_CTRL0			0x0030
+#define RK3366_WIN0_CTRL1			0x0034
+#define RK3366_WIN0_COLOR_KEY			0x0038
+#define RK3366_WIN0_VIR				0x003c
+#define RK3366_WIN0_YRGB_MST			0x0040
+#define RK3366_WIN0_CBR_MST			0x0044
+#define RK3366_WIN0_ACT_INFO			0x0048
+#define RK3366_WIN0_DSP_INFO			0x004c
+#define RK3366_WIN0_DSP_ST			0x0050
+#define RK3366_WIN0_SCL_FACTOR_YRGB		0x0054
+#define RK3366_WIN0_SCL_FACTOR_CBR		0x0058
+#define RK3366_WIN0_SCL_OFFSET			0x005c
+#define RK3366_WIN0_SRC_ALPHA_CTRL		0x0060
+#define RK3366_WIN0_DST_ALPHA_CTRL		0x0064
+#define RK3366_WIN0_FADING_CTRL			0x0068
+#define RK3366_WIN0_CTRL2			0x006c
+#define RK3366_WIN1_CTRL0			0x0070
+#define RK3366_WIN1_CTRL1			0x0074
+#define RK3366_WIN1_COLOR_KEY			0x0078
+#define RK3366_WIN1_VIR				0x007c
+#define RK3366_WIN1_YRGB_MST			0x0080
+#define RK3366_WIN1_CBR_MST			0x0084
+#define RK3366_WIN1_ACT_INFO			0x0088
+#define RK3366_WIN1_DSP_INFO			0x008c
+#define RK3366_WIN1_DSP_ST			0x0090
+#define RK3366_WIN1_SCL_FACTOR_YRGB		0x0094
+#define RK3366_WIN1_SCL_FACTOR_CBR		0x0098
+#define RK3366_WIN1_SCL_OFFSET			0x009c
+#define RK3366_WIN1_SRC_ALPHA_CTRL		0x00a0
+#define RK3366_WIN1_DST_ALPHA_CTRL		0x00a4
+#define RK3366_WIN1_FADING_CTRL			0x00a8
+#define RK3366_WIN1_CTRL2			0x00ac
+#define RK3366_WIN2_CTRL0			0x00b0
+#define RK3366_WIN2_CTRL1			0x00b4
+#define RK3366_WIN2_VIR0_1			0x00b8
+#define RK3366_WIN2_VIR2_3			0x00bc
+#define RK3366_WIN2_MST0			0x00c0
+#define RK3366_WIN2_DSP_INFO0			0x00c4
+#define RK3366_WIN2_DSP_ST0			0x00c8
+#define RK3366_WIN2_COLOR_KEY			0x00cc
+#define RK3366_WIN2_MST1			0x00d0
+#define RK3366_WIN2_DSP_INFO1			0x00d4
+#define RK3366_WIN2_DSP_ST1			0x00d8
+#define RK3366_WIN2_SRC_ALPHA_CTRL		0x00dc
+#define RK3366_WIN2_MST2			0x00e0
+#define RK3366_WIN2_DSP_INFO2			0x00e4
+#define RK3366_WIN2_DSP_ST2			0x00e8
+#define RK3366_WIN2_DST_ALPHA_CTRL		0x00ec
+#define RK3366_WIN2_MST3			0x00f0
+#define RK3366_WIN2_DSP_INFO3			0x00f4
+#define RK3366_WIN2_DSP_ST3			0x00f8
+#define RK3366_WIN2_FADING_CTRL			0x00fc
+#define RK3366_WIN3_CTRL0			0x0100
+#define RK3366_WIN3_CTRL1			0x0104
+#define RK3366_WIN3_VIR0_1			0x0108
+#define RK3366_WIN3_VIR2_3			0x010c
+#define RK3366_WIN3_MST0			0x0110
+#define RK3366_WIN3_DSP_INFO0			0x0114
+#define RK3366_WIN3_DSP_ST0			0x0118
+#define RK3366_WIN3_COLOR_KEY			0x011c
+#define RK3366_WIN3_MST1			0x0120
+#define RK3366_WIN3_DSP_INFO1			0x0124
+#define RK3366_WIN3_DSP_ST1			0x0128
+#define RK3366_WIN3_SRC_ALPHA_CTRL		0x012c
+#define RK3366_WIN3_MST2			0x0130
+#define RK3366_WIN3_DSP_INFO2			0x0134
+#define RK3366_WIN3_DSP_ST2			0x0138
+#define RK3366_WIN3_DST_ALPHA_CTRL		0x013c
+#define RK3366_WIN3_MST3			0x0140
+#define RK3366_WIN3_DSP_INFO3			0x0144
+#define RK3366_WIN3_DSP_ST3			0x0148
+#define RK3366_WIN3_FADING_CTRL			0x014c
+#define RK3366_HWC_CTRL0			0x0150
+#define RK3366_HWC_CTRL1			0x0154
+#define RK3366_HWC_MST				0x0158
+#define RK3366_HWC_DSP_ST			0x015c
+#define RK3366_HWC_SRC_ALPHA_CTRL		0x0160
+#define RK3366_HWC_DST_ALPHA_CTRL		0x0164
+#define RK3366_HWC_FADING_CTRL			0x0168
+#define RK3366_HWC_RESERVED1			0x016c
+#define RK3366_POST_DSP_HACT_INFO		0x0170
+#define RK3366_POST_DSP_VACT_INFO		0x0174
+#define RK3366_POST_SCL_FACTOR_YRGB		0x0178
+#define RK3366_POST_RESERVED			0x017c
+#define RK3366_POST_SCL_CTRL			0x0180
+#define RK3366_POST_DSP_VACT_INFO_F1		0x0184
+#define RK3366_DSP_HTOTAL_HS_END		0x0188
+#define RK3366_DSP_HACT_ST_END			0x018c
+#define RK3366_DSP_VTOTAL_VS_END		0x0190
+#define RK3366_DSP_VACT_ST_END			0x0194
+#define RK3366_DSP_VS_ST_END_F1			0x0198
+#define RK3366_DSP_VACT_ST_END_F1		0x019c
+#define RK3366_PWM_CTRL				0x01a0
+#define RK3366_PWM_PERIOD_HPR			0x01a4
+#define RK3366_PWM_DUTY_LPR			0x01a8
+#define RK3366_PWM_CNT				0x01ac
+#define RK3366_BCSH_COLOR_BAR			0x01b0
+#define RK3366_BCSH_BCS				0x01b4
+#define RK3366_BCSH_H				0x01b8
+#define RK3366_BCSH_CTRL			0x01bc
+#define RK3366_CABC_CTRL0			0x01c0
+#define RK3366_CABC_CTRL1			0x01c4
+#define RK3366_CABC_CTRL2			0x01c8
+#define RK3366_CABC_CTRL3			0x01cc
+#define RK3366_CABC_GAUSS_LINE0_0		0x01d0
+#define RK3366_CABC_GAUSS_LINE0_1		0x01d4
+#define RK3366_CABC_GAUSS_LINE1_0		0x01d8
+#define RK3366_CABC_GAUSS_LINE1_1		0x01dc
+#define RK3366_CABC_GAUSS_LINE2_0		0x01e0
+#define RK3366_CABC_GAUSS_LINE2_1		0x01e4
+#define RK3366_FRC_LOWER01_0			0x01e8
+#define RK3366_FRC_LOWER01_1			0x01ec
+#define RK3366_FRC_LOWER10_0			0x01f0
+#define RK3366_FRC_LOWER10_1			0x01f4
+#define RK3366_FRC_LOWER11_0			0x01f8
+#define RK3366_FRC_LOWER11_1			0x01fc
+#define RK3366_INTR_EN0				0x0280
+#define RK3366_INTR_CLEAR0			0x0284
+#define RK3366_INTR_STATUS0			0x0288
+#define RK3366_INTR_RAW_STATUS0			0x028c
+#define RK3366_INTR_EN1				0x0290
+#define RK3366_INTR_CLEAR1			0x0294
+#define RK3366_INTR_STATUS1			0x0298
+#define RK3366_INTR_RAW_STATUS1			0x029c
+#define RK3366_LINE_FLAG			0x02a0
+#define RK3366_VOP_STATUS			0x02a4
+#define RK3366_BLANKING_VALUE			0x02a8
+#define RK3366_WIN0_DSP_BG			0x02b0
+#define RK3366_WIN1_DSP_BG			0x02b4
+#define RK3366_WIN2_DSP_BG			0x02b8
+#define RK3366_WIN3_DSP_BG			0x02bc
+#define RK3366_WIN2_LUT_ADDR			0x0400
+#define RK3366_WIN3_LUT_ADDR			0x0800
+#define RK3366_HWC_LUT_ADDR			0x0c00
+#define RK3366_GAMMA0_LUT_ADDR			0x1000
+#define RK3366_GAMMA1_LUT_ADDR			0x1400
+#define RK3366_CABC_GAMMA_LUT_ADDR		0x1800
+#define RK3366_MCU_BYPASS_WPORT			0x2200
+#define RK3366_MCU_BYPASS_RPORT			0x2300
+#define RK3366_MMU_DTE_ADDR			0x2400
+#define RK3366_MMU_STATUS			0x2404
+#define RK3366_MMU_COMMAND			0x2408
+#define RK3366_MMU_PAGE_FAULT_ADDR		0x240c
+#define RK3366_MMU_ZAP_ONE_LINE			0x2410
+#define RK3366_MMU_INT_RAWSTAT			0x2414
+#define RK3366_MMU_INT_CLEAR			0x2418
+#define RK3366_MMU_INT_MASK			0x241c
+#define RK3366_MMU_INT_STATUS			0x2420
+#define RK3366_MMU_AUTO_GATING			0x2424
+
+/* rk3399 register definition */
+#define RK3399_REG_CFG_DONE			0x0000
+#define RK3399_VERSION_INFO			0x0004
+#define RK3399_SYS_CTRL				0x0008
+#define RK3399_SYS_CTRL1			0x000c
+#define RK3399_DSP_CTRL0			0x0010
+#define RK3399_DSP_CTRL1			0x0014
+#define RK3399_DSP_BG				0x0018
+#define RK3399_MCU_CTRL				0x001c
+#define RK3399_WB_CTRL0				0x0020
+#define RK3399_WB_CTRL1				0x0024
+#define RK3399_WB_YRGB_MST			0x0028
+#define RK3399_WB_CBR_MST			0x002c
+#define RK3399_WIN0_CTRL0			0x0030
+#define RK3399_WIN0_CTRL1			0x0034
+#define RK3399_WIN0_COLOR_KEY			0x0038
+#define RK3399_WIN0_VIR				0x003c
+#define RK3399_WIN0_YRGB_MST			0x0040
+#define RK3399_WIN0_CBR_MST			0x0044
+#define RK3399_WIN0_ACT_INFO			0x0048
+#define RK3399_WIN0_DSP_INFO			0x004c
+#define RK3399_WIN0_DSP_ST			0x0050
+#define RK3399_WIN0_SCL_FACTOR_YRGB		0x0054
+#define RK3399_WIN0_SCL_FACTOR_CBR		0x0058
+#define RK3399_WIN0_SCL_OFFSET			0x005c
+#define RK3399_WIN0_SRC_ALPHA_CTRL		0x0060
+#define RK3399_WIN0_DST_ALPHA_CTRL		0x0064
+#define RK3399_WIN0_FADING_CTRL			0x0068
+#define RK3399_WIN0_CTRL2			0x006c
+#define RK3399_WIN1_CTRL0			0x0070
+#define RK3399_WIN1_CTRL1			0x0074
+#define RK3399_WIN1_COLOR_KEY			0x0078
+#define RK3399_WIN1_VIR				0x007c
+#define RK3399_WIN1_YRGB_MST			0x0080
+#define RK3399_WIN1_CBR_MST			0x0084
+#define RK3399_WIN1_ACT_INFO			0x0088
+#define RK3399_WIN1_DSP_INFO			0x008c
+#define RK3399_WIN1_DSP_ST			0x0090
+#define RK3399_WIN1_SCL_FACTOR_YRGB		0x0094
+#define RK3399_WIN1_SCL_FACTOR_CBR		0x0098
+#define RK3399_WIN1_SCL_OFFSET			0x009c
+#define RK3399_WIN1_SRC_ALPHA_CTRL		0x00a0
+#define RK3399_WIN1_DST_ALPHA_CTRL		0x00a4
+#define RK3399_WIN1_FADING_CTRL			0x00a8
+#define RK3399_WIN1_CTRL2			0x00ac
+#define RK3399_WIN2_CTRL0			0x00b0
+#define RK3399_WIN2_CTRL1			0x00b4
+#define RK3399_WIN2_VIR0_1			0x00b8
+#define RK3399_WIN2_VIR2_3			0x00bc
+#define RK3399_WIN2_MST0			0x00c0
+#define RK3399_WIN2_DSP_INFO0			0x00c4
+#define RK3399_WIN2_DSP_ST0			0x00c8
+#define RK3399_WIN2_COLOR_KEY			0x00cc
+#define RK3399_WIN2_MST1			0x00d0
+#define RK3399_WIN2_DSP_INFO1			0x00d4
+#define RK3399_WIN2_DSP_ST1			0x00d8
+#define RK3399_WIN2_SRC_ALPHA_CTRL		0x00dc
+#define RK3399_WIN2_MST2			0x00e0
+#define RK3399_WIN2_DSP_INFO2			0x00e4
+#define RK3399_WIN2_DSP_ST2			0x00e8
+#define RK3399_WIN2_DST_ALPHA_CTRL		0x00ec
+#define RK3399_WIN2_MST3			0x00f0
+#define RK3399_WIN2_DSP_INFO3			0x00f4
+#define RK3399_WIN2_DSP_ST3			0x00f8
+#define RK3399_WIN2_FADING_CTRL			0x00fc
+#define RK3399_WIN3_CTRL0			0x0100
+#define RK3399_WIN3_CTRL1			0x0104
+#define RK3399_WIN3_VIR0_1			0x0108
+#define RK3399_WIN3_VIR2_3			0x010c
+#define RK3399_WIN3_MST0			0x0110
+#define RK3399_WIN3_DSP_INFO0			0x0114
+#define RK3399_WIN3_DSP_ST0			0x0118
+#define RK3399_WIN3_COLOR_KEY			0x011c
+#define RK3399_WIN3_MST1			0x0120
+#define RK3399_WIN3_DSP_INFO1			0x0124
+#define RK3399_WIN3_DSP_ST1			0x0128
+#define RK3399_WIN3_SRC_ALPHA_CTRL		0x012c
+#define RK3399_WIN3_MST2			0x0130
+#define RK3399_WIN3_DSP_INFO2			0x0134
+#define RK3399_WIN3_DSP_ST2			0x0138
+#define RK3399_WIN3_DST_ALPHA_CTRL		0x013c
+#define RK3399_WIN3_MST3			0x0140
+#define RK3399_WIN3_DSP_INFO3			0x0144
+#define RK3399_WIN3_DSP_ST3			0x0148
+#define RK3399_WIN3_FADING_CTRL			0x014c
+#define RK3399_HWC_CTRL0			0x0150
+#define RK3399_HWC_CTRL1			0x0154
+#define RK3399_HWC_MST				0x0158
+#define RK3399_HWC_DSP_ST			0x015c
+#define RK3399_HWC_SRC_ALPHA_CTRL		0x0160
+#define RK3399_HWC_DST_ALPHA_CTRL		0x0164
+#define RK3399_HWC_FADING_CTRL			0x0168
+#define RK3399_HWC_RESERVED1			0x016c
+#define RK3399_POST_DSP_HACT_INFO		0x0170
+#define RK3399_POST_DSP_VACT_INFO		0x0174
+#define RK3399_POST_SCL_FACTOR_YRGB		0x0178
+#define RK3399_POST_RESERVED			0x017c
+#define RK3399_POST_SCL_CTRL			0x0180
+#define RK3399_POST_DSP_VACT_INFO_F1		0x0184
+#define RK3399_DSP_HTOTAL_HS_END		0x0188
+#define RK3399_DSP_HACT_ST_END			0x018c
+#define RK3399_DSP_VTOTAL_VS_END		0x0190
+#define RK3399_DSP_VACT_ST_END			0x0194
+#define RK3399_DSP_VS_ST_END_F1			0x0198
+#define RK3399_DSP_VACT_ST_END_F1		0x019c
+#define RK3399_PWM_CTRL				0x01a0
+#define RK3399_PWM_PERIOD_HPR			0x01a4
+#define RK3399_PWM_DUTY_LPR			0x01a8
+#define RK3399_PWM_CNT				0x01ac
+#define RK3399_BCSH_COLOR_BAR			0x01b0
+#define RK3399_BCSH_BCS				0x01b4
+#define RK3399_BCSH_H				0x01b8
+#define RK3399_BCSH_CTRL			0x01bc
+#define RK3399_CABC_CTRL0			0x01c0
+#define RK3399_CABC_CTRL1			0x01c4
+#define RK3399_CABC_CTRL2			0x01c8
+#define RK3399_CABC_CTRL3			0x01cc
+#define RK3399_CABC_GAUSS_LINE0_0		0x01d0
+#define RK3399_CABC_GAUSS_LINE0_1		0x01d4
+#define RK3399_CABC_GAUSS_LINE1_0		0x01d8
+#define RK3399_CABC_GAUSS_LINE1_1		0x01dc
+#define RK3399_CABC_GAUSS_LINE2_0		0x01e0
+#define RK3399_CABC_GAUSS_LINE2_1		0x01e4
+#define RK3399_FRC_LOWER01_0			0x01e8
+#define RK3399_FRC_LOWER01_1			0x01ec
+#define RK3399_FRC_LOWER10_0			0x01f0
+#define RK3399_FRC_LOWER10_1			0x01f4
+#define RK3399_FRC_LOWER11_0			0x01f8
+#define RK3399_FRC_LOWER11_1			0x01fc
+#define RK3399_AFBCD0_CTRL			0x0200
+#define RK3399_AFBCD0_HDR_PTR			0x0204
+#define RK3399_AFBCD0_PIC_SIZE			0x0208
+#define RK3399_AFBCD0_STATUS			0x020c
+#define RK3399_AFBCD1_CTRL			0x0220
+#define RK3399_AFBCD1_HDR_PTR			0x0224
+#define RK3399_AFBCD1_PIC_SIZE			0x0228
+#define RK3399_AFBCD1_STATUS			0x022c
+#define RK3399_AFBCD2_CTRL			0x0240
+#define RK3399_AFBCD2_HDR_PTR			0x0244
+#define RK3399_AFBCD2_PIC_SIZE			0x0248
+#define RK3399_AFBCD2_STATUS			0x024c
+#define RK3399_AFBCD3_CTRL			0x0260
+#define RK3399_AFBCD3_HDR_PTR			0x0264
+#define RK3399_AFBCD3_PIC_SIZE			0x0268
+#define RK3399_AFBCD3_STATUS			0x026c
+#define RK3399_INTR_EN0				0x0280
+#define RK3399_INTR_CLEAR0			0x0284
+#define RK3399_INTR_STATUS0			0x0288
+#define RK3399_INTR_RAW_STATUS0			0x028c
+#define RK3399_INTR_EN1				0x0290
+#define RK3399_INTR_CLEAR1			0x0294
+#define RK3399_INTR_STATUS1			0x0298
+#define RK3399_INTR_RAW_STATUS1			0x029c
+#define RK3399_LINE_FLAG			0x02a0
+#define RK3399_VOP_STATUS			0x02a4
+#define RK3399_BLANKING_VALUE			0x02a8
+#define RK3399_MCU_BYPASS_PORT			0x02ac
+#define RK3399_WIN0_DSP_BG			0x02b0
+#define RK3399_WIN1_DSP_BG			0x02b4
+#define RK3399_WIN2_DSP_BG			0x02b8
+#define RK3399_WIN3_DSP_BG			0x02bc
+#define RK3399_YUV2YUV_WIN			0x02c0
+#define RK3399_YUV2YUV_POST			0x02c4
+#define RK3399_AUTO_GATING_EN			0x02cc
+#define RK3399_WIN0_CSC_COE			0x03a0
+#define RK3399_WIN1_CSC_COE			0x03c0
+#define RK3399_WIN2_CSC_COE			0x03e0
+#define RK3399_WIN3_CSC_COE			0x0400
+#define RK3399_HWC_CSC_COE			0x0420
+#define RK3399_BCSH_R2Y_CSC_COE			0x0440
+#define RK3399_BCSH_Y2R_CSC_COE			0x0460
+#define RK3399_POST_YUV2YUV_Y2R_COE		0x0480
+#define RK3399_POST_YUV2YUV_3X3_COE		0x04a0
+#define RK3399_POST_YUV2YUV_R2Y_COE		0x04c0
+#define RK3399_WIN0_YUV2YUV_Y2R			0x04e0
+#define RK3399_WIN0_YUV2YUV_3X3			0x0500
+#define RK3399_WIN0_YUV2YUV_R2Y			0x0520
+#define RK3399_WIN1_YUV2YUV_Y2R			0x0540
+#define RK3399_WIN1_YUV2YUV_3X3			0x0560
+#define RK3399_WIN1_YUV2YUV_R2Y			0x0580
+#define RK3399_WIN2_YUV2YUV_Y2R			0x05a0
+#define RK3399_WIN2_YUV2YUV_3X3			0x05c0
+#define RK3399_WIN2_YUV2YUV_R2Y			0x05e0
+#define RK3399_WIN3_YUV2YUV_Y2R			0x0600
+#define RK3399_WIN3_YUV2YUV_3X3			0x0620
+#define RK3399_WIN3_YUV2YUV_R2Y			0x0640
+#define RK3399_WIN2_LUT_ADDR			0x1000
+#define RK3399_WIN3_LUT_ADDR			0x1400
+#define RK3399_HWC_LUT_ADDR			0x1800
+#define RK3399_CABC_GAMMA_LUT_ADDR		0x1c00
+#define RK3399_GAMMA_LUT_ADDR			0x2000
+/* rk3399 register definition end */
+
+/* rk3328 register definition end */
+#define RK3328_REG_CFG_DONE			0x00000000
+#define RK3328_VERSION_INFO			0x00000004
+#define RK3328_SYS_CTRL				0x00000008
+#define RK3328_SYS_CTRL1			0x0000000c
+#define RK3328_DSP_CTRL0			0x00000010
+#define RK3328_DSP_CTRL1			0x00000014
+#define RK3328_DSP_BG				0x00000018
+#define RK3328_AUTO_GATING_EN			0x0000003c
+#define RK3328_LINE_FLAG			0x00000040
+#define RK3328_VOP_STATUS			0x00000044
+#define RK3328_BLANKING_VALUE			0x00000048
+#define RK3328_WIN0_DSP_BG			0x00000050
+#define RK3328_WIN1_DSP_BG			0x00000054
+#define RK3328_DBG_PERF_LATENCY_CTRL0		0x000000c0
+#define RK3328_DBG_PERF_RD_MAX_LATENCY_NUM0	0x000000c4
+#define RK3328_DBG_PERF_RD_LATENCY_THR_NUM0	0x000000c8
+#define RK3328_DBG_PERF_RD_LATENCY_SAMP_NUM0	0x000000cc
+#define RK3328_INTR_EN0				0x000000e0
+#define RK3328_INTR_CLEAR0			0x000000e4
+#define RK3328_INTR_STATUS0			0x000000e8
+#define RK3328_INTR_RAW_STATUS0			0x000000ec
+#define RK3328_INTR_EN1				0x000000f0
+#define RK3328_INTR_CLEAR1			0x000000f4
+#define RK3328_INTR_STATUS1			0x000000f8
+#define RK3328_INTR_RAW_STATUS1			0x000000fc
+#define RK3328_WIN0_CTRL0			0x00000100
+#define RK3328_WIN0_CTRL1			0x00000104
+#define RK3328_WIN0_COLOR_KEY			0x00000108
+#define RK3328_WIN0_VIR				0x0000010c
+#define RK3328_WIN0_YRGB_MST			0x00000110
+#define RK3328_WIN0_CBR_MST			0x00000114
+#define RK3328_WIN0_ACT_INFO			0x00000118
+#define RK3328_WIN0_DSP_INFO			0x0000011c
+#define RK3328_WIN0_DSP_ST			0x00000120
+#define RK3328_WIN0_SCL_FACTOR_YRGB		0x00000124
+#define RK3328_WIN0_SCL_FACTOR_CBR		0x00000128
+#define RK3328_WIN0_SCL_OFFSET			0x0000012c
+#define RK3328_WIN0_SRC_ALPHA_CTRL		0x00000130
+#define RK3328_WIN0_DST_ALPHA_CTRL		0x00000134
+#define RK3328_WIN0_FADING_CTRL			0x00000138
+#define RK3328_WIN0_CTRL2			0x0000013c
+#define RK3328_DBG_WIN0_REG0			0x000001f0
+#define RK3328_DBG_WIN0_REG1			0x000001f4
+#define RK3328_DBG_WIN0_REG2			0x000001f8
+#define RK3328_DBG_WIN0_RESERVED		0x000001fc
+#define RK3328_WIN1_CTRL0			0x00000200
+#define RK3328_WIN1_CTRL1			0x00000204
+#define RK3328_WIN1_COLOR_KEY			0x00000208
+#define RK3328_WIN1_VIR				0x0000020c
+#define RK3328_WIN1_YRGB_MST			0x00000210
+#define RK3328_WIN1_CBR_MST			0x00000214
+#define RK3328_WIN1_ACT_INFO			0x00000218
+#define RK3328_WIN1_DSP_INFO			0x0000021c
+#define RK3328_WIN1_DSP_ST			0x00000220
+#define RK3328_WIN1_SCL_FACTOR_YRGB		0x00000224
+#define RK3328_WIN1_SCL_FACTOR_CBR		0x00000228
+#define RK3328_WIN1_SCL_OFFSET			0x0000022c
+#define RK3328_WIN1_SRC_ALPHA_CTRL		0x00000230
+#define RK3328_WIN1_DST_ALPHA_CTRL		0x00000234
+#define RK3328_WIN1_FADING_CTRL			0x00000238
+#define RK3328_WIN1_CTRL2			0x0000023c
+#define RK3328_DBG_WIN1_REG0			0x000002f0
+#define RK3328_DBG_WIN1_REG1			0x000002f4
+#define RK3328_DBG_WIN1_REG2			0x000002f8
+#define RK3328_DBG_WIN1_RESERVED		0x000002fc
+#define RK3328_WIN2_CTRL0			0x00000300
+#define RK3328_WIN2_CTRL1			0x00000304
+#define RK3328_WIN2_COLOR_KEY			0x00000308
+#define RK3328_WIN2_VIR				0x0000030c
+#define RK3328_WIN2_YRGB_MST			0x00000310
+#define RK3328_WIN2_CBR_MST			0x00000314
+#define RK3328_WIN2_ACT_INFO			0x00000318
+#define RK3328_WIN2_DSP_INFO			0x0000031c
+#define RK3328_WIN2_DSP_ST			0x00000320
+#define RK3328_WIN2_SCL_FACTOR_YRGB		0x00000324
+#define RK3328_WIN2_SCL_FACTOR_CBR		0x00000328
+#define RK3328_WIN2_SCL_OFFSET			0x0000032c
+#define RK3328_WIN2_SRC_ALPHA_CTRL		0x00000330
+#define RK3328_WIN2_DST_ALPHA_CTRL		0x00000334
+#define RK3328_WIN2_FADING_CTRL			0x00000338
+#define RK3328_WIN2_CTRL2			0x0000033c
+#define RK3328_DBG_WIN2_REG0			0x000003f0
+#define RK3328_DBG_WIN2_REG1			0x000003f4
+#define RK3328_DBG_WIN2_REG2			0x000003f8
+#define RK3328_DBG_WIN2_RESERVED		0x000003fc
+#define RK3328_WIN3_CTRL0			0x00000400
+#define RK3328_WIN3_CTRL1			0x00000404
+#define RK3328_WIN3_COLOR_KEY			0x00000408
+#define RK3328_WIN3_VIR				0x0000040c
+#define RK3328_WIN3_YRGB_MST			0x00000410
+#define RK3328_WIN3_CBR_MST			0x00000414
+#define RK3328_WIN3_ACT_INFO			0x00000418
+#define RK3328_WIN3_DSP_INFO			0x0000041c
+#define RK3328_WIN3_DSP_ST			0x00000420
+#define RK3328_WIN3_SCL_FACTOR_YRGB		0x00000424
+#define RK3328_WIN3_SCL_FACTOR_CBR		0x00000428
+#define RK3328_WIN3_SCL_OFFSET			0x0000042c
+#define RK3328_WIN3_SRC_ALPHA_CTRL		0x00000430
+#define RK3328_WIN3_DST_ALPHA_CTRL		0x00000434
+#define RK3328_WIN3_FADING_CTRL			0x00000438
+#define RK3328_WIN3_CTRL2			0x0000043c
+#define RK3328_DBG_WIN3_REG0			0x000004f0
+#define RK3328_DBG_WIN3_REG1			0x000004f4
+#define RK3328_DBG_WIN3_REG2			0x000004f8
+#define RK3328_DBG_WIN3_RESERVED		0x000004fc
+
+#define RK3328_HWC_CTRL0			0x00000500
+#define RK3328_HWC_CTRL1			0x00000504
+#define RK3328_HWC_MST				0x00000508
+#define RK3328_HWC_DSP_ST			0x0000050c
+#define RK3328_HWC_SRC_ALPHA_CTRL		0x00000510
+#define RK3328_HWC_DST_ALPHA_CTRL		0x00000514
+#define RK3328_HWC_FADING_CTRL			0x00000518
+#define RK3328_HWC_RESERVED1			0x0000051c
+#define RK3328_POST_DSP_HACT_INFO		0x00000600
+#define RK3328_POST_DSP_VACT_INFO		0x00000604
+#define RK3328_POST_SCL_FACTOR_YRGB		0x00000608
+#define RK3328_POST_RESERVED			0x0000060c
+#define RK3328_POST_SCL_CTRL			0x00000610
+#define RK3328_POST_DSP_VACT_INFO_F1		0x00000614
+#define RK3328_DSP_HTOTAL_HS_END		0x00000618
+#define RK3328_DSP_HACT_ST_END			0x0000061c
+#define RK3328_DSP_VTOTAL_VS_END		0x00000620
+#define RK3328_DSP_VACT_ST_END			0x00000624
+#define RK3328_DSP_VS_ST_END_F1			0x00000628
+#define RK3328_DSP_VACT_ST_END_F1		0x0000062c
+#define RK3328_BCSH_COLOR_BAR			0x00000640
+#define RK3328_BCSH_BCS				0x00000644
+#define RK3328_BCSH_H				0x00000648
+#define RK3328_BCSH_CTRL			0x0000064c
+#define RK3328_FRC_LOWER01_0			0x00000678
+#define RK3328_FRC_LOWER01_1			0x0000067c
+#define RK3328_FRC_LOWER10_0			0x00000680
+#define RK3328_FRC_LOWER10_1			0x00000684
+#define RK3328_FRC_LOWER11_0			0x00000688
+#define RK3328_FRC_LOWER11_1			0x0000068c
+#define RK3328_DBG_POST_REG0			0x000006e8
+#define RK3328_DBG_POST_RESERVED		0x000006ec
+#define RK3328_DBG_DATAO			0x000006f0
+#define RK3328_DBG_DATAO_2			0x000006f4
+
+/* sdr to hdr */
+#define RK3328_SDR2HDR_CTRL			0x00000700
+#define RK3328_EOTF_OETF_Y0			0x00000704
+#define RK3328_RESERVED0001			0x00000708
+#define RK3328_RESERVED0002			0x0000070c
+#define RK3328_EOTF_OETF_Y1			0x00000710
+#define RK3328_EOTF_OETF_Y64			0x0000080c
+#define RK3328_OETF_DX_DXPOW1			0x00000810
+#define RK3328_OETF_DX_DXPOW64			0x0000090c
+#define RK3328_OETF_XN1				0x00000910
+#define RK3328_OETF_XN63			0x00000a08
+
+/* hdr to sdr */
+#define RK3328_HDR2SDR_CTRL			0x00000a10
+#define RK3328_HDR2SDR_SRC_RANGE		0x00000a14
+#define RK3328_HDR2SDR_NORMFACEETF		0x00000a18
+#define RK3328_RESERVED0003			0x00000a1c
+#define RK3328_HDR2SDR_DST_RANGE		0x00000a20
+#define RK3328_HDR2SDR_NORMFACCGAMMA		0x00000a24
+#define RK3328_EETF_OETF_Y0			0x00000a28
+#define RK3328_SAT_Y0				0x00000a2c
+#define RK3328_EETF_OETF_Y1			0x00000a30
+#define RK3328_SAT_Y1				0x00000ab0
+#define RK3328_SAT_Y8				0x00000acc
+
+#define RK3328_HWC_LUT_ADDR			0x00000c00
+
 /* rk3036 register definition */
 #define RK3036_SYS_CTRL			0x00
 #define RK3036_DSP_CTRL0		0x04
@@ -166,197 +878,4 @@
 #define RK3036_HWC_LUT_ADDR		0x800
 /* rk3036 register definition end */
 
-/* rk3399 register definition */
-#define RK3399_REG_CFG_DONE		0x00000
-#define RK3399_VERSION_INFO		0x00004
-#define RK3399_SYS_CTRL			0x00008
-#define RK3399_SYS_CTRL1		0x0000c
-#define RK3399_DSP_CTRL0		0x00010
-#define RK3399_DSP_CTRL1		0x00014
-#define RK3399_DSP_BG			0x00018
-#define RK3399_MCU_CTRL			0x0001c
-#define RK3399_WB_CTRL0			0x00020
-#define RK3399_WB_CTRL1			0x00024
-#define RK3399_WB_YRGB_MST		0x00028
-#define RK3399_WB_CBR_MST		0x0002c
-#define RK3399_WIN0_CTRL0		0x00030
-#define RK3399_WIN0_CTRL1		0x00034
-#define RK3399_WIN0_COLOR_KEY		0x00038
-#define RK3399_WIN0_VIR			0x0003c
-#define RK3399_WIN0_YRGB_MST		0x00040
-#define RK3399_WIN0_CBR_MST		0x00044
-#define RK3399_WIN0_ACT_INFO		0x00048
-#define RK3399_WIN0_DSP_INFO		0x0004c
-#define RK3399_WIN0_DSP_ST		0x00050
-#define RK3399_WIN0_SCL_FACTOR_YRGB	0x00054
-#define RK3399_WIN0_SCL_FACTOR_CBR	0x00058
-#define RK3399_WIN0_SCL_OFFSET		0x0005c
-#define RK3399_WIN0_SRC_ALPHA_CTRL	0x00060
-#define RK3399_WIN0_DST_ALPHA_CTRL	0x00064
-#define RK3399_WIN0_FADING_CTRL		0x00068
-#define RK3399_WIN0_CTRL2		0x0006c
-#define RK3399_WIN1_CTRL0		0x00070
-#define RK3399_WIN1_CTRL1		0x00074
-#define RK3399_WIN1_COLOR_KEY		0x00078
-#define RK3399_WIN1_VIR			0x0007c
-#define RK3399_WIN1_YRGB_MST		0x00080
-#define RK3399_WIN1_CBR_MST		0x00084
-#define RK3399_WIN1_ACT_INFO		0x00088
-#define RK3399_WIN1_DSP_INFO		0x0008c
-#define RK3399_WIN1_DSP_ST		0x00090
-#define RK3399_WIN1_SCL_FACTOR_YRGB	0x00094
-#define RK3399_WIN1_SCL_FACTOR_CBR	0x00098
-#define RK3399_WIN1_SCL_OFFSET		0x0009c
-#define RK3399_WIN1_SRC_ALPHA_CTRL	0x000a0
-#define RK3399_WIN1_DST_ALPHA_CTRL	0x000a4
-#define RK3399_WIN1_FADING_CTRL		0x000a8
-#define RK3399_WIN1_CTRL2		0x000ac
-#define RK3399_WIN2_CTRL0		0x000b0
-#define RK3399_WIN2_CTRL1		0x000b4
-#define RK3399_WIN2_VIR0_1		0x000b8
-#define RK3399_WIN2_VIR2_3		0x000bc
-#define RK3399_WIN2_MST0		0x000c0
-#define RK3399_WIN2_DSP_INFO0		0x000c4
-#define RK3399_WIN2_DSP_ST0		0x000c8
-#define RK3399_WIN2_COLOR_KEY		0x000cc
-#define RK3399_WIN2_MST1		0x000d0
-#define RK3399_WIN2_DSP_INFO1		0x000d4
-#define RK3399_WIN2_DSP_ST1		0x000d8
-#define RK3399_WIN2_SRC_ALPHA_CTRL	0x000dc
-#define RK3399_WIN2_MST2		0x000e0
-#define RK3399_WIN2_DSP_INFO2		0x000e4
-#define RK3399_WIN2_DSP_ST2		0x000e8
-#define RK3399_WIN2_DST_ALPHA_CTRL	0x000ec
-#define RK3399_WIN2_MST3		0x000f0
-#define RK3399_WIN2_DSP_INFO3		0x000f4
-#define RK3399_WIN2_DSP_ST3		0x000f8
-#define RK3399_WIN2_FADING_CTRL		0x000fc
-#define RK3399_WIN3_CTRL0		0x00100
-#define RK3399_WIN3_CTRL1		0x00104
-#define RK3399_WIN3_VIR0_1		0x00108
-#define RK3399_WIN3_VIR2_3		0x0010c
-#define RK3399_WIN3_MST0		0x00110
-#define RK3399_WIN3_DSP_INFO0		0x00114
-#define RK3399_WIN3_DSP_ST0		0x00118
-#define RK3399_WIN3_COLOR_KEY		0x0011c
-#define RK3399_WIN3_MST1		0x00120
-#define RK3399_WIN3_DSP_INFO1		0x00124
-#define RK3399_WIN3_DSP_ST1		0x00128
-#define RK3399_WIN3_SRC_ALPHA_CTRL	0x0012c
-#define RK3399_WIN3_MST2		0x00130
-#define RK3399_WIN3_DSP_INFO2		0x00134
-#define RK3399_WIN3_DSP_ST2		0x00138
-#define RK3399_WIN3_DST_ALPHA_CTRL	0x0013c
-#define RK3399_WIN3_MST3		0x00140
-#define RK3399_WIN3_DSP_INFO3		0x00144
-#define RK3399_WIN3_DSP_ST3		0x00148
-#define RK3399_WIN3_FADING_CTRL		0x0014c
-#define RK3399_HWC_CTRL0		0x00150
-#define RK3399_HWC_CTRL1		0x00154
-#define RK3399_HWC_MST			0x00158
-#define RK3399_HWC_DSP_ST		0x0015c
-#define RK3399_HWC_SRC_ALPHA_CTRL	0x00160
-#define RK3399_HWC_DST_ALPHA_CTRL	0x00164
-#define RK3399_HWC_FADING_CTRL		0x00168
-#define RK3399_HWC_RESERVED1		0x0016c
-#define RK3399_POST_DSP_HACT_INFO	0x00170
-#define RK3399_POST_DSP_VACT_INFO	0x00174
-#define RK3399_POST_SCL_FACTOR_YRGB	0x00178
-#define RK3399_POST_RESERVED		0x0017c
-#define RK3399_POST_SCL_CTRL		0x00180
-#define RK3399_POST_DSP_VACT_INFO_F1	0x00184
-#define RK3399_DSP_HTOTAL_HS_END	0x00188
-#define RK3399_DSP_HACT_ST_END		0x0018c
-#define RK3399_DSP_VTOTAL_VS_END	0x00190
-#define RK3399_DSP_VACT_ST_END		0x00194
-#define RK3399_DSP_VS_ST_END_F1		0x00198
-#define RK3399_DSP_VACT_ST_END_F1	0x0019c
-#define RK3399_PWM_CTRL			0x001a0
-#define RK3399_PWM_PERIOD_HPR		0x001a4
-#define RK3399_PWM_DUTY_LPR		0x001a8
-#define RK3399_PWM_CNT			0x001ac
-#define RK3399_BCSH_COLOR_BAR		0x001b0
-#define RK3399_BCSH_BCS			0x001b4
-#define RK3399_BCSH_H			0x001b8
-#define RK3399_BCSH_CTRL		0x001bc
-#define RK3399_CABC_CTRL0		0x001c0
-#define RK3399_CABC_CTRL1		0x001c4
-#define RK3399_CABC_CTRL2		0x001c8
-#define RK3399_CABC_CTRL3		0x001cc
-#define RK3399_CABC_GAUSS_LINE0_0	0x001d0
-#define RK3399_CABC_GAUSS_LINE0_1	0x001d4
-#define RK3399_CABC_GAUSS_LINE1_0	0x001d8
-#define RK3399_CABC_GAUSS_LINE1_1	0x001dc
-#define RK3399_CABC_GAUSS_LINE2_0	0x001e0
-#define RK3399_CABC_GAUSS_LINE2_1	0x001e4
-#define RK3399_FRC_LOWER01_0		0x001e8
-#define RK3399_FRC_LOWER01_1		0x001ec
-#define RK3399_FRC_LOWER10_0		0x001f0
-#define RK3399_FRC_LOWER10_1		0x001f4
-#define RK3399_FRC_LOWER11_0		0x001f8
-#define RK3399_FRC_LOWER11_1		0x001fc
-#define RK3399_AFBCD0_CTRL		0x00200
-#define RK3399_AFBCD0_HDR_PTR		0x00204
-#define RK3399_AFBCD0_PIC_SIZE		0x00208
-#define RK3399_AFBCD0_STATUS		0x0020c
-#define RK3399_AFBCD1_CTRL		0x00220
-#define RK3399_AFBCD1_HDR_PTR		0x00224
-#define RK3399_AFBCD1_PIC_SIZE		0x00228
-#define RK3399_AFBCD1_STATUS		0x0022c
-#define RK3399_AFBCD2_CTRL		0x00240
-#define RK3399_AFBCD2_HDR_PTR		0x00244
-#define RK3399_AFBCD2_PIC_SIZE		0x00248
-#define RK3399_AFBCD2_STATUS		0x0024c
-#define RK3399_AFBCD3_CTRL		0x00260
-#define RK3399_AFBCD3_HDR_PTR		0x00264
-#define RK3399_AFBCD3_PIC_SIZE		0x00268
-#define RK3399_AFBCD3_STATUS		0x0026c
-#define RK3399_INTR_EN0			0x00280
-#define RK3399_INTR_CLEAR0		0x00284
-#define RK3399_INTR_STATUS0		0x00288
-#define RK3399_INTR_RAW_STATUS0		0x0028c
-#define RK3399_INTR_EN1			0x00290
-#define RK3399_INTR_CLEAR1		0x00294
-#define RK3399_INTR_STATUS1		0x00298
-#define RK3399_INTR_RAW_STATUS1		0x0029c
-#define RK3399_LINE_FLAG		0x002a0
-#define RK3399_VOP_STATUS		0x002a4
-#define RK3399_BLANKING_VALUE		0x002a8
-#define RK3399_MCU_BYPASS_PORT		0x002ac
-#define RK3399_WIN0_DSP_BG		0x002b0
-#define RK3399_WIN1_DSP_BG		0x002b4
-#define RK3399_WIN2_DSP_BG		0x002b8
-#define RK3399_WIN3_DSP_BG		0x002bc
-#define RK3399_YUV2YUV_WIN		0x002c0
-#define RK3399_YUV2YUV_POST		0x002c4
-#define RK3399_AUTO_GATING_EN		0x002cc
-#define RK3399_WIN0_CSC_COE		0x003a0
-#define RK3399_WIN1_CSC_COE		0x003c0
-#define RK3399_WIN2_CSC_COE		0x003e0
-#define RK3399_WIN3_CSC_COE		0x00400
-#define RK3399_HWC_CSC_COE		0x00420
-#define RK3399_BCSH_R2Y_CSC_COE		0x00440
-#define RK3399_BCSH_Y2R_CSC_COE		0x00460
-#define RK3399_POST_YUV2YUV_Y2R_COE	0x00480
-#define RK3399_POST_YUV2YUV_3X3_COE	0x004a0
-#define RK3399_POST_YUV2YUV_R2Y_COE	0x004c0
-#define RK3399_WIN0_YUV2YUV_Y2R		0x004e0
-#define RK3399_WIN0_YUV2YUV_3X3		0x00500
-#define RK3399_WIN0_YUV2YUV_R2Y		0x00520
-#define RK3399_WIN1_YUV2YUV_Y2R		0x00540
-#define RK3399_WIN1_YUV2YUV_3X3		0x00560
-#define RK3399_WIN1_YUV2YUV_R2Y		0x00580
-#define RK3399_WIN2_YUV2YUV_Y2R		0x005a0
-#define RK3399_WIN2_YUV2YUV_3X3		0x005c0
-#define RK3399_WIN2_YUV2YUV_R2Y		0x005e0
-#define RK3399_WIN3_YUV2YUV_Y2R		0x00600
-#define RK3399_WIN3_YUV2YUV_3X3		0x00620
-#define RK3399_WIN3_YUV2YUV_R2Y		0x00640
-#define RK3399_WIN2_LUT_ADDR		0x01000
-#define RK3399_WIN3_LUT_ADDR		0x01400
-#define RK3399_HWC_LUT_ADDR		0x01800
-#define RK3399_CABC_GAMMA_LUT_ADDR	0x01c00
-#define RK3399_GAMMA_LUT_ADDR		0x02000
-/* rk3399 register definition end */
-
 #endif /* _ROCKCHIP_VOP_REG_H */
-- 
1.9.1

[PATCH v2 4/5] drm/rockchip: vop: add a series of vop support

[Sean Paul]

On Wed, Jul 12, 2017 at 10:03:54AM +0800, Mark Yao wrote:
> Vop Full framework now has following vops:
> IP version    chipname
>   3.1           rk3288
>   3.2           rk3368
>   3.4           rk3366
>   3.5           rk3399 big
>   3.6           rk3399 lit

Below you say little vop is major == 2, but you have major == 3 here.

>   3.7           rk3228
>   3.8           rk3328
> 
> The above IP version is from H/W define, some of vop support get
> the IP version from VERSION_INFO register, some are not.
> hardcode the IP version for each vop to identify them.
> 
> major version: used for IP structure, Vop full framework is 3,
>                vop little framework is 2.
> minor version: on same structure, newer design vop will bigger
>                then old one.
> 
> Changes in v2:
> - rename rk322x to rk3228(Heiko St?bner)
> - correct some vop registers define
> 
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 202 +++++--
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.h | 905 ++++++++++++++++++++++------
>  2 files changed, 863 insertions(+), 244 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 159cedf..b33483c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -211,6 +211,7 @@
>  	.standby = VOP_REG(RK3288_SYS_CTRL, 0x1, 22),
>  	.gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23),
>  	.mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20),
> +	.dp_en = VOP_REG_VER(RK3399_SYS_CTRL, 0x1, 11, 3, 5, -1),
>  	.rgb_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 12),
>  	.hdmi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 13),
>  	.edp_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 14),
> @@ -220,14 +221,19 @@
>  	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
>  	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
>  	.out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
> -	.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
> +	.pin_pol = VOP_REG_VER(RK3288_DSP_CTRL0, 0xf, 4, 3, 0, 1),
> +	.dp_pin_pol = VOP_REG_VER(RK3399_DSP_CTRL1, 0xf, 16, 3, 5, -1),
> +	.rgb_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 16, 3, 2, -1),
> +	.hdmi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 20, 3, 2, -1),
> +	.edp_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 24, 3, 2, -1),
> +	.mipi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 28, 3, 2, -1),
>  	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
>  	.hact_st_end = VOP_REG(RK3288_DSP_HACT_ST_END, 0x1fff1fff, 0),
>  	.vtotal_pw = VOP_REG(RK3288_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>  	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>  	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> -	.global_regdone_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 11),
> +	.global_regdone_en = VOP_REG_VER(RK3288_SYS_CTRL, 0x1, 11, 3, 2, -1),
>  	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),

I'm not really convinced VOP_REG_VER is a good idea. In the case of dp_en and
pin_pol, the register is already used for different offsets, so presumably
you're writing into don't care offsets?

In the other cases, it just looks like a few new registers added for 3368 and
3399. In this scenario, I don't think it's that bad to just have separate
structs for each version that has distinct features. There's going to be more
duplication, but then it's super easy to understand which platform has which
registers.

The whole versioning system is a little strange. For example, is each version
guaranteed to have the registered defined for the previous version (ie: 3.6
contains all registers defined for 3.5)?

Sean


<snip>

> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

[PATCH v2 4/5] drm/rockchip: vop: add a series of vop support

[Sean Paul]

On Wed, Jul 12, 2017 at 9:31 PM, Mark yao <mark.yao@rock-chips.com> wrote:
> On 2017?07?13? 02:33, Sean Paul wrote:
>
> On Wed, Jul 12, 2017 at 10:03:54AM +0800, Mark Yao wrote:
>
> Vop Full framework now has following vops:
> IP version    chipname
>   3.1           rk3288
>   3.2           rk3368
>   3.4           rk3366
>   3.5           rk3399 big
>   3.6           rk3399 lit
>
> Below you say little vop is major == 2, but you have major == 3 here.
>
>
> "RK3399 lit" is a full design vop framework, just cut down some feature from
> rk3399 big design, so it's IP major is 3.
>
> the little vop is rk3066, rk3188, rk3036, etc.
>

Hi Mark,
Please avoid using HTML mail on the mailing list, it makes mutt angry
and gmail doesn't quote the thread properly.

>
>
>   3.7           rk3228
>   3.8           rk3328
>
> The above IP version is from H/W define, some of vop support get
> the IP version from VERSION_INFO register, some are not.
> hardcode the IP version for each vop to identify them.
>
> major version: used for IP structure, Vop full framework is 3,
>                vop little framework is 2.
> minor version: on same structure, newer design vop will bigger
>                then old one.
>
> Changes in v2:
> - rename rk322x to rk3228(Heiko St?bner)
> - correct some vop registers define
>
> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 202 +++++--
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.h | 905
> ++++++++++++++++++++++------
>  2 files changed, 863 insertions(+), 244 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 159cedf..b33483c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -211,6 +211,7 @@
>   .standby = VOP_REG(RK3288_SYS_CTRL, 0x1, 22),
>   .gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23),
>   .mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20),
> + .dp_en = VOP_REG_VER(RK3399_SYS_CTRL, 0x1, 11, 3, 5, -1),
>   .rgb_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 12),
>   .hdmi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 13),
>   .edp_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 14),
> @@ -220,14 +221,19 @@
>   .data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
>   .dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
>   .out_mode = VOP_REG(RK3288_DSP_CTRL0, 0xf, 0),
> - .pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),
> + .pin_pol = VOP_REG_VER(RK3288_DSP_CTRL0, 0xf, 4, 3, 0, 1),
> + .dp_pin_pol = VOP_REG_VER(RK3399_DSP_CTRL1, 0xf, 16, 3, 5, -1),
> + .rgb_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 16, 3, 2, -1),
> + .hdmi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 20, 3, 2, -1),
> + .edp_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 24, 3, 2, -1),
> + .mipi_pin_pol = VOP_REG_VER(RK3368_DSP_CTRL1, 0xf, 28, 3, 2, -1),
>   .htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
>   .hact_st_end = VOP_REG(RK3288_DSP_HACT_ST_END, 0x1fff1fff, 0),
>   .vtotal_pw = VOP_REG(RK3288_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>   .vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>   .hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>   .vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> - .global_regdone_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 11),
> + .global_regdone_en = VOP_REG_VER(RK3288_SYS_CTRL, 0x1, 11, 3, 2, -1),
>   .cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
>
> I'm not really convinced VOP_REG_VER is a good idea. In the case of dp_en
> and
> pin_pol, the register is already used for different offsets, so presumably
> you're writing into don't care offsets?
>
>
> VOP_REG_VER control when the register works.
> for pin_pol, it only works on version(3.0 - 3.1),  rk3288, it's dummy
> operate on other chips.
> for dp_en, it works on version (3.5 - bigger), rk3399 big, rk3399 lit,
> rk322x, etc.
> it's dummy operate on rk3288, rk3368.
>
> Using VOP_REG means the register works on all platform,  Using VOP_REG_VER
> means only can use on some platform.
>
> So don't worry about writing wrong offsets, the register would be dummy on
> un-support platform.

Yeah, I understand what it does, and why you did it, I just think it's
overly complicated. I'd rather just see separate structures for each
unique piece of silicon vs having to parse version numbers to figure
out what's present on my chip.

>
> In the other cases, it just looks like a few new registers added for 3368
> and
> 3399. In this scenario, I don't think it's that bad to just have separate
> structs for each version that has distinct features. There's going to be
> more
> duplication, but then it's super easy to understand which platform has which
> registers.
>
>
> separate structs is TODO on my plan, separate with control, modesetting,
> etc.
> but many register has no a clean group define, and need rewrite write/read
> ops,
> need handle conflict for each platform.

I'm not worried about further splitting everything up. If you see a
pattern, go for it, otherwise just duplicate. It's not that big of a
deal IMO.

Sean

>
> And I think  VOP_REG_VER mechanism can both works with separate structs.
>
>
> The whole versioning system is a little strange. For example, is each
> version
> guaranteed to have the registered defined for the previous version (ie: 3.6
> contains all registers defined for 3.5)?
>
>
> Mostly, new version will contains all register of previous, if some register
> only support on old version,
> example, pin_pol, only support on rk3288, use VOP_REG_VER limit it only can
> work on 3288.
> so it's no problem for this case.
>
> And If newer design vop actually have many register difference, no possible
> to using VOP_REG_VER,
> example rk3328, that means we need create a new register table for it.
>
>
> Sean
>
>
>
> Using VOP_REG_VER is to solved tiny difference registers on same vop design,
> then those
> vop can reuse register table. after that we can easy found what register
> difference
> for those same design vop from register table.
>
> H/W design always do that on design update: fix bugs and add new feature,
> With register table reusing, if we add a new feature for old design, all the
> newer design
> will get benefit.
>
>
>
> <snip>
>
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> ?ark Yao

[PATCH v2 5/5] dt-bindings: display: fill Documents for series of vop

[Mark Yao]

Changes in v2:
- rename rk322x to rk3228(Heiko St?bner)

Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
---
 Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
index 9eb3f0a..5d835d9 100644
--- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt
@@ -8,8 +8,12 @@ Required properties:
 - compatible: value should be one of the following
 		"rockchip,rk3036-vop";
 		"rockchip,rk3288-vop";
+		"rockchip,rk3368-vop";
+		"rockchip,rk3366-vop";
 		"rockchip,rk3399-vop-big";
 		"rockchip,rk3399-vop-lit";
+		"rockchip,rk3228-vop";
+		"rockchip,rk3328-vop";
 
 - interrupts: should contain a list of all VOP IP block interrupts in the
 		 order: VSYNC, LCD_SYSTEM. The interrupt specifier
-- 
1.9.1

[PATCH v2 5/5] dt-bindings: display: fill Documents for series of vop

[Rob Herring]

On Wed, Jul 12, 2017 at 10:04:02AM +0800, Mark Yao wrote:
> Changes in v2:
> - rename rk322x to rk3228(Heiko St?bner)

This goes below '---' and you need a commit msg here. Also, it is not 
clear in the subject this is for Rockchip.


> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
> ---
>  Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt | 4 ++++
>  1 file changed, 4 insertions(+)

[PATCH v2 5/5] dt-bindings: display: fill Documents for series of vop

[Mark yao]

On 2017?07?18? 01:24, Rob Herring wrote:
> On Wed, Jul 12, 2017 at 10:04:02AM +0800, Mark Yao wrote:
>> Changes in v2:
>> - rename rk322x to rk3228(Heiko St?bner)
> This goes below '---' and you need a commit msg here. Also, it is not
> clear in the subject this is for Rockchip.
>

Got it, will fix it at next version.

Thanks.

>> Signed-off-by: Mark Yao <mark.yao@rock-chips.com>
>> ---
>>   Documentation/devicetree/bindings/display/rockchip/rockchip-vop.txt | 4 ++++
>>   1 file changed, 4 insertions(+)
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>


-- 
?ark Yao