Re: [PATCH v6 4/7] drm/rockchip: vop: group vop registers

2017-07-27 Thread Mark yao

Hi Heiko

On 2017年07月28日 09:02, Mark yao wrote:

Hi Heiko

Thanks for the test.

On 2017年07月27日 18:10, Heiko Stübner wrote:

Am Donnerstag, 27. Juli 2017, 11:51:06 CEST schrieb Heiko Stübner:

Hi Mark,

Am Mittwoch, 26. Juli 2017, 14:19:25 CEST schrieb Mark Yao:

Grouping the vop registers facilitates make register
definition clearer, and also is useful for different vop
reuse the same group register.

Signed-off-by: Mark Yao 
---

  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  99
  
  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  60 ---
  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 112

+++- 3 files changed, 144 insertions(+), 127
deletions(-)

This breaks display support on both rk3036 and rk3288 and I end up
with a null pointer dereference in

[   10.640297] Unable to handle kernel NULL pointer dereference at virtual
address  [   10.654430] pgd = c0204000
[   10.657452] [] *pgd=
[   10.661473] Internal error: Oops: 5 [#1] SMP ARM
[   10.35] Modules linked in: snd_pcm media snd_timer phy_rockchip_dp
snd soundcore rockchipdrm dw_hdmi analogix_dp rtc_rk808 pwm_rockchip
clk_rk808 spi_rockchip [   10.682897] CPU: 2 PID: 143 Comm: kworker/2:2 Not
tainted 4.13.0-rc2-01791-g2b86603d0515 #355 [   10.692430] Hardware name:
Rockchip (Device Tree)
[   10.697692] Workqueue: events deferred_probe_work_func
[   10.702152] Linux video capture interface: v2.00
[   10.708590] task: ee38c800 task.stack: ed2e6000
[   10.713656] PC is at vop_reg_set.constprop.4+0x4/0xa8 [rockchipdrm]
[   10.720668] LR is at vop_bind+0x568/0x8a0 [rockchipdrm]

The obvious reason for that is


@@ -164,14 +153,20 @@ static inline uint32_t vop_read_reg(struct vop *vop,
uint32_t base, return (vop_readl(vop, base + reg->offset) >> reg->shift) &
reg->mask; }

-static inline void vop_mask_write(struct vop *vop, uint32_t offset,
-  uint32_t mask, uint32_t shift, uint32_t v,
-  bool write_mask, bool relaxed)
+static void vop_reg_set(struct vop *vop, const struct vop_reg *reg,
+uint32_t _offset, uint32_t _mask, uint32_t v,
+const char *reg_name)
  {
-if (!mask)
+int offset = reg->offset + _offset;
+int mask = reg->mask & _mask;
+int shift = reg->shift;


Does the crash is that using reg->offset/mask/shift before !reg checking?


I reproduce this cause, from objdump, it crash on "int mask = reg->mask & _mask; 
"
Seems difference gcc has difference behavior, my aarch64 gcc maybe optimize it,
only access when the value be used. I think that is the reason why rk3399 works 
on my test.

I will fix it at next version.




+
+if (!reg || !reg->mask) {
+dev_dbg(vop->dev, "Warning: not support %s\n", reg_name);
  return;
+}

where the check for !reg happens after it got already dereferenced.
But even with that fixed I end up with

on rk3288:
[7.254823] rockchip-vop ff93.vop: Warning: not support global_regdone_en
[7.262847] rockchip-vop ff93.vop: Warning: not support gate
[7.269580] rockchip-vop ff93.vop: Warning: not support gate
[7.302765] rockchip-vop ff94.vop: Warning: not support global_regdone_en
[7.310758] rockchip-vop ff94.vop: Warning: not support gate
[7.317475] rockchip-vop ff94.vop: Warning: not support gate
[7.425724] rockchip-vop ff93.vop: Warning: not support edp_pin_pol
[7.526298] rockchip-vop ff94.vop: Warning: not support hdmi_pin_pol


Rk3288 does not support independent pin_pol settings, all output interfaces 
share the pin_pol register,
So not support hdmi_pin_pol here is correct.

hdmi pin pol would be set by following register:
.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),



on rk3036:
[   12.389138] rockchip-vop 10118000.vop: Warning: not support global_regdone_en
[   12.397324] rockchip-vop 10118000.vop: Warning: not support gate
[   12.404165] rockchip-vop 10118000.vop: Warning: not support gate
[   13.747361] rockchip-vop 10118000.vop: Warning: not support hdmi_pin_pol
[   13.747371] rockchip-vop 10118000.vop: Warning: not support hdmi_en
[   13.747379] rockchip-vop 10118000.vop: Warning: not support hpost_st_end
[   13.747385] rockchip-vop 10118000.vop: Warning: not support vpost_st_end
[   13.747461] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.767098] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.786060] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl

