[PATCH] drm/amd/display: Add fast path for cursor plane updates

[Kazlauskas, Nicholas]

On 12/13/18 10:48 AM, Michel Dänzer wrote:
> On 2018-12-05 8:59 p.m., Nicholas Kazlauskas wrote:
>> [Why]
>> Legacy cursor plane updates from drm helpers go through the full
>> atomic codepath. A high volume of cursor updates through this slow
>> code path can cause subsequent page-flips to skip vblank intervals
>> since each individual update is slow.
>>
>> This problem is particularly noticeable for the compton compositor.
>>
>> [How]
>> A fast path for cursor plane updates is added by using DRM asynchronous
>> commit support provided by async_check and async_update. These don't do
>> a full state/flip_done dependency stall and they don't block other
>> commit work.
>>
>> However, DC still expects itself to be single-threaded for anything
>> that can issue register writes. Screen corruption or hangs can occur
>> if write sequences overlap. Every call that potentially perform
>> register writes needs to be guarded for asynchronous updates to work.
>> The dc_lock mutex was added for this.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106175
>>
>> Cc: Leo Li <sunpeng.li at amd.com>
>> Cc: Harry Wentland <harry.wentland at amd.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> 
> Looks like this change introduced (or at least exposed) a reference
> counting bug resulting in use-after-free when Xorg shuts down[0]. See
> the attached dmesg excerpt (note that I wrapped the !bo->pin_count check
> in amdgpu_bo_unpin in WARN_ON_ONCE).
> 
> 
> [0] Only with
> https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/commit/0d60233d26ec70d4e1faa343b438e33829c6d5e4
> , i.e. alternating between two BOs for the HW cursor, instead of always
> using the same one.
> 
> 

Thanks for the heads up, I don't think I had that patch in my 
xf86-video-amdgpu when testing the desktop stack.

The async atomic helper does the:

drm_atomic_helper_prepare_planes
drm_atomic_helper_async_commit
drm_atomic_helper_cleanup_planes

...sequence correctly from what I can tell, so maybe it's something with
dm_plane_helper_prepare_fb or dm_plane_helper_cleanup_fb itself.

One case where unref could be called (not following a ref) is during 
drm_atomic_helper_prepare_planes - if prepare_fb fails then cleanup_fb 
gets called regardless, and we only ref the fb if prepare_fb is in the 
success path.

Nicholas Kazlauskas