[Mesa-dev] [PATCH] egl: deduplicate allocations of local buffer over each platform backend (v2)
Eric Engestrom
eric.engestrom at imgtec.com
Fri Aug 4 15:42:47 UTC 2017
On Saturday, 2017-08-05 00:16:11 +0900, Gwan-gyeong Mun wrote:
> platform_drm, platform_wayland and platform_android have similiar local buffer
> allocation routines. For deduplicating, it unifies dri2_egl_surface's
> local buffer allocation routines. And it polishes inconsistent indentations.
>
> Note that as dri2_wl_get_buffers_with_format() have not make a __DRI_BUFFER_BACK_LEFT
> attachment buffer for local_buffers, new helper function, dri2_egl_surface_free_local_buffers(),
> will drop the __DRI_BUFFER_BACK_LEFT check.
> So if other platforms use new helper functions, we have to ensure not to make
> __DRI_BUFFER_BACK_LEFT attachment buffer for local_buffers.
>
> v2: Fixes from Emil's review:
> a) Make local_buffers variable, dri2_egl_surface_alloc_local_buffer() and
> dri2_egl_surface_free_local_buffers() unconditionally.
> b) Preserve the original codeflow for error_path and normal_path.
> c) Add note on commit messages for dropping of __DRI_BUFFER_BACK_LEFT check.
> c) Rollback the unrelated whitespace changes.
The dri2_drm_get_buffers_with_format() unrelated whitespace change is
still here :P
(don't bother unless you need to do a v3 for some other reason)
Once this lands, platform_drm (and probably others) could use
a full whitespace cleanup pass, if you want :)
> d) Add a missing blank line.
>
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
v2 looks good to me.
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
Thanks!
> ---
> src/egl/drivers/dri2/egl_dri2.c | 34 +++++++++++++++++++++
> src/egl/drivers/dri2/egl_dri2.h | 14 ++++++---
> src/egl/drivers/dri2/platform_android.c | 40 ++-----------------------
> src/egl/drivers/dri2/platform_drm.c | 46 ++++++++---------------------
> src/egl/drivers/dri2/platform_wayland.c | 52 ++++++++-------------------------
> 5 files changed, 71 insertions(+), 115 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 733659d547..129cc35872 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -972,6 +972,40 @@ dri2_display_destroy(_EGLDisplay *disp)
> disp->DriverData = NULL;
> }
>
> +__DRIbuffer *
> +dri2_egl_surface_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
> + unsigned int att, unsigned int format)
> +{
> + struct dri2_egl_display *dri2_dpy =
> + dri2_egl_display(dri2_surf->base.Resource.Display);
> +
> + if (att >= ARRAY_SIZE(dri2_surf->local_buffers))
> + return NULL;
> +
> + if (!dri2_surf->local_buffers[att]) {
> + dri2_surf->local_buffers[att] =
> + dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, att, format,
> + dri2_surf->base.Width, dri2_surf->base.Height);
> + }
> +
> + return dri2_surf->local_buffers[att];
> +}
> +
> +void
> +dri2_egl_surface_free_local_buffers(struct dri2_egl_surface *dri2_surf)
> +{
> + struct dri2_egl_display *dri2_dpy =
> + dri2_egl_display(dri2_surf->base.Resource.Display);
> +
> + for (int i = 0; i < ARRAY_SIZE(dri2_surf->local_buffers); i++) {
> + if (dri2_surf->local_buffers[i]) {
> + dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> + dri2_surf->local_buffers[i]);
> + dri2_surf->local_buffers[i] = NULL;
> + }
> + }
> +}
> +
> /**
> * Called via eglTerminate(), drv->API.Terminate().
> *
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index ccfefef61f..6c7d75587a 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -283,8 +283,10 @@ struct dri2_egl_surface
> struct gbm_dri_surface *gbm_surf;
> #endif
>
> + /* EGL-owned buffers */
> + __DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
> +
> #if defined(HAVE_WAYLAND_PLATFORM) || defined(HAVE_DRM_PLATFORM)
> - __DRIbuffer *dri_buffers[__DRI_BUFFER_COUNT];
> struct {
> #ifdef HAVE_WAYLAND_PLATFORM
> struct wl_buffer *wl_buffer;
> @@ -309,9 +311,6 @@ struct dri2_egl_surface
> __DRIimage *dri_image_back;
> __DRIimage *dri_image_front;
>
> - /* EGL-owned buffers */
> - __DRIbuffer *local_buffers[__DRI_BUFFER_COUNT];
> -
> /* Used to record all the buffers created by ANativeWindow and their ages.
> * Usually Android uses at most triple buffers in ANativeWindow
> * so hardcode the number of color_buffers to 3.
> @@ -451,4 +450,11 @@ dri2_set_WL_bind_wayland_display(_EGLDriver *drv, _EGLDisplay *disp)
> void
> dri2_display_destroy(_EGLDisplay *disp);
>
> +__DRIbuffer *
> +dri2_egl_surface_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
> + unsigned int att, unsigned int format);
> +
> +void
> +dri2_egl_surface_free_local_buffers(struct dri2_egl_surface *dri2_surf);
> +
> #endif /* EGL_DRI2_INCLUDED */
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index 50a8248695..71dd1594c7 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -271,40 +271,6 @@ droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
> }
> }
>
> -static __DRIbuffer *
> -droid_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
> - unsigned int att, unsigned int format)
> -{
> - struct dri2_egl_display *dri2_dpy =
> - dri2_egl_display(dri2_surf->base.Resource.Display);
> -
> - if (att >= ARRAY_SIZE(dri2_surf->local_buffers))
> - return NULL;
> -
> - if (!dri2_surf->local_buffers[att]) {
> - dri2_surf->local_buffers[att] =
> - dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, att, format,
> - dri2_surf->base.Width, dri2_surf->base.Height);
> - }
> -
> - return dri2_surf->local_buffers[att];
> -}
> -
> -static void
> -droid_free_local_buffers(struct dri2_egl_surface *dri2_surf)
> -{
> - struct dri2_egl_display *dri2_dpy =
> - dri2_egl_display(dri2_surf->base.Resource.Display);
> -
> - for (int i = 0; i < ARRAY_SIZE(dri2_surf->local_buffers); i++) {
> - if (dri2_surf->local_buffers[i]) {
> - dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> - dri2_surf->local_buffers[i]);
> - dri2_surf->local_buffers[i] = NULL;
> - }
> - }
> -}
> -
> static _EGLSurface *
> droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
> _EGLConfig *conf, void *native_window,
> @@ -400,7 +366,7 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
>
> - droid_free_local_buffers(dri2_surf);
> + dri2_egl_surface_free_local_buffers(dri2_surf);
>
> if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> if (dri2_surf->buffer)
> @@ -447,7 +413,7 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> /* free outdated buffers and update the surface size */
> if (dri2_surf->base.Width != dri2_surf->buffer->width ||
> dri2_surf->base.Height != dri2_surf->buffer->height) {
> - droid_free_local_buffers(dri2_surf);
> + dri2_egl_surface_free_local_buffers(dri2_surf);
> dri2_surf->base.Width = dri2_surf->buffer->width;
> dri2_surf->base.Height = dri2_surf->buffer->height;
> }
> @@ -970,7 +936,7 @@ droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf,
> case __DRI_BUFFER_ACCUM:
> case __DRI_BUFFER_DEPTH_STENCIL:
> case __DRI_BUFFER_HIZ:
> - local = droid_alloc_local_buffer(dri2_surf,
> + local = dri2_egl_surface_alloc_local_buffer(dri2_surf,
> attachments[i], attachments[i + 1]);
>
> if (local) {
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index a952aa5456..6895d2fbfe 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -196,11 +196,7 @@ dri2_drm_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> gbm_bo_destroy(dri2_surf->color_buffers[i].bo);
> }
>
> - for (unsigned i = 0; i < __DRI_BUFFER_COUNT; i++) {
> - if (dri2_surf->dri_buffers[i])
> - dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> - dri2_surf->dri_buffers[i]);
> - }
> + dri2_egl_surface_free_local_buffers(dri2_surf);
>
> free(surf);
>
> @@ -291,39 +287,18 @@ back_bo_to_dri_buffer(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
> buffer->flags = 0;
> }
>
> -static int
> -get_aux_bo(struct dri2_egl_surface *dri2_surf,
> - unsigned int attachment, unsigned int format, __DRIbuffer *buffer)
> -{
> - struct dri2_egl_display *dri2_dpy =
> - dri2_egl_display(dri2_surf->base.Resource.Display);
> - __DRIbuffer *b = dri2_surf->dri_buffers[attachment];
> -
> - if (b == NULL) {
> - b = dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen,
> - attachment, format,
> - dri2_surf->base.Width,
> - dri2_surf->base.Height);
> - dri2_surf->dri_buffers[attachment] = b;
> - }
> - if (b == NULL)
> - return -1;
> -
> - memcpy(buffer, b, sizeof *buffer);
> -
> - return 0;
> -}
> -
> static __DRIbuffer *
> dri2_drm_get_buffers_with_format(__DRIdrawable *driDrawable,
> - int *width, int *height,
> - unsigned int *attachments, int count,
> - int *out_count, void *loaderPrivate)
> + int *width, int *height,
> + unsigned int *attachments, int count,
> + int *out_count, void *loaderPrivate)
> {
> struct dri2_egl_surface *dri2_surf = loaderPrivate;
> int i, j;
>
> for (i = 0, j = 0; i < 2 * count; i += 2, j++) {
> + __DRIbuffer *local;
> +
> assert(attachments[i] < __DRI_BUFFER_COUNT);
> assert(j < ARRAY_SIZE(dri2_surf->buffers));
>
> @@ -336,11 +311,14 @@ dri2_drm_get_buffers_with_format(__DRIdrawable *driDrawable,
> back_bo_to_dri_buffer(dri2_surf, &dri2_surf->buffers[j]);
> break;
> default:
> - if (get_aux_bo(dri2_surf, attachments[i], attachments[i + 1],
> - &dri2_surf->buffers[j]) < 0) {
> - _eglError(EGL_BAD_ALLOC, "failed to allocate aux buffer");
> + local = dri2_egl_surface_alloc_local_buffer(dri2_surf, attachments[i],
> + attachments[i + 1]);
> +
> + if (!local) {
> + _eglError(EGL_BAD_ALLOC, "failed to allocate local buffer");
> return NULL;
> }
> + dri2_surf->buffers[j] = *local;
> break;
> }
> }
> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> index 73966b7c50..54de55ba4c 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -282,13 +282,8 @@ dri2_wl_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
> dri2_surf->color_buffers[i].data_size);
> }
>
> - if (dri2_dpy->dri2) {
> - for (int i = 0; i < __DRI_BUFFER_COUNT; i++)
> - if (dri2_surf->dri_buffers[i] &&
> - dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
> - dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> - dri2_surf->dri_buffers[i]);
> - }
> + if (dri2_dpy->dri2)
> + dri2_egl_surface_free_local_buffers(dri2_surf);
>
> if (dri2_surf->throttle_callback)
> wl_callback_destroy(dri2_surf->throttle_callback);
> @@ -335,13 +330,8 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
> dri2_surf->color_buffers[i].locked = false;
> }
>
> - if (dri2_dpy->dri2) {
> - for (int i = 0; i < __DRI_BUFFER_COUNT; i++)
> - if (dri2_surf->dri_buffers[i] &&
> - dri2_surf->dri_buffers[i]->attachment != __DRI_BUFFER_BACK_LEFT)
> - dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
> - dri2_surf->dri_buffers[i]);
> - }
> + if (dri2_dpy->dri2)
> + dri2_egl_surface_free_local_buffers(dri2_surf);
> }
>
> static int
> @@ -496,29 +486,6 @@ back_bo_to_dri_buffer(struct dri2_egl_surface *dri2_surf, __DRIbuffer *buffer)
> }
>
> static int
> -get_aux_bo(struct dri2_egl_surface *dri2_surf,
> - unsigned int attachment, unsigned int format, __DRIbuffer *buffer)
> -{
> - struct dri2_egl_display *dri2_dpy =
> - dri2_egl_display(dri2_surf->base.Resource.Display);
> - __DRIbuffer *b = dri2_surf->dri_buffers[attachment];
> -
> - if (b == NULL) {
> - b = dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen,
> - attachment, format,
> - dri2_surf->base.Width,
> - dri2_surf->base.Height);
> - dri2_surf->dri_buffers[attachment] = b;
> - }
> - if (b == NULL)
> - return -1;
> -
> - memcpy(buffer, b, sizeof *buffer);
> -
> - return 0;
> -}
> -
> -static int
> update_buffers(struct dri2_egl_surface *dri2_surf)
> {
> struct dri2_egl_display *dri2_dpy =
> @@ -572,16 +539,21 @@ dri2_wl_get_buffers_with_format(__DRIdrawable * driDrawable,
> return NULL;
>
> for (i = 0, j = 0; i < 2 * count; i += 2, j++) {
> + __DRIbuffer *local;
> +
> switch (attachments[i]) {
> case __DRI_BUFFER_BACK_LEFT:
> back_bo_to_dri_buffer(dri2_surf, &dri2_surf->buffers[j]);
> break;
> default:
> - if (get_aux_bo(dri2_surf, attachments[i], attachments[i + 1],
> - &dri2_surf->buffers[j]) < 0) {
> - _eglError(EGL_BAD_ALLOC, "failed to allocate aux buffer");
> + local = dri2_egl_surface_alloc_local_buffer(dri2_surf, attachments[i],
> + attachments[i + 1]);
> +
> + if (!local) {
> + _eglError(EGL_BAD_ALLOC, "failed to allocate local buffer");
> return NULL;
> }
> + dri2_surf->buffers[j] = *local;
> break;
> }
> }
> --
> 2.13.3
>
More information about the mesa-dev
mailing list