While reqdone and friends are obviously features of newer vops, at least
the hdmi pin-pol is available on both these socs.

With this patch applied (and null-ptr fixed) I end up without hdmi output
on both socs.


Hmmm, I am confused, from code review, I didn't see what will cause hdmi not 
work on rk3036 and rk3288,

Give me some time, I try to bringup my popmetal rk3288 board to do the test.

Heiko, Thanks very much for your test.


Hi Heiko

Re: [PATCH v6 4/7] drm/rockchip: vop: group vop registers

2017-07-27 Thread Mark yao

Hi Heiko

Thanks for the test.

On 2017年07月27日 18:10, Heiko Stübner wrote:

Am Donnerstag, 27. Juli 2017, 11:51:06 CEST schrieb Heiko Stübner:

Hi Mark,

Am Mittwoch, 26. Juli 2017, 14:19:25 CEST schrieb Mark Yao:

Grouping the vop registers facilitates make register
definition clearer, and also is useful for different vop
reuse the same group register.

Signed-off-by: Mark Yao 
---

  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  99
  
  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  60 ---
  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 112

+++- 3 files changed, 144 insertions(+), 127
deletions(-)

This breaks display support on both rk3036 and rk3288 and I end up
with a null pointer dereference in

[   10.640297] Unable to handle kernel NULL pointer dereference at virtual
address  [   10.654430] pgd = c0204000
[   10.657452] [] *pgd=
[   10.661473] Internal error: Oops: 5 [#1] SMP ARM
[   10.35] Modules linked in: snd_pcm media snd_timer phy_rockchip_dp
snd soundcore rockchipdrm dw_hdmi analogix_dp rtc_rk808 pwm_rockchip
clk_rk808 spi_rockchip [   10.682897] CPU: 2 PID: 143 Comm: kworker/2:2 Not
tainted 4.13.0-rc2-01791-g2b86603d0515 #355 [   10.692430] Hardware name:
Rockchip (Device Tree)
[   10.697692] Workqueue: events deferred_probe_work_func
[   10.702152] Linux video capture interface: v2.00
[   10.708590] task: ee38c800 task.stack: ed2e6000
[   10.713656] PC is at vop_reg_set.constprop.4+0x4/0xa8 [rockchipdrm]
[   10.720668] LR is at vop_bind+0x568/0x8a0 [rockchipdrm]

The obvious reason for that is


@@ -164,14 +153,20 @@ static inline uint32_t vop_read_reg(struct vop *vop,
uint32_t base, return (vop_readl(vop, base + reg->offset) >> reg->shift) &
reg->mask; }

-static inline void vop_mask_write(struct vop *vop, uint32_t offset,
- uint32_t mask, uint32_t shift, uint32_t v,
- bool write_mask, bool relaxed)
+static void vop_reg_set(struct vop *vop, const struct vop_reg *reg,
+   uint32_t _offset, uint32_t _mask, uint32_t v,
+   const char *reg_name)
  {
-   if (!mask)
+   int offset = reg->offset + _offset;
+   int mask = reg->mask & _mask;
+   int shift = reg->shift;


Does the crash is that using reg->offset/mask/shift before !reg checking?


+
+   if (!reg || !reg->mask) {
+   dev_dbg(vop->dev, "Warning: not support %s\n", reg_name);
return;
+   }

where the check for !reg happens after it got already dereferenced.
But even with that fixed I end up with

on rk3288:
[7.254823] rockchip-vop ff93.vop: Warning: not support global_regdone_en
[7.262847] rockchip-vop ff93.vop: Warning: not support gate
[7.269580] rockchip-vop ff93.vop: Warning: not support gate
[7.302765] rockchip-vop ff94.vop: Warning: not support global_regdone_en
[7.310758] rockchip-vop ff94.vop: Warning: not support gate
[7.317475] rockchip-vop ff94.vop: Warning: not support gate
[7.425724] rockchip-vop ff93.vop: Warning: not support edp_pin_pol
[7.526298] rockchip-vop ff94.vop: Warning: not support hdmi_pin_pol


Rk3288 does not support independent pin_pol settings, all output interfaces 
share the pin_pol register,
So not support hdmi_pin_pol here is correct.

hdmi pin pol would be set by following register:
.pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4),



on rk3036:
[   12.389138] rockchip-vop 10118000.vop: Warning: not support global_regdone_en
[   12.397324] rockchip-vop 10118000.vop: Warning: not support gate
[   12.404165] rockchip-vop 10118000.vop: Warning: not support gate
[   13.747361] rockchip-vop 10118000.vop: Warning: not support hdmi_pin_pol
[   13.747371] rockchip-vop 10118000.vop: Warning: not support hdmi_en
[   13.747379] rockchip-vop 10118000.vop: Warning: not support hpost_st_end
[   13.747385] rockchip-vop 10118000.vop: Warning: not support vpost_st_end
[   13.747461] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.767098] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.786060] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl

While reqdone and friends are obviously features of newer vops, at least
the hdmi pin-pol is available on both these socs.

With this patch applied (and null-ptr fixed) I end up without hdmi output
on both socs.


Hmmm, I am confused, from code review, I didn't see what will cause hdmi not 
work on rk3036 and rk3288,

Give me some time, I try to bringup my popmetal rk3288 board to do the test.

Heiko, Thanks very much for your test.




Heiko

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip






--
Mark Yao


___
dri-devel mailing 

Re: [PATCH v6 4/7] drm/rockchip: vop: group vop registers

2017-07-27 Thread Heiko Stübner
Am Donnerstag, 27. Juli 2017, 11:51:06 CEST schrieb Heiko Stübner:
> Hi Mark,
> 
> Am Mittwoch, 26. Juli 2017, 14:19:25 CEST schrieb Mark Yao:
> > Grouping the vop registers facilitates make register
> > definition clearer, and also is useful for different vop
> > reuse the same group register.
> > 
> > Signed-off-by: Mark Yao 
> > ---
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  99
> >  
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  60 ---
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 112
> > 
> > +++- 3 files changed, 144 insertions(+), 127
> > deletions(-)
> 
> This breaks display support on both rk3036 and rk3288 and I end up
> with a null pointer dereference in
> 
> [   10.640297] Unable to handle kernel NULL pointer dereference at virtual
> address  [   10.654430] pgd = c0204000
> [   10.657452] [] *pgd=
> [   10.661473] Internal error: Oops: 5 [#1] SMP ARM
> [   10.35] Modules linked in: snd_pcm media snd_timer phy_rockchip_dp
> snd soundcore rockchipdrm dw_hdmi analogix_dp rtc_rk808 pwm_rockchip
> clk_rk808 spi_rockchip [   10.682897] CPU: 2 PID: 143 Comm: kworker/2:2 Not
> tainted 4.13.0-rc2-01791-g2b86603d0515 #355 [   10.692430] Hardware name:
> Rockchip (Device Tree)
> [   10.697692] Workqueue: events deferred_probe_work_func
> [   10.702152] Linux video capture interface: v2.00
> [   10.708590] task: ee38c800 task.stack: ed2e6000
> [   10.713656] PC is at vop_reg_set.constprop.4+0x4/0xa8 [rockchipdrm]
> [   10.720668] LR is at vop_bind+0x568/0x8a0 [rockchipdrm]

The obvious reason for that is

> @@ -164,14 +153,20 @@ static inline uint32_t vop_read_reg(struct vop *vop,
> uint32_t base, return (vop_readl(vop, base + reg->offset) >> reg->shift) &
> reg->mask; }
> 
> -static inline void vop_mask_write(struct vop *vop, uint32_t offset,
> -   uint32_t mask, uint32_t shift, uint32_t v,
> -   bool write_mask, bool relaxed)
> +static void vop_reg_set(struct vop *vop, const struct vop_reg *reg,
> + uint32_t _offset, uint32_t _mask, uint32_t v,
> + const char *reg_name)
>  {
> - if (!mask)
> + int offset = reg->offset + _offset;
> + int mask = reg->mask & _mask;
> + int shift = reg->shift;
> +
> + if (!reg || !reg->mask) {
> + dev_dbg(vop->dev, "Warning: not support %s\n", reg_name);
>   return;
> + }

where the check for !reg happens after it got already dereferenced.
But even with that fixed I end up with

on rk3288:
[7.254823] rockchip-vop ff93.vop: Warning: not support global_regdone_en
[7.262847] rockchip-vop ff93.vop: Warning: not support gate
[7.269580] rockchip-vop ff93.vop: Warning: not support gate
[7.302765] rockchip-vop ff94.vop: Warning: not support global_regdone_en
[7.310758] rockchip-vop ff94.vop: Warning: not support gate
[7.317475] rockchip-vop ff94.vop: Warning: not support gate
[7.425724] rockchip-vop ff93.vop: Warning: not support edp_pin_pol
[7.526298] rockchip-vop ff94.vop: Warning: not support hdmi_pin_pol

on rk3036:
[   12.389138] rockchip-vop 10118000.vop: Warning: not support global_regdone_en
[   12.397324] rockchip-vop 10118000.vop: Warning: not support gate
[   12.404165] rockchip-vop 10118000.vop: Warning: not support gate
[   13.747361] rockchip-vop 10118000.vop: Warning: not support hdmi_pin_pol
[   13.747371] rockchip-vop 10118000.vop: Warning: not support hdmi_en
[   13.747379] rockchip-vop 10118000.vop: Warning: not support hpost_st_end
[   13.747385] rockchip-vop 10118000.vop: Warning: not support vpost_st_end
[   13.747461] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.767098] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl
[   13.786060] rockchip-vop 10118000.vop: Warning: not support src_alpha_ctl

While reqdone and friends are obviously features of newer vops, at least
the hdmi pin-pol is available on both these socs.

With this patch applied (and null-ptr fixed) I end up without hdmi output
on both socs.


Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v6 4/7] drm/rockchip: vop: group vop registers

