On Thu, Nov 19, 2020 at 2:26 AM <veeras@xxxxxxxxxxxxxx> wrote: > > On 2020-11-13 12:45, Daniel Vetter wrote: > > On Thu, Nov 12, 2020 at 10:27:23AM -0800, Veera Sundaram Sankaran > > 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 uses the > >> retire-fences > >> as an alternative to vblank when frame-updates are in progress Set the > >> fence timestamp during send vblank event to avoid discrepancies. > > > > I think a reference to the exact source code in android that does this > > would be really useful. Something in drm_hwcomposer or whatever is > > doing > > this. > > > > Thanks for the review. Sorry for not getting back earlier, was waiting > for the review on [patch 1/2], so that both comments can be addressed > together. > Here is the reference for Android expecting retire-fence timestamp to > match exactly with hardware vsync as it is used for the dispsync model. > > Usage: https://source.android.com/devices/graphics/implement-vsync > Code: > https://android.googlesource.com/platform/frameworks/native/+/master/services/surfaceflinger/Scheduler/Scheduler.cpp#397 > Will update the commit-text with the links as part of V2 patch. Yeah sounds good. Might be good to mention that Android requires this in the code comment too, since it's potentially confusing. -Daniel > Thanks, > Veera > > > Aside from documenting why we want to do this I think this all looks > > reasonable. > > -Daniel > > > >> > >> Signed-off-by: Veera Sundaram Sankaran <veeras@xxxxxxxxxxxxxx> > >> --- > >> drivers/gpu/drm/drm_vblank.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_vblank.c > >> b/drivers/gpu/drm/drm_vblank.c > >> index b18e1ef..b38e50c 100644 > >> --- a/drivers/gpu/drm/drm_vblank.c > >> +++ b/drivers/gpu/drm/drm_vblank.c > >> @@ -24,6 +24,7 @@ > >> * OTHER DEALINGS IN THE SOFTWARE. > >> */ > >> > >> +#include <linux/dma-fence.h> > >> #include <linux/export.h> > >> #include <linux/kthread.h> > >> #include <linux/moduleparam.h> > >> @@ -999,6 +1000,14 @@ static void send_vblank_event(struct drm_device > >> *dev, > >> e->event.seq.time_ns = ktime_to_ns(now); > >> break; > >> } > >> + > >> + /* > >> + * update fence timestamp with the same vblank timestamp as both > >> + * are signaled by the same event > >> + */ > >> + if (e->base.fence) > >> + e->base.fence->timestamp = now; > >> + > >> trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe, seq); > >> drm_send_event_locked(dev, &e->base); > >> } > >> -- > >> 2.7.4 > >> -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
