[PATCH weston v2 4/4] gl-renderer: Emit GPU rendering begin and end timeline timepoints

[Alexandros Frantzis]

On Mon, Sep 25, 2017 at 05:40:36PM +0300, Pekka Paalanen wrote:
> On Tue, 19 Sep 2017 14:59:11 +0300
> Alexandros Frantzis <alexandros.frantzis at collabora.com> wrote:
> 
> > Use EGL fence sync objects to emit timepoints for the beginning and the
> > end of rendering on the GPU. The timepoints are emitted asynchronously
> > using the sync file fds associated with the fence sync objects. The sync
> > file fds are acquired using the facilities provided by the
> > EGL_ANDROID_native_fence_sync extension.
> > 
> > The asynchronous timepoint submissions are stored in a list in
> > gl_output_state until they are executed, and any pending submissions
> > that remain at output destruction time are cleaned up.
> > 
> > If timelining is inactive or the required EGL extensions are not
> > present, then GPU timepoint processing and emission are skipped.
> > 
> > Note that the GPU timestamps returned by sync files are in the
> > CLOCK_MONOTONIC clock domain, and are thus compatible with the
> > timeline timestamps (which also are in the CLOCK_MONOTONIC domain).
> > 
> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> > ---
> > 
> > Changes in v2:
> >  - Added Signed-off-by.
> >  - Used fallback sync_file.h header if required.
> >  - Improved error checking.
> >  - Used enum timeline_render_point_type instead of int.
> >  - Stored pending timeline submissions in a list in gl_output_state.
> >  - Renamed various struct and function to improve consistency and clarity.
> >  - Added fallbacks for newly used EGL definitions.
> >  - Added log messages for render gpu related warnings.
> > 
> >  libweston/gl-renderer.c | 162 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  shared/weston-egl-ext.h |  12 ++++
> >  2 files changed, 174 insertions(+)
> 
> Hi,
> 
> patches 1, 2 and 4 are:
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Hi Pekka,

<snip>
> > +
> > +	enum timeline_render_point_type type;
> > +	int fd;
> > +	struct weston_output *output;
> > +	struct wl_event_source *event_source;
> > +};
> > +
> >  static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
> >  
> >  static inline const char *
> > @@ -274,6 +301,115 @@ get_renderer(struct weston_compositor *ec)
> >  	return (struct gl_renderer *)ec->renderer;
> >  }
> >  
> > +static int
> > +linux_sync_file_read_timestamp(int fd, uint64_t *ts)
> > +{
> > +	struct sync_file_info file_info = { 0 };
> > +	struct sync_fence_info fence_info = { 0 };
> 
> For me, these generate compiler warnings.
> 
> /home/pq/git/weston/libweston/gl-renderer.c: In function ‘linux_sync_file_read_timestamp’:
> /home/pq/git/weston/libweston/gl-renderer.c:307:9: warning: missing braces around initializer [-Wmissing-braces]
>   struct sync_file_info file_info = { 0 };
>          ^
> /home/pq/git/weston/libweston/gl-renderer.c:307:9: warning: (near initialization for ‘file_info.name’) [-Wmissing-braces]
> /home/pq/git/weston/libweston/gl-renderer.c:308:9: warning: missing braces around initializer [-Wmissing-braces]
>   struct sync_fence_info fence_info = { 0 };
>          ^
> 
> I suppose it's a gcc version thing, looks like I'm still on 4.9.3.
> 
> I think just dropping the zero there is enough to fix it. I can do that
> when merging the patches.

The issue here is that the first struct member is an array, and
this seems to confuse older gcc versions when using '= { 0 }'.

I considered '= {}' but this seems to be a GCC extension, and wasn't
used anywhere else in the code base, so in v3 I opted for the standard
compliant solution '= { { 0 } }'.

> 
> > +
> > +	assert(ts != NULL);
> > +
> > +	file_info.sync_fence_info = (uint64_t)(uintptr_t)&fence_info;
> > +	file_info.num_fences = 1;
> > +
> > +	if (ioctl(fd, SYNC_IOC_FILE_INFO, &file_info) < 0)
> > +		return -1;
> > +
> > +	*ts = fence_info.timestamp_ns;
> > +
> > +	return 0;
> > +}
> 
> 
> Thanks,
> pq

Thanks,
Alexandros