2017-07-27 Thread Heiko Stübner
Hi Mark,

Am Mittwoch, 26. Juli 2017, 14:19:25 CEST schrieb Mark Yao:
> Grouping the vop registers facilitates make register
> definition clearer, and also is useful for different vop
> reuse the same group register.
> 
> Signed-off-by: Mark Yao 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  99 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  60 ---
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 112
> +++- 3 files changed, 144 insertions(+), 127
> deletions(-)

This breaks display support on both rk3036 and rk3288 and I end up
with a null pointer dereference in

[   10.640297] Unable to handle kernel NULL pointer dereference at virtual 
address 
[   10.654430] pgd = c0204000
[   10.657452] [] *pgd=
[   10.661473] Internal error: Oops: 5 [#1] SMP ARM
[   10.35] Modules linked in: snd_pcm media snd_timer phy_rockchip_dp snd 
soundcore rockchipdrm dw_hdmi analogix_dp rtc_rk808 pwm_rockchip clk_rk808 
spi_rockchip
[   10.682897] CPU: 2 PID: 143 Comm: kworker/2:2 Not tainted 
4.13.0-rc2-01791-g2b86603d0515 #355
[   10.692430] Hardware name: Rockchip (Device Tree)
[   10.697692] Workqueue: events deferred_probe_work_func
[   10.702152] Linux video capture interface: v2.00
[   10.708590] task: ee38c800 task.stack: ed2e6000
[   10.713656] PC is at vop_reg_set.constprop.4+0x4/0xa8 [rockchipdrm]
[   10.720668] LR is at vop_bind+0x568/0x8a0 [rockchipdrm]
[   10.726507] pc : []lr : []psr: 40010013
[   10.733514] sp : ed2e7d68  ip : 0004  fp : bf054988
[   10.739350] r10: bf054988  r9 :   r8 : 0001
[   10.745189] r7 : ed66f500  r6 : ee29da10  r5 :   r4 : ed22e010
[   10.752487] r3 :   r2 :   r1 :   r0 : ed22e010
[   10.759785] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   10.767763] Control: 10c5387d  Table: 2d4e806a  DAC: 0051
[   10.774188] Process kworker/2:2 (pid: 143, stack limit = 0xed2e6220)
[...]
[   11.058818] [] (vop_reg_set.constprop.4 [rockchipdrm]) from 
[] (vop_bind+0x568/0x8a0 [rockchipdrm])
[   11.058828] [] (vop_bind [rockchipdrm]) from [] 
(component_bind_all+0x11c/0x23c)
[   11.058836] [] (component_bind_all) from [] 
(rockchip_drm_bind+0x90/0x1d4 [rockchipdrm])
[   11.058843] [] (rockchip_drm_bind [rockchipdrm]) from [] 
(try_to_bring_up_master+0x148/0x184)
[   11.058847] [] (try_to_bring_up_master) from [] 
(component_add+0x98/0x144)
[   11.058853] [] (component_add) from [] 
(rockchip_dp_probe+0x7c/0x8c [rockchipdrm])
[   11.058860] [] (rockchip_dp_probe [rockchipdrm]) from [] 
(platform_drv_probe+0x50/0xb0)
[   11.058865] [] (platform_drv_probe) from [] 
(driver_probe_device+0x230/0x2e4)
[   11.058869] [] (driver_probe_device) from [] 
(bus_for_each_drv+0x60/0x94)
[   11.058873] [] (bus_for_each_drv) from [] 
(__device_attach+0xb0/0x114)
[   11.058876] [] (__device_attach) from [] 
(bus_probe_device+0x84/0x8c)
[   11.058879] [] (bus_probe_device) from [] 
(deferred_probe_work_func+0x68/0x94)
[   11.058884] [] (deferred_probe_work_func) from [] 
(process_one_work+0x200/0x504)
[   11.058889] [] (process_one_work) from [] 
(worker_thread+0x38/0x594)
[   11.058894] [] (worker_thread) from [] 
(kthread+0x128/0x158)
[   11.058900] [] (kthread) from [] 
(ret_from_fork+0x14/0x3c)
[   11.058904] Code: eae0 e3a03004 eaef e92d4070 (e5914000)
[   11.058930] ---[ end trace 9caa88bbcb1af5e4 ]---

