On Thu, Jan 7, 2021 at 12:54 AM Veera Sundaram Sankaran
<veeras at codeaurora.org> 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