On Thu, Jan 7, 2021 at 12:54 AM Veera Sundaram Sankaran <veeras@xxxxxxxxxxxxxx> wrote: > > The explicit out-fences in crtc are signaled as part of vblank event, > indicating all framebuffers present on the Atomic Commit request are > scanned out on the screen. Though the fence signal and the vblank event > notification happens at the same time, triggered by the same hardware > vsync event, the timestamp set in both are different. With drivers > supporting precise vblank timestamp the difference between the two > timestamps would be even higher. This might have an impact on use-mode > frameworks using these fence timestamps for purposes other than simple > buffer usage. For instance, the Android framework [1] uses the > retire-fences as an alternative to vblank when frame-updates are in > progress. Set the fence timestamp during send vblank event using a new > drm_send_event_timestamp_locked variant to avoid discrepancies. > > [1] https://android.googlesource.com/platform/frameworks/native/+/master/ > services/surfaceflinger/Scheduler/Scheduler.cpp#397 > > Changes in v2: > - Use drm_send_event_timestamp_locked to update fence timestamp > - add more information to commit text Thanks for sending this out! One small note: > @@ -775,6 +775,49 @@ void drm_event_cancel_free(struct drm_device *dev, > EXPORT_SYMBOL(drm_event_cancel_free); > > /** > + * drm_send_event_timestamp_locked - send DRM event to file descriptor > + * @dev: DRM device > + * @e: DRM event to deliver > + * @timestamp: timestamp to set for the fence event > + * > + * This function sends the event @e, initialized with drm_event_reserve_init(), > + * to its associated userspace DRM file. Callers must already hold > + * &drm_device.event_lock, see drm_send_event() for the unlocked version. > + * > + * Note that the core will take care of unlinking and disarming events when the > + * corresponding DRM file is closed. Drivers need not worry about whether the > + * DRM file for this event still exists and can call this function upon > + * completion of the asynchronous work unconditionally. > + */ > +void drm_send_event_timestamp_locked(struct drm_device *dev, > + struct drm_pending_event *e, ktime_t timestamp) > +{ > + assert_spin_locked(&dev->event_lock); > + > + if (e->completion) { > + complete_all(e->completion); > + e->completion_release(e->completion); > + e->completion = NULL; > + } > + > + if (e->fence) { > + dma_fence_signal_timestamp(e->fence, timestamp); > + dma_fence_put(e->fence); > + } > + > + if (!e->file_priv) { > + kfree(e); > + return; > + } > + > + list_del(&e->pending_link); > + list_add_tail(&e->link, > + &e->file_priv->event_list); > + wake_up_interruptible(&e->file_priv->event_wait); > +} > +EXPORT_SYMBOL(drm_send_event_timestamp_locked); This seems to duplicate much of drm_send_event_locked(). Should a common backend function be used between them? thanks -john
