On Wed, Jan 13, 2021 at 11:52 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 > > Changes in v3: > - use same backend helper function for variants of drm_send_event to > avoid code duplications > > Signed-off-by: Veera Sundaram Sankaran <veeras@xxxxxxxxxxxxxx> > --- > drivers/gpu/drm/drm_file.c | 71 ++++++++++++++++++++++++++++++++++++-------- > drivers/gpu/drm/drm_vblank.c | 9 +++++- > include/drm/drm_file.h | 3 ++ > 3 files changed, 70 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 0ac4566..b8348ca 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -775,20 +775,19 @@ void drm_event_cancel_free(struct drm_device *dev, > EXPORT_SYMBOL(drm_event_cancel_free); > > /** > - * drm_send_event_locked - send DRM event to file descriptor > + * drm_send_event_helper - send DRM event to file descriptor > * @dev: DRM device > * @e: DRM event to deliver > + * @timestamp: timestamp to set for the fence event in kernel's CLOCK_MONOTONIC > + * time domain > * > - * 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. > + * This helper function sends the event @e, initialized with > + * drm_event_reserve_init(), to its associated userspace DRM file. > + * The timestamp variant of dma_fence_signal is used when the caller > + * sends a valid timestamp. > */ > -void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > +void drm_send_event_helper(struct drm_device *dev, > + struct drm_pending_event *e, ktime_t timestamp) > { > assert_spin_locked(&dev->event_lock); > > @@ -799,7 +798,10 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > } > > if (e->fence) { > - dma_fence_signal(e->fence); > + if (timestamp) > + dma_fence_signal_timestamp(e->fence, timestamp); > + else > + dma_fence_signal(e->fence); > dma_fence_put(e->fence); > } > > @@ -814,6 +816,51 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) > wake_up_interruptible_poll(&e->file_priv->event_wait, > EPOLLIN | EPOLLRDNORM); > } > + > +/** > + * 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 in kernel's CLOCK_MONOTONIC > + * time domain > + * > + * 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. > + * > + * 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) > +{ > + WARN_ON(!timestamp); > + > + drm_send_event_helper(dev, e, timestamp); > + > +} > +EXPORT_SYMBOL(drm_send_event_timestamp_locked); Hey Veera, So actually, after closer look at the testing I was doing, we seem to be hitting that WARN_ON right as the display first comes up (I see this on both db845c and HiKey960). It seems in drm_crtc_send_vblank_event(), if "now" is set by drm_vblank_count_and_time(), the first timestamp value we get from it seems to be 0. Let me know if you need any help reproducing and sorting this issue out. thanks -john _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
