egl image creation in case of atomic

Submitted by Hosokawa, Kenji (ADITG/ESB) on Aug. 28, 2018, 12:10 p.m.

Details

Message ID E30AD9B905CEA2449AD5E0BE05388CD90509DE4E@HI2EXCH01.adit-jv.com
State New
Series "egl image creation in case of atomic"
Headers show

Commit Message

Hosokawa, Kenji (ADITG/ESB) Aug. 28, 2018, 12:10 p.m.
Hi,

When eglSwapInterval is 0, clients may send buffers without waiting frame callbacks. Currently, egl image is created when a buffer is committed from client. But egl image is only needed, when gl-renderer is used to render the framebuffer. Creating an egl image in every commit causes additional CPU load in weston when clients are sending more buffers than display refresh rate. Furthermore, creating egl images are not needed at all, when the client buffer can be imported to a DRM plane. We would like to reduce CPU usage of weston in that case.
In my investigation, egl image creation can produce higher CPU load in Weston. I tried to remove egl image creation with this simple ugly patch for weston.


Without the patch (original), total CPU usage of weston was 25%. With the patch, it was decreased to 10%.
I used weston v4.0.92, weston-simple-egl with -o and -b options, rcar h3 target. It was measured with top command simply.

My question is that can egl image creation be postponed until repaint output (gl_renderer_repaint_output)?

Best regards

Kenji Hosokawa
Engineering Software Base (ADITG/ESB)

