lkml.org 
[lkml]   [2018]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    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):
    >
    > DRM_IOCTL_CRTC_QUEUE_SYNCOBJ
    >
    > 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
    > DRM_IOCTL_CRTC_QUEUE_SEQUENCE.

    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
    exception.

    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.

    > DRM_IOCTL_CRTC_GET_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
    http://blog.ffwll.ch

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