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

[Pekka Paalanen]

On Wed, 27 Sep 2017 15:01:55 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 27 September 2017 at 13:26, Alexandros Frantzis
> <alexandros.frantzis at collabora.com> wrote:
> > 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 } }'.
> >  
> FWIW, some versions of GCC/Clang will report a warning on '{ 0 }'
> while others - '{ { 0 } }'.
> The GCC extension '{}' is supported by both, for ages. Not sure about
> other compilers.
> 
> Regardless, staying consistent with the existing code sounds like the
> best option IMHO.

Hm, weston already has 19 instances of {}, but 12 instances of {{0}}.

Whatever, I don't mind as long as the compiler doesn't mind.

Emil, thanks for confirming the {} syntax should essentially be safe.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170928/d74eef36/attachment.sig>