drm/vc4: Correct buffer address on async commit when vertically flipped #7169
Conversation
The buffer address needs to be for the last line of the image if vertically flipped. The main commit code was handling this, but the async path wasn't. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
|
Need to test on a Pi5 before merging, hence draft. Ideally all the address fixups (eg source cropping) for the commit should apply to async path too, but that's pretty rare. |
|
|
||
| /* We must point to the last line when Y reflection is enabled. */ | ||
| if (rotation & DRM_MODE_REFLECT_Y) | ||
| dma_addr += fb->pitches[0] * ((vc4_state->src_h[0] >> 16) - 1); |
There was a problem hiding this comment.
My gut feeling is this should be rounded up.
If height is 1.9*(1<<16) without flipping, then I'd expect the HVS to fetch line 0, then 1.
With flipping I'd expect it to fetch line 1, then 0.
It feels like with the round down, it will be fetching line 0, then line -1.
Note: I suspect there are other instances of vc4_state->src_h[0] >> 16 being used as height in this file which need thought about rounding.
There was a problem hiding this comment.
It's matching the handling in the non-async path at https://github.com/raspberrypi/linux/blob/rpi-6.12.y/drivers/gpu/drm/vc4/vc4_plane.c#L1387-L1389.
Ideally the common bits should be extracted into a helper rather than duplicated, but I didn't fancy doing that right now.
Kivy is triggering this path due to waiting on the v3d buffer fence triggering vc4_async_page_flip_fence_complete / vc4_async_page_flip_complete. It looks like they just call DRM_IOCTL_MODE_PAGE_FLIP to trigger drm_atomic_set_fb_for_plane.
AIUI this is the legacy async flip hook, and applications should really be using the atomic async commits which would go through vc4_plane_atomic_async_[check|update] which would use vc4_plane_mode_set to generate a complete dlist using the normal path. I'm really loathed to expend too much energy on fixing up the legacy path.
|
Confirmed to work on Pi5 too (it was also broken before). |
popcornmix
left a comment
popcornmix
left a comment
There was a problem hiding this comment.
I think the rounding is wrong, but that can be dealt with in a subsequent PR that also handles the non-async path.
2 participants
The buffer address needs to be for the last line of the image if vertically flipped. The main commit code was handling this, but the async path wasn't.
raspberrypi/firmware#1851