On Dienstag, 21. Dezember 2021 14:44:39 CET Nicolas Frattaroli wrote:

On Montag, 20. Dezember 2021 12:06:30 CET Sascha Hauer wrote:

quoted

From: Andy Yan <andy.yan@rock-chips.com>

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@pengutronix.de>
---

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




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel