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 <[email protected]> > Reviewed-by: Emil Velikov <[email protected]> v2 looks good to me. Reviewed-by: Eric Engestrom <[email protected]> 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 > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
