[lkml]   [2018]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
On Fri, Apr 06, 2018 at 04:56:48PM -0700, Keith Packard wrote:
> (This is an RFC on whether this pair of ioctls seems reasonable. The
> code compiles, but I haven't tested it as I'm away from home this
> weekend.)
> I'm rewriting my implementation of the Vulkan EXT_display_control
> extension, which provides a way to signal a Vulkan fence at vblank
> time. I had implemented it using events, but that isn't great as the
> Vulkan API includes the ability to wait for any of a set of fences to
> be signaled. As the other Vulkan fences are implemented using
> dma_fences in the kernel, and (eventually) using syncobj up in user
> space, it seems reasonable to use syncobjs for everything and hook
> vblank to those.
> In any case, I'm proposing two new syncobj/vblank ioctls (the names
> aren't great; suggestions welcome, as usual):
> Create a new syncobj that will be signaled at (or after) the
> specified vblank sequence value. This uses the same parameters
> to specify the target sequence as

My understanding of drm_syncobj is that you:

- Create a syncobj with the drm_syncobj_create ioctl.

- Pass it around to various driver callbacks who update the attached
dma_fence pointer using drm_syncobj_replace_fence().

Yes amdgpu does this a bit differently, but that seems to be the

From that pov I'd massage the uapi to just extend
drm_crtc_queue_sequence_ioctl with a new syncobj flag. Syncobj we can just
add at the bottom of struct drm_crtc_queue_sequence (drm structs can be
extended like that, it's part of the uapi). Or we reuse user_data, but
that's a bit a hack.

We also don't need a new event type, vblank code simply singals
event->fence, which we'll arrange to be the fence behind the syncobj.

> Once the above syncobj has been signaled, this ioctl allows
> the application to find out when that happened, returning both
> the vblank sequence count and time (in ns).

The android syncpt stuff already had the concept of a fence timestamp, and
we carried it over as part of struct dma_fence.timestamp. It's just not
exposed yet as proper uapi. I think we should aim a bit more into that
direction with something like the below sketch:

- Add a dma_fence_signal_ts, which allows us to set the timestamp from a
hw clock.

- Use that in the vblank code.

- Add new drm_syncobj ioctl to query the timestamp of the attached fence
(if it's signalled).

That would entirely avoid the special-case ioctl just for vblank syncobj
here. Also, this might be useful in your implementation of
VK_GOOGLE_display_timing, since the current query timestamp you're using
won't take into account irq wakeup latency. Using fence->timestamp will
still miss the irq->atomic worker wakupe latency, but should be a lot
better already.

> I'd like to hear comments on whether this seems reasonable, or whether
> I should go in some other direction.

Besides a few bikesheds to align better with other stuff going around I
think this looks good.
Daniel Vetter
Software Engineer, Intel Corporation

 \ /
  Last update: 2018-04-09 11:15    [W:0.058 / U:32.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site