Patch hide | download patch | download mbox

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index 2c50d2d..bbb5846 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -2260,6 +2260,8 @@  gl_renderer_attach_dmabuf(struct weston_surface *surface,
        buffer->y_inverted =
                !(dmabuf->attributes.flags & ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);

+       return;
+
        for (i = 0; i < gs->num_images; i++)
                egl_image_unref(gs->images[i]);
        gs->num_images = 0;

Comments

Hosokawa, Kenji (ADITG/ESB) Sept. 10, 2018, 3:35 p.m.
> -----Original Message-----
> From: wayland-devel <wayland-devel-bounces@lists.freedesktop.org> On Behalf Of Hosokawa, Kenji (ADITG/ESB)
> Sent: Dienstag, 28. August 2018 14:32
> To: wayland-devel@lists.freedesktop.org
> Subject: egl image creation in case of atomic
> 
> Hi,
> 
> When eglSwapInterval is 0, clients may send buffers without waiting frame callbacks. Currently, egl image is created when a buffer is
> committed from client. But egl image is only needed, when gl-renderer is used to render the framebuffer. Creating an egl image in
> every commit causes additional CPU load in weston when clients are sending more buffers than display refresh rate. Furthermore,
> creating egl images are not needed at all, when the client buffer can be imported to a DRM plane. We would like to reduce CPU usage
> of weston in that case.
> In my investigation, egl image creation can produce higher CPU load in Weston. I tried to remove egl image creation with this simple
> ugly patch for weston.
> 
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 2c50d2d..bbb5846 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -2260,6 +2260,8 @@ gl_renderer_attach_dmabuf(struct weston_surface *surface,
>         buffer->y_inverted =
>                 !(dmabuf->attributes.flags & ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);
> 
> +       return;
> +
>         for (i = 0; i < gs->num_images; i++)
>                 egl_image_unref(gs->images[i]);
>         gs->num_images = 0;
> 
> Without the patch (original), total CPU usage of weston was 25%. With the patch, it was decreased to 10%.
> I used weston v4.0.92, weston-simple-egl with -o and -b options, rcar h3 target. It was measured with top command simply.
> 
> My question is that can egl image creation be postponed until repaint output (gl_renderer_repaint_output)?

Hi,

Do you have any ideas for my question? If it makes sense in principle, I will jump into the detail to achieve it.
If there are some problems which I don't know, I hope you'll let me know.

Best regards

Kenji Hosokawa
Engineering Software Base (ADITG/ESB)

> Best regards
> 
> Kenji Hosokawa
> Engineering Software Base (ADITG/ESB)
Pekka Paalanen Sept. 11, 2018, 11:19 a.m.
On Tue, 28 Aug 2018 12:10:06 +0000
"Hosokawa, Kenji (ADITG/ESB)" <khosokawa@de.adit-jv.com> wrote:

> Hi,
> 
> When eglSwapInterval is 0, clients may send buffers without waiting
> frame callbacks. Currently, egl image is created when a buffer is
> committed from client. But egl image is only needed, when gl-renderer
> is used to render the framebuffer. Creating an egl image in every
> commit causes additional CPU load in weston when clients are sending
> more buffers than display refresh rate. Furthermore, creating egl
> images are not needed at all, when the client buffer can be imported
> to a DRM plane. We would like to reduce CPU usage of weston in that
> case. In my investigation, egl image creation can produce higher CPU
> load in Weston. I tried to remove egl image creation with this simple
> ugly patch for weston.
> 
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 2c50d2d..bbb5846 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -2260,6 +2260,8 @@ gl_renderer_attach_dmabuf(struct weston_surface *surface,
>         buffer->y_inverted =
>                 !(dmabuf->attributes.flags & ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);
> 
> +       return;
> +
>         for (i = 0; i < gs->num_images; i++)
>                 egl_image_unref(gs->images[i]);
>         gs->num_images = 0;
> 
> Without the patch (original), total CPU usage of weston was 25%. With
> the patch, it was decreased to 10%. I used weston v4.0.92,
> weston-simple-egl with -o and -b options, rcar h3 target. It was
> measured with top command simply.
> 
> My question is that can egl image creation be postponed until repaint
> output (gl_renderer_repaint_output)?

Hi,

I suppose the EGLImage creation could be postponed to the repaint
phase.

The client is being abusive, which is why I think this has not been an
issue before. The best solution would be to fix the client to throttle
appropriately, that would produce the biggest savings. Why not do that
instead?

My recommended approach in clients is to use SwapInterval=0 but throttle
manually to the wl_surface.frame callbacks or presentation_feedback.
This would give the most control in the client while not wasting power.


Thanks,
pq
Hosokawa, Kenji (ADITG/ESB) Sept. 11, 2018, 12:47 p.m.
> -----Original Message-----
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Dienstag, 11. September 2018 13:20
> To: Hosokawa, Kenji (ADITG/ESB) <khosokawa@de.adit-jv.com>
> Cc: wayland-devel@lists.freedesktop.org
> Subject: Re: egl image creation in case of atomic
> 
> On Tue, 28 Aug 2018 12:10:06 +0000
> "Hosokawa, Kenji (ADITG/ESB)" <khosokawa@de.adit-jv.com> wrote:
> 
> > Hi,
> >
> > When eglSwapInterval is 0, clients may send buffers without waiting
> > frame callbacks. Currently, egl image is created when a buffer is
> > committed from client. But egl image is only needed, when gl-renderer
> > is used to render the framebuffer. Creating an egl image in every
> > commit causes additional CPU load in weston when clients are sending
> > more buffers than display refresh rate. Furthermore, creating egl
> > images are not needed at all, when the client buffer can be imported
> > to a DRM plane. We would like to reduce CPU usage of weston in that
> > case. In my investigation, egl image creation can produce higher CPU
> > load in Weston. I tried to remove egl image creation with this simple
> > ugly patch for weston.
> >
> > diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c index
> > 2c50d2d..bbb5846 100644
> > --- a/libweston/gl-renderer.c
> > +++ b/libweston/gl-renderer.c
> > @@ -2260,6 +2260,8 @@ gl_renderer_attach_dmabuf(struct weston_surface *surface,
> >         buffer->y_inverted =
> >                 !(dmabuf->attributes.flags &
> > ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT);
> >
> > +       return;
> > +
> >         for (i = 0; i < gs->num_images; i++)
> >                 egl_image_unref(gs->images[i]);
> >         gs->num_images = 0;
> >
> > Without the patch (original), total CPU usage of weston was 25%. With
> > the patch, it was decreased to 10%. I used weston v4.0.92,
> > weston-simple-egl with -o and -b options, rcar h3 target. It was
> > measured with top command simply.
> >
> > My question is that can egl image creation be postponed until repaint
> > output (gl_renderer_repaint_output)?
> 
> Hi,
> 
> I suppose the EGLImage creation could be postponed to the repaint phase.
> 
> The client is being abusive, which is why I think this has not been an issue before. The best solution would be to fix the client to
> throttle appropriately, that would produce the biggest savings. Why not do that instead?
> 
> My recommended approach in clients is to use SwapInterval=0 but throttle manually to the wl_surface.frame callbacks or
> presentation_feedback.
> This would give the most control in the client while not wasting power.

Hi Pekka,

Thank you for the comments and sorry for my misleading comments.
Even in our use case, there are no clients like sending 2000 frames per second. The client should be fix if it exists.

We would like to discuss the following part.
>Creating egl images are not needed at all, when the client buffer can be imported to a DRM plane. We would like to reduce CPU usage of weston in that case.

Best regards

Kenji Hosokawa
Engineering Software Base (ADITG/ESB)
Pekka Paalanen Sept. 11, 2018, 1:10 p.m.
On Tue, 11 Sep 2018 12:47:15 +0000
"Hosokawa, Kenji (ADITG/ESB)" <khosokawa@de.adit-jv.com> wrote:

> We would like to discuss the following part.
> >Creating egl images are not needed at all, when the client buffer
> >can be imported to a DRM plane. We would like to reduce CPU usage of
> >weston in that case.  

Hi,

ok, that sounds fine to me. Maybe something like this: the GL-renderer
attach hook just unrefs existing EGLImages but does not create new
ones. New ones are created at repaint time on demand. This should
implictly result in composite-bypass avoiding the EGLImage creation,
because the renderer does not repaint those views.

Does this answer your question?

This is just off the top of my head, I didn't actually check how the
code would allow this.


Thanks,
pq
Pekka Paalanen Sept. 11, 2018, 2:34 p.m.
On Tue, 11 Sep 2018 16:10:17 +0300
Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Tue, 11 Sep 2018 12:47:15 +0000
> "Hosokawa, Kenji (ADITG/ESB)" <khosokawa@de.adit-jv.com> wrote:
> 
> > We would like to discuss the following part.  
> > >Creating egl images are not needed at all, when the client buffer
> > >can be imported to a DRM plane. We would like to reduce CPU usage of
> > >weston in that case.    
> 
> Hi,
> 
> ok, that sounds fine to me. Maybe something like this: the GL-renderer
> attach hook just unrefs existing EGLImages but does not create new
> ones. New ones are created at repaint time on demand. This should
> implictly result in composite-bypass avoiding the EGLImage creation,
> because the renderer does not repaint those views.
> 
> Does this answer your question?
> 
> This is just off the top of my head, I didn't actually check how the
> code would allow this.

Oh, one important detail:

One cannot skip creating the EGLImage in the
zwp_linux_buffer_params_v1.create path, because it is the test to see
if the compositor can use the dmabuf at all. It must be done at the
create step, so that potential failure can be communicated gracefully.

One might be able to argue, that skipping the initial import test in
create_immed path is ok, because it is meant to be unchecked anyway and
documented to result in fatal protocol errors if anything fails.

So you may not be able to eliminate the EGLImage creation during
wl_buffer creation, but you should be able to eliminate or optimize the
EGLImage creation when an existing wl_buffer is re-used. I hope that is
sufficient for your use cases.


Thanks,
pq
Hosokawa, Kenji (ADITG/ESB) Sept. 13, 2018, 10:48 a.m.
> -----Original Message-----
> From: Pekka Paalanen <ppaalanen@gmail.com>
> Sent: Dienstag, 11. September 2018 16:35
> To: Hosokawa, Kenji (ADITG/ESB) <khosokawa@de.adit-jv.com>
> Cc: wayland-devel@lists.freedesktop.org
> Subject: Re: egl image creation in case of atomic
> 
> On Tue, 11 Sep 2018 16:10:17 +0300
> Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Tue, 11 Sep 2018 12:47:15 +0000
> > "Hosokawa, Kenji (ADITG/ESB)" <khosokawa@de.adit-jv.com> wrote:
> >
> > > We would like to discuss the following part.
> > > >Creating egl images are not needed at all, when the client buffer
> > > >can be imported to a DRM plane. We would like to reduce CPU usage of
> > > >weston in that case.
> >
> > Hi,
> >
> > ok, that sounds fine to me. Maybe something like this: the GL-renderer
> > attach hook just unrefs existing EGLImages but does not create new
> > ones. New ones are created at repaint time on demand. This should
> > implictly result in composite-bypass avoiding the EGLImage creation,
> > because the renderer does not repaint those views.

Hi Pekka,

I was just thinking like that.

> >
> > Does this answer your question?
> >
> > This is just off the top of my head, I didn't actually check how the
> > code would allow this.
> 
> Oh, one important detail:
> 
> One cannot skip creating the EGLImage in the zwp_linux_buffer_params_v1.create path, because it is the test to see if the
> compositor can use the dmabuf at all. It must be done at the create step, so that potential failure can be communicated gracefully.
> 
> One might be able to argue, that skipping the initial import test in create_immed path is ok, because it is meant to be unchecked
> anyway and documented to result in fatal protocol errors if anything fails.
> 
> So you may not be able to eliminate the EGLImage creation during wl_buffer creation, but you should be able to eliminate or optimize
> the EGLImage creation when an existing wl_buffer is re-used. I hope that is sufficient for your use cases.

Thank you so much for the knowledge. I wanted to know such high-level design information.
I thought that some EGLimage creation would be necessary but I was not sure. I will look at there.

Best regards

Kenji Hosokawa
Engineering Software Base (ADITG/ESB)