[Intel-gfx] [PATCH v2] drm: Add high-precision time to vblank trace event

[Heinrich Fink]

On Tue, 3 Sep 2019 at 11:53, Daniel Vetter <daniel at ffwll.ch> wrote:
>
> On Tue, Sep 03, 2019 at 11:19:19AM +0200, Heinrich Fink wrote:
> > On Tue, 3 Sep 2019 at 09:46, Daniel Vetter <daniel at ffwll.ch> wrote:
> > >
> > > On Mon, Sep 02, 2019 at 04:24:12PM +0200, Heinrich Fink wrote:
> > > > Store the timestamp of the current vblank in the new field 'time' of the
> > > > vblank trace event. If the timestamp is calculated by a driver that
> > > > supports high-precision vblank timing, set the field 'high-prec' to
> > > > 'true'.
> > > >
> > > > User space can now access actual hardware vblank times via the tracing
> > > > infrastructure. Tracing applications (such as GPUVis, see [0] for
> > > > related discussion), can use the newly added information to conduct a
> > > > more accurate analysis of display timing.
> > > >
> > > > v2 Fix author name (missing last name)
> > > >
> > > > [0] https://github.com/mikesart/gpuvis/issues/30
> > > >
> > > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > Signed-off-by: Heinrich Fink <heinrich.fink at daqri.com>
> > >
> > > Applied, thanks.
> >
> > thanks! One question: After sending v2, I got an email from patchwork
> > pointing at some failed style checks (CHECK:PARENTHESIS_ALIGNMENT,
> > CHECK:COMPARISON_TO_NULL). Just so I know for the future, are these
> > checks mandatory to be addressed? I haven't had a chance to address
> > them yet. FWIW, linux-tree/scripts/checkpatch.pl did not complain.
>
> It's the same script, but I think CI uses some different options/flags. I
> generally ignore these, but also generally good to stick to the style.
>
> $ dim checkpatch
>
> in our maintainer-tools should give you the drm flavoured checkpatch.
> -Daniel

Apologies if that's a basic question, but at which point is this patch
landing upstream? I am monitoring the 5.4 merge window and couldn't
figure out what the stages are for this patch to get there. Is there
anything that is still left to do from my side?

Thanks, Heinrich

>
> >
> > - Heinrich
> >
> > > -Daniel
> > >
> > > > ---
> > > >  drivers/gpu/drm/drm_trace.h  | 14 ++++++++++----
> > > >  drivers/gpu/drm/drm_vblank.c |  3 ++-
> > > >  2 files changed, 12 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
> > > > index 471eb927474b..11c6dd577e8e 100644
> > > > --- a/drivers/gpu/drm/drm_trace.h
> > > > +++ b/drivers/gpu/drm/drm_trace.h
> > > > @@ -13,17 +13,23 @@ struct drm_file;
> > > >  #define TRACE_INCLUDE_FILE drm_trace
> > > >
> > > >  TRACE_EVENT(drm_vblank_event,
> > > > -         TP_PROTO(int crtc, unsigned int seq),
> > > > -         TP_ARGS(crtc, seq),
> > > > +         TP_PROTO(int crtc, unsigned int seq, ktime_t time, bool high_prec),
> > > > +         TP_ARGS(crtc, seq, time, high_prec),
> > > >           TP_STRUCT__entry(
> > > >                   __field(int, crtc)
> > > >                   __field(unsigned int, seq)
> > > > +                 __field(ktime_t, time)
> > > > +                 __field(bool, high_prec)
> > > >                   ),
> > > >           TP_fast_assign(
> > > >                   __entry->crtc = crtc;
> > > >                   __entry->seq = seq;
> > > > -                 ),
> > > > -         TP_printk("crtc=%d, seq=%u", __entry->crtc, __entry->seq)
> > > > +                 __entry->time = time;
> > > > +                 __entry->high_prec = high_prec;
> > > > +                     ),
> > > > +         TP_printk("crtc=%d, seq=%u, time=%lld, high-prec=%s",
> > > > +                     __entry->crtc, __entry->seq, __entry->time,
> > > > +                     __entry->high_prec ? "true" : "false")
> > > >  );
> > > >
> > > >  TRACE_EVENT(drm_vblank_event_queued,
> > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> > > > index fd1fbc77871f..c99feda25dea 100644
> > > > --- a/drivers/gpu/drm/drm_vblank.c
> > > > +++ b/drivers/gpu/drm/drm_vblank.c
> > > > @@ -1731,7 +1731,8 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
> > > >               send_vblank_event(dev, e, seq, now);
> > > >       }
> > > >
> > > > -     trace_drm_vblank_event(pipe, seq);
> > > > +     trace_drm_vblank_event(pipe, seq, now,
> > > > +                     dev->driver->get_vblank_timestamp != NULL);
> > > >  }
> > > >
> > > >  /**
> > > > --
> > > > 2.23.0.rc1
> > > >
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch