Re: [PATCH RESEND v2 2/2] drm/drm_vblank: set the dma-fence timestamp during send_vblank_event

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux