| Message ID | 1501049966-6070-1-git-send-email-mark.yao@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi mark, On 07/26/2017 02:19 PM, Mark yao wrote: > 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<mark.yao@rock-chips.com> Reviewed-by: Jeffy Chen <jeffy.chen@rock-chips.com>
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 <mark.yao@rock-chips.com> > --- > 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 00000000 [ 10.654430] pgd = c0204000 [ 10.657452] [00000000] *pgd=00000000 [ 10.661473] Internal error: Oops: 5 [#1] SMP ARM [ 10.666635] 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 : [<bf04db28>] lr : [<bf04e134>] psr: 40010013 [ 10.733514] sp : ed2e7d68 ip : 00000004 fp : bf054988 [ 10.739350] r10: bf054988 r9 : 00000000 r8 : 00000001 [ 10.745189] r7 : ed66f500 r6 : ee29da10 r5 : 00000000 r4 : ed22e010 [ 10.752487] r3 : ffffffff r2 : 00000000 r1 : 00000000 r0 : ed22e010 [ 10.759785] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 10.767763] Control: 10c5387d Table: 2d4e806a DAC: 00000051 [ 10.774188] Process kworker/2:2 (pid: 143, stack limit = 0xed2e6220) [...] [ 11.058818] [<bf04db28>] (vop_reg_set.constprop.4 [rockchipdrm]) from [<bf04e134>] (vop_bind+0x568/0x8a0 [rockchipdrm]) [ 11.058828] [<bf04e134>] (vop_bind [rockchipdrm]) from [<c0870400>] (component_bind_all+0x11c/0x23c) [ 11.058836] [<c0870400>] (component_bind_all) from [<bf04c1cc>] (rockchip_drm_bind+0x90/0x1d4 [rockchipdrm]) [ 11.058843] [<bf04c1cc>] (rockchip_drm_bind [rockchipdrm]) from [<c0870854>] (try_to_bring_up_master+0x148/0x184) [ 11.058847] [<c0870854>] (try_to_bring_up_master) from [<c0870928>] (component_add+0x98/0x144) [ 11.058853] [<c0870928>] (component_add) from [<bf050d90>] (rockchip_dp_probe+0x7c/0x8c [rockchipdrm]) [ 11.058860] [<bf050d90>] (rockchip_dp_probe [rockchipdrm]) from [<c0877660>] (platform_drv_probe+0x50/0xb0) [ 11.058865] [<c0877660>] (platform_drv_probe) from [<c0875b48>] (driver_probe_device+0x230/0x2e4) [ 11.058869] [<c0875b48>] (driver_probe_device) from [<c0874208>] (bus_for_each_drv+0x60/0x94) [ 11.058873] [<c0874208>] (bus_for_each_drv) from [<c0875838>] (__device_attach+0xb0/0x114) [ 11.058876] [<c0875838>] (__device_attach) from [<c0874ec8>] (bus_probe_device+0x84/0x8c) [ 11.058879] [<c0874ec8>] (bus_probe_device) from [<c087534c>] (deferred_probe_work_func+0x68/0x94) [ 11.058884] [<c087534c>] (deferred_probe_work_func) from [<c035a884>] (process_one_work+0x200/0x504) [ 11.058889] [<c035a884>] (process_one_work) from [<c035b610>] (worker_thread+0x38/0x594) [ 11.058894] [<c035b610>] (worker_thread) from [<c036045c>] (kthread+0x128/0x158) [ 11.058900] [<c036045c>] (kthread) from [<c0307d18>] (ret_from_fork+0x14/0x3c) [ 11.058904] Code: eaffffe0 e3a03004 eaffffef 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
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 <mark.yao@rock-chips.com> > > --- > > > > 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 00000000 [ 10.654430] pgd = c0204000 > [ 10.657452] [00000000] *pgd=00000000 > [ 10.661473] Internal error: Oops: 5 [#1] SMP ARM > [ 10.666635] 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 ff930000.vop: Warning: not support global_regdone_en [ 7.262847] rockchip-vop ff930000.vop: Warning: not support gate [ 7.269580] rockchip-vop ff930000.vop: Warning: not support gate [ 7.302765] rockchip-vop ff940000.vop: Warning: not support global_regdone_en [ 7.310758] rockchip-vop ff940000.vop: Warning: not support gate [ 7.317475] rockchip-vop ff940000.vop: Warning: not support gate [ 7.425724] rockchip-vop ff930000.vop: Warning: not support edp_pin_pol [ 7.526298] rockchip-vop ff940000.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
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 <mark.yao@rock-chips.com> >>> --- >>> >>> 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 00000000 [ 10.654430] pgd = c0204000 >> [ 10.657452] [00000000] *pgd=00000000 >> [ 10.661473] Internal error: Oops: 5 [#1] SMP ARM >> [ 10.666635] 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 ff930000.vop: Warning: not support global_regdone_en > [ 7.262847] rockchip-vop ff930000.vop: Warning: not support gate > [ 7.269580] rockchip-vop ff930000.vop: Warning: not support gate > [ 7.302765] rockchip-vop ff940000.vop: Warning: not support global_regdone_en > [ 7.310758] rockchip-vop ff940000.vop: Warning: not support gate > [ 7.317475] rockchip-vop ff940000.vop: Warning: not support gate > [ 7.425724] rockchip-vop ff930000.vop: Warning: not support edp_pin_pol > [ 7.526298] rockchip-vop ff940000.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-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > >
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 <mark.yao@rock-chips.com> >>>> --- >>>> >>>> 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 00000000 [ 10.654430] pgd = c0204000 >>> [ 10.657452] [00000000] *pgd=00000000 >>> [ 10.661473] Internal error: Oops: 5 [#1] SMP ARM >>> [ 10.666635] 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 ff930000.vop: Warning: not support global_regdone_en >> [ 7.262847] rockchip-vop ff930000.vop: Warning: not support gate >> [ 7.269580] rockchip-vop ff930000.vop: Warning: not support gate >> [ 7.302765] rockchip-vop ff940000.vop: Warning: not support global_regdone_en >> [ 7.310758] rockchip-vop ff940000.vop: Warning: not support gate >> [ 7.317475] rockchip-vop ff940000.vop: Warning: not support gate >> [ 7.425724] rockchip-vop ff930000.vop: Warning: not support edp_pin_pol >> [ 7.526298] rockchip-vop ff940000.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 After fix null pointer problem, On rk3288 popmetal board, hdmi works good on my test, Can you double check it? Thanks. > >> >> >> Heiko >> >> _______________________________________________ >> Linux-rockchip mailing list >> Linux-rockchip@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-rockchip >> >> >> > >
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, &win->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, &win->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, &vop->data->ctrl->name) - -#define VOP_INTR_SET(vop, name, v) \ - REG_SET(vop, 0, vop->data->intr->name, v) + vop_reg_set(vop, &win->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, &vop->data->intr->name, 0, mask, v, #name) + +#define VOP_REG_SET(vop, group, name, v) \ + vop_reg_set(vop, &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, &vop->data->intr->name, type) #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) @@ -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) & 0xffff) | (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(&vop->reg_lock); - VOP_CTRL_SET(vop, standby, 0); + VOP_REG_SET(vop, common, standby, 1); spin_unlock(&vop->reg_lock); @@ -600,7 +595,7 @@ static void vop_crtc_disable(struct drm_crtc *crtc) spin_lock(&vop->reg_lock); - VOP_CTRL_SET(vop, standby, 1); + VOP_REG_SET(vop, common, standby, 1); spin_unlock(&vop->reg_lock); @@ -923,7 +918,7 @@ static void vop_crtc_enable(struct drm_crtc *crtc) spin_lock(&vop->reg_lock); - VOP_CTRL_SET(vop, standby, 1); + VOP_REG_SET(vop, common, standby, 1); spin_unlock(&vop->reg_lock); @@ -937,29 +932,29 @@ static void vop_crtc_enable(struct drm_crtc *crtc) BIT(HSYNC_POSITIVE) : 0; pin_pol |= (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC) ? BIT(VSYNC_POSITIVE) : 0; - VOP_CTRL_SET(vop, pin_pol, pin_pol); + VOP_REG_SET(vop, output, pin_pol, pin_pol); switch (s->output_type) { case DRM_MODE_CONNECTOR_LVDS: - VOP_CTRL_SET(vop, rgb_en, 1); - VOP_CTRL_SET(vop, rgb_pin_pol, pin_pol); + VOP_REG_SET(vop, output, rgb_en, 1); + VOP_REG_SET(vop, output, rgb_pin_pol, pin_pol); break; case DRM_MODE_CONNECTOR_eDP: - VOP_CTRL_SET(vop, edp_pin_pol, pin_pol); - VOP_CTRL_SET(vop, edp_en, 1); + VOP_REG_SET(vop, output, edp_pin_pol, pin_pol); + VOP_REG_SET(vop, output, edp_en, 1); break; case DRM_MODE_CONNECTOR_HDMIA: - VOP_CTRL_SET(vop, hdmi_pin_pol, pin_pol); - VOP_CTRL_SET(vop, hdmi_en, 1); + VOP_REG_SET(vop, output, hdmi_pin_pol, pin_pol); + VOP_REG_SET(vop, output, hdmi_en, 1); break; case DRM_MODE_CONNECTOR_DSI: - VOP_CTRL_SET(vop, mipi_pin_pol, pin_pol); - VOP_CTRL_SET(vop, mipi_en, 1); + VOP_REG_SET(vop, output, mipi_pin_pol, pin_pol); + VOP_REG_SET(vop, output, mipi_en, 1); break; case DRM_MODE_CONNECTOR_DisplayPort: pin_pol &= ~BIT(DCLK_INVERT); - VOP_CTRL_SET(vop, dp_pin_pol, pin_pol); - VOP_CTRL_SET(vop, dp_en, 1); + VOP_REG_SET(vop, output, dp_pin_pol, pin_pol); + VOP_REG_SET(vop, output, dp_en, 1); break; default: DRM_DEV_ERROR(vop->dev, "unsupported connector_type [%d]\n", @@ -972,25 +967,25 @@ static void vop_crtc_enable(struct drm_crtc *crtc) if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && !(vop_data->feature & VOP_FEATURE_OUTPUT_RGB10)) s->output_mode = ROCKCHIP_OUT_MODE_P888; - VOP_CTRL_SET(vop, out_mode, s->output_mode); + VOP_REG_SET(vop, common, out_mode, s->output_mode); - VOP_CTRL_SET(vop, htotal_pw, (htotal << 16) | hsync_len); + VOP_REG_SET(vop, modeset, htotal_pw, (htotal << 16) | hsync_len); val = hact_st << 16; val |= hact_end; - VOP_CTRL_SET(vop, hact_st_end, val); - VOP_CTRL_SET(vop, hpost_st_end, val); + VOP_REG_SET(vop, modeset, hact_st_end, val); + VOP_REG_SET(vop, modeset, hpost_st_end, val); - VOP_CTRL_SET(vop, vtotal_pw, (vtotal << 16) | vsync_len); + VOP_REG_SET(vop, modeset, vtotal_pw, (vtotal << 16) | vsync_len); val = vact_st << 16; val |= vact_end; - VOP_CTRL_SET(vop, vact_st_end, val); - VOP_CTRL_SET(vop, vpost_st_end, val); + VOP_REG_SET(vop, modeset, vact_st_end, val); + VOP_REG_SET(vop, modeset, vpost_st_end, val); - VOP_INTR_SET(vop, line_flag_num[0], vact_end); + VOP_REG_SET(vop, intr, line_flag_num[0], vact_end); clk_set_rate(vop->dclk, adjusted_mode->clock * 1000); - VOP_CTRL_SET(vop, standby, 0); + VOP_REG_SET(vop, common, standby, 0); rockchip_drm_psr_activate(&vop->crtc); } @@ -1452,8 +1447,8 @@ static int vop_initial(struct vop *vop) memcpy(vop->regsbak, vop->regs, vop->len); - VOP_CTRL_SET(vop, global_regdone_en, 1); - VOP_CTRL_SET(vop, dsp_blank, 0); + VOP_REG_SET(vop, misc, global_regdone_en, 1); + VOP_REG_SET(vop, common, dsp_blank, 0); for (i = 0; i < vop_data->win_size; i++) { const struct vop_win_data *win = &vop_data->win[i]; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 850f8e4..3ba962c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -25,43 +25,50 @@ enum vop_data_format { }; struct vop_reg { - uint32_t offset; - uint32_t shift; uint32_t mask; + uint16_t offset; + uint8_t shift; bool write_mask; bool relaxed; }; -struct vop_ctrl { - struct vop_reg standby; - struct vop_reg data_blank; - struct vop_reg gate_en; - struct vop_reg mmu_en; - struct vop_reg rgb_en; +struct vop_modeset { + struct vop_reg htotal_pw; + struct vop_reg hact_st_end; + struct vop_reg hpost_st_end; + struct vop_reg vtotal_pw; + struct vop_reg vact_st_end; + struct vop_reg vpost_st_end; +}; + +struct vop_output { + struct vop_reg pin_pol; + struct vop_reg dp_pin_pol; + struct vop_reg edp_pin_pol; + struct vop_reg hdmi_pin_pol; + struct vop_reg mipi_pin_pol; + struct vop_reg rgb_pin_pol; + struct vop_reg dp_en; struct vop_reg edp_en; struct vop_reg hdmi_en; struct vop_reg mipi_en; - struct vop_reg dp_en; + struct vop_reg rgb_en; +}; + +struct vop_common { + struct vop_reg cfg_done; struct vop_reg dsp_blank; - struct vop_reg out_mode; + struct vop_reg data_blank; struct vop_reg dither_down; struct vop_reg dither_up; - struct vop_reg pin_pol; - struct vop_reg rgb_pin_pol; - struct vop_reg hdmi_pin_pol; - struct vop_reg edp_pin_pol; - struct vop_reg mipi_pin_pol; - struct vop_reg dp_pin_pol; - - struct vop_reg htotal_pw; - struct vop_reg hact_st_end; - struct vop_reg vtotal_pw; - struct vop_reg vact_st_end; - struct vop_reg hpost_st_end; - struct vop_reg vpost_st_end; + struct vop_reg gate_en; + struct vop_reg mmu_en; + struct vop_reg out_mode; + struct vop_reg standby; +}; +struct vop_misc { struct vop_reg global_regdone_en; - struct vop_reg cfg_done; }; struct vop_intr { @@ -135,8 +142,11 @@ struct vop_win_data { }; struct vop_data { - const struct vop_ctrl *ctrl; const struct vop_intr *intr; + const struct vop_common *common; + const struct vop_misc *misc; + const struct vop_modeset *modeset; + const struct vop_output *output; const struct vop_win_data *win; unsigned int win_size; diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index 0a5f0d2..20607a8 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -117,26 +117,34 @@ .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), + .status = VOP_REG_SYNC(RK3036_INT_STATUS, 0xf, 0), + .enable = VOP_REG_SYNC(RK3036_INT_STATUS, 0xf, 4), + .clear = VOP_REG_SYNC(RK3036_INT_STATUS, 0xf, 8), }; -static const struct vop_ctrl rk3036_ctrl_data = { - .standby = VOP_REG_SYNC(RK3036_SYS_CTRL, 0x1, 30), - .out_mode = VOP_REG(RK3036_DSP_CTRL0, 0xf, 0), - .pin_pol = VOP_REG(RK3036_DSP_CTRL0, 0xf, 4), - .dsp_blank = VOP_REG(RK3036_DSP_CTRL1, 0x1, 24), +static const struct vop_modeset rk3036_modeset = { .htotal_pw = VOP_REG(RK3036_DSP_HTOTAL_HS_END, 0x1fff1fff, 0), .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), +}; + +static const struct vop_output rk3036_output = { + .pin_pol = VOP_REG(RK3036_DSP_CTRL0, 0xf, 4), +}; + +static const struct vop_common rk3036_common = { + .standby = VOP_REG_SYNC(RK3036_SYS_CTRL, 0x1, 30), + .out_mode = VOP_REG(RK3036_DSP_CTRL0, 0xf, 0), + .dsp_blank = VOP_REG(RK3036_DSP_CTRL1, 0x1, 24), .cfg_done = VOP_REG_SYNC(RK3036_REG_CFG_DONE, 0x1, 0), }; static const struct vop_data rk3036_vop = { - .ctrl = &rk3036_ctrl_data, .intr = &rk3036_intr, + .common = &rk3036_common, + .modeset = &rk3036_modeset, + .output = &rk3036_output, .win = rk3036_vop_win_data, .win_size = ARRAY_SIZE(rk3036_vop_win_data), }; @@ -206,27 +214,32 @@ .dst_alpha_ctl = VOP_REG(RK3288_WIN2_DST_ALPHA_CTRL, 0xff, 0), }; -static const struct vop_ctrl rk3288_ctrl_data = { - .standby = VOP_REG_SYNC(RK3288_SYS_CTRL, 0x1, 22), - .gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23), - .mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20), +static const struct vop_modeset rk3288_modeset = { + .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), +}; + +static const struct vop_output rk3288_output = { + .pin_pol = VOP_REG(RK3288_DSP_CTRL0, 0xf, 4), .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), .mipi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 15), +}; + +static const struct vop_common rk3288_common = { + .standby = VOP_REG_SYNC(RK3288_SYS_CTRL, 0x1, 22), + .gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23), + .mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20), .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), - .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), .cfg_done = VOP_REG_SYNC(RK3288_REG_CFG_DONE, 0x1, 0), }; @@ -266,37 +279,13 @@ static const struct vop_data rk3288_vop = { .feature = VOP_FEATURE_OUTPUT_RGB10, .intr = &rk3288_vop_intr, - .ctrl = &rk3288_ctrl_data, + .common = &rk3288_common, + .modeset = &rk3288_modeset, + .output = &rk3288_output, .win = rk3288_vop_win_data, .win_size = ARRAY_SIZE(rk3288_vop_win_data), }; -static const struct vop_ctrl rk3399_ctrl_data = { - .standby = VOP_REG_SYNC(RK3399_SYS_CTRL, 0x1, 22), - .gate_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 23), - .dp_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 11), - .rgb_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 12), - .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_SYNC(RK3399_REG_CFG_DONE, 0x1, 0), -}; - static const int rk3399_vop_intrs[] = { FS_INTR, 0, 0, @@ -317,10 +306,30 @@ .clear = VOP_REG_MASK_SYNC(RK3399_INTR_CLEAR0, 0xffff, 0), }; +static const struct vop_output rk3399_output = { + .dp_pin_pol = VOP_REG(RK3399_DSP_CTRL1, 0xf, 16), + .rgb_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), + .dp_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 11), + .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), + .mipi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 15), +}; + +static const struct vop_misc rk3399_misc = { + .global_regdone_en = VOP_REG(RK3399_SYS_CTRL, 0x1, 11), +}; + static const struct vop_data rk3399_vop_big = { .feature = VOP_FEATURE_OUTPUT_RGB10, .intr = &rk3399_vop_intr, - .ctrl = &rk3399_ctrl_data, + .common = &rk3288_common, + .modeset = &rk3288_modeset, + .output = &rk3399_output, + .misc = &rk3399_misc, /* * rk3399 vop big windows register layout is same as rk3288. */ @@ -337,7 +346,10 @@ static const struct vop_data rk3399_vop_lit = { .intr = &rk3399_vop_intr, - .ctrl = &rk3399_ctrl_data, + .common = &rk3288_common, + .modeset = &rk3288_modeset, + .output = &rk3399_output, + .misc = &rk3399_misc, /* * rk3399 vop lit windows register layout is same as rk3288, * but cut off the win1 and win3 windows.
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 <mark.yao@rock-chips.com> --- 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(-)