[PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper

[Daniel Stone]

Hi,

On 22 October 2015 at 21:25, Daniel Vetter <daniel at ffwll.ch> wrote:
> Maarten Lankhorst had patches to do that for i915, but they just didn't go
> anywhere since i915 gem locking is a bit ... antique. But if your goal is
> to only fix up the page_flip path, and only for i915 as the display
> driver, then the only thing you need to do is:
> - grab the reservation of the gem bo underlying the fb
> - grab the exclusive fence (fence_get)
> - wait for that fence to complete in the async worker (we already have the
>   mmio_flip stuff for when you do the flip from the cpu)
> - drop the fence once done with fence_put
>
> Works as long as the producer correctly sets the exclusive fence (doesn't
> matter whether that's done with an explicit ioctl or at command
> submission). And that's already done by nouveau (well ttm) afaik.
>
> No changes at all in userspace required.

Totally agree. For implicit sync, this is also how we (and ChromeOS)
do it on ARM, e.g.:
https://git.collabora.com/cgit/user/daniels/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_fb.c?h=tmp/lfrb#n288

This was never upstreamed as the users (Exynos, Rockchip, et al)
depended on Mali, but it does offer a perfect implementation of
implicit synchronisation between multiple separate devices when you
place reservation objects.

> Now if you want to do proper explicit fencing for kms, that's an entirely
> different story. For that we want something like android's hw composer
> in/out fences, and obviously for atomic (because interface extensions for
> legacy modeset ioctls just don't make sense). But that means we need the
> full stack, using open-source userspace, and that's a lot more pain.

Totally agreed. New legacy API is pretty much a no-go - especially
given the painful semantics they force you to implement - and wouldn't
take any less time to roll out through the kernel and userspace,
including working on tests, than it would to wait for Maarten
Lankhorst's existing async-atomic work to land.

Cheers,
Daniel