On Dienstag, 21. Dezember 2021 14:44:39 CET Nicolas Frattaroli wrote: > On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote: > > From: Andy Yan <andy.yan@xxxxxxxxxxxxxx> > > > > The VOP2 unit is found on Rockchip SoCs beginning with rk3566/rk3568. > > It replaces the VOP unit found in the older Rockchip SoCs. > > > > This driver has been derived from the downstream Rockchip Kernel and > > heavily modified: > > > > - All nonstandard DRM properties have been removed > > - dropped struct vop2_plane_state and pass around less data between > > functions > > - Dropped all DRM_FORMAT_* not known on upstream > > - rework register access to get rid of excessively used macros > > - Drop all waiting for framesyncs > > > > The driver is tested with HDMI and MIPI-DSI display on a RK3568-EVB > > board. Overlay support is tested with the modetest utility. AFBC support > > on the cluster windows is tested with weston-simple-dmabuf-egl on > > weston using the (yet to be upstreamed) panfrost driver support. > > > > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > > --- > > Hi Sascha, > > quick partial review of the code in-line. > > For reference, I debugged locking issues with the kernel lock > debug config options and assert_spin_locked in the reg write > functions, as well as some manual deduction. > As a small follow-up, I've completely mapped out the calls to vop2_writel, vop2_readl, vop2_vp_write and vop2_win_write and coloured in whether they were called with the lock held or not. The conclusion is startling: Most of the code absolutely does not care about the reg_lock. Here's the graph as an SVG: https://overviewer.org/~pillow/up/6800427ef3/vop2_callgraph_modified.svg vop2_isr here needs to be paid special attention, as it also acquires a different spinlock, and we want to avoid deadlocks. Perhaps we should precisely define which lock must be held for what registers, such that the vop2_isr can write its interrupt related registers without acquiring the "big" reg_lock. I'm also not entirely sure whether I should assume vop2_readl needs to be called with the lock held. This needs some investigating both in terms of whether the hardware presents a writel as an atomic write of a long, and whether the code assumes the state between readl calls is ever a consistent view. Regards, Nicolas Frattaroli
