On Mon, Apr 09, 2018 at 11:14:17AM +0200, Daniel Vetter wrote: > 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. Ok, I'm going to retract my entire suggestion above for the GET ioctl. It would neatly work for vk_google_display_timing, but the KHR version of that extensions very much sounds like we want the kernel to report 2 (or maybe even 3) different timestamps back. An ioctl is much easier to extend than shoe-horning all that into the dma_fence/drm_syncobj abstraction. One bikeshed maybe on top: Maybe call the ioctl CRTC_GET_TS_DATA, since that's what you're getting, not the syncobj. Sorry for flopping around on this, just learned this all in discussions past week. -Daniel > > > 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