I'll try to investigate a bit more, but maybe you'll be able to
find the issue faster than me in the meantime.


Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v6 4/7] drm/rockchip: vop: group vop registers

2017-07-26 Thread Mark Yao
Grouping the vop registers facilitates make register
definition clearer, and also is useful for different vop
reuse the same group register.

Signed-off-by: Mark Yao 
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  99 
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  60 ---
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 112 +++-
 3 files changed, 144 insertions(+), 127 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fd47da5..92d098b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -42,30 +42,19 @@
 #include "rockchip_drm_psr.h"
 #include "rockchip_drm_vop.h"
 
-#define REG_SET(x, base, reg, v) \
-   vop_mask_write(x, base + reg.offset, reg.mask, reg.shift, \
-  v, reg.write_mask, reg.relaxed)
-#define REG_SET_MASK(x, base, reg, mask, v) \
-   vop_mask_write(x, base + reg.offset, \
-  mask, reg.shift, v, reg.write_mask, reg.relaxed)
-
 #define VOP_WIN_SET(x, win, name, v) \
-   REG_SET(x, win->base, win->phy->name, v)
+   vop_reg_set(vop, >phy->name, win->base, ~0, v, #name)
 #define VOP_SCL_SET(x, win, name, v) \
-   REG_SET(x, win->base, win->phy->scl->name, v)
+   vop_reg_set(vop, >phy->scl->name, win->base, ~0, v, #name)
 #define VOP_SCL_SET_EXT(x, win, name, v) \
-   REG_SET(x, win->base, win->phy->scl->ext->name, v)
-#define VOP_CTRL_SET(x, name, v) \
-   REG_SET(x, 0, (x)->data->ctrl->name, v)
-
-#define VOP_INTR_GET(vop, name) \
-   vop_read_reg(vop, 0, >data->ctrl->name)
-
-#define VOP_INTR_SET(vop, name, v) \
-   REG_SET(vop, 0, vop->data->intr->name, v)
+   vop_reg_set(vop, >phy->scl->ext->name, \
+   win->base, ~0, v, #name)
 
 #define VOP_INTR_SET_MASK(vop, name, mask, v) \
-   REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v)
+   vop_reg_set(vop, >data->intr->name, 0, mask, v, #name)
+
+#define VOP_REG_SET(vop, group, name, v) \
+   vop_reg_set(vop, >data->group->name, 0, ~0, v, #name)
 
 #define VOP_INTR_SET_TYPE(vop, name, type, v) \
do { \
@@ -82,7 +71,7 @@
vop_get_intr_type(vop, >data->intr->name, type)
 
 #define VOP_WIN_GET(x, win, name) \
-   vop_read_reg(x, win->base, >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)
@@ -164,14 +153,20 @@ static inline uint32_t vop_read_reg(struct vop *vop, 
uint32_t base,
return (vop_readl(vop, base + reg->offset) >> reg->shift) & reg->mask;
 }
 
-static inline void vop_mask_write(struct vop *vop, uint32_t offset,
- uint32_t mask, uint32_t shift, uint32_t v,
- bool write_mask, bool relaxed)
+static void vop_reg_set(struct vop *vop, const struct vop_reg *reg,
+   uint32_t _offset, uint32_t _mask, uint32_t v,
+   const char *reg_name)
 {
-   if (!mask)
+   int offset = reg->offset + _offset;
+   int mask = reg->mask & _mask;
+   int shift = reg->shift;
+
+   if (!reg || !reg->mask) {
+   dev_dbg(vop->dev, "Warning: not support %s\n", reg_name);
return;
+   }
 
-   if (write_mask) {
+   if (reg->write_mask) {
v = ((v << shift) & 0x) | (mask << (shift + 16));
} else {
uint32_t cached_val = vop->regsbak[offset >> 2];
@@ -180,7 +175,7 @@ static inline void vop_mask_write(struct vop *vop, uint32_t 
offset,
vop->regsbak[offset >> 2] = v;
}
 
-   if (relaxed)
+   if (reg->relaxed)
writel_relaxed(v, vop->regs + offset);
else
writel(v, vop->regs + offset);
@@ -202,7 +197,7 @@ static inline uint32_t vop_get_intr_type(struct vop *vop,
 
 static inline void vop_cfg_done(struct vop *vop)
 {
-   VOP_CTRL_SET(vop, cfg_done, 1);
+   VOP_REG_SET(vop, common, cfg_done, 1);
 }
 
 static bool has_rb_swapped(uint32_t format)
@@ -540,7 +535,7 @@ static int vop_enable(struct drm_crtc *crtc)
 
spin_lock(>reg_lock);
 
-   VOP_CTRL_SET(vop, standby, 0);
+   VOP_REG_SET(vop, common, standby, 1);
 
spin_unlock(>reg_lock);
 
@@ -600,7 +595,7 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
 
spin_lock(>reg_lock);
 
-   VOP_CTRL_SET(vop, standby, 1);
+   VOP_REG_SET(vop, common, standby, 1);
 
spin_unlock(>reg_lock);
 
@@ -923,7 +918,7 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
 
spin_lock(>reg_lock);
 
-   VOP_CTRL_SET(vop, standby, 1);
+