[Mesa-dev] [PATCH v2 5/9] egl/tizen: add support of dri2_loader
Emil Velikov
emil.l.velikov at gmail.com
Thu Sep 28 14:24:23 UTC 2017
On 17 September 2017 at 19:01, Gwan-gyeong Mun <elongbug at gmail.com> wrote:
> It adds support of dri2_loader to egl dri2 tizen backend.
> - referenced a basic buffer flow and management implementation from android.
>
> And it implements a query buffer age extesion for tizen and turn on
> swap_buffers_with_damage extension.
> - it add color buffer related member variables to dri_egl_surface for a
> management of color buffers.
>
> Signed-off-by: Mun Gwan-gyeong <elongbug at gmail.com>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 9 ++
> src/egl/drivers/dri2/platform_tizen.c | 289 ++++++++++++++++++++++++++++++++--
> 2 files changed, 289 insertions(+), 9 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index 4b29b0d406..46d56e93a2 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -340,6 +340,15 @@ struct dri2_egl_surface
> tpl_surface_t *tpl_surface;
> tbm_surface_h tbm_surface;
> tbm_format tbm_format;
> +
> + /* Used to record all the tbm_surface created by tpl_surface and their ages.
> + * Usually Tizen uses at most triple buffers in tpl_surface (tbm_surface_queue)
> + * so hardcode the number of color_buffers to 3.
> + */
> + struct {
> + tbm_surface_h tbm_surface;
> + int age;
> + } color_buffers[3], *back;
> #endif
>
> #if defined(HAVE_SURFACELESS_PLATFORM)
> diff --git a/src/egl/drivers/dri2/platform_tizen.c b/src/egl/drivers/dri2/platform_tizen.c
> index efdf79682b..77684d3c1a 100644
> --- a/src/egl/drivers/dri2/platform_tizen.c
> +++ b/src/egl/drivers/dri2/platform_tizen.c
> @@ -47,6 +47,53 @@
> #include "egl_dri2_fallbacks.h"
> #include "loader.h"
>
> +static int get_format_bpp(tbm_format format)
> +{
> + int bpp;
> +
There's no need for the temporary variable, return directly.
> + switch (format) {
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wswitch"
> + case TBM_FORMAT_BGRA8888:
> + case TBM_FORMAT_RGBA8888:
> + case TBM_FORMAT_RGBX8888:
> + case TBM_FORMAT_ARGB8888:
> +#pragma GCC diagnostic pop
Please drop the pragma magic.
> + case TBM_FORMAT_XRGB8888:
> + bpp = 4;
> + break;
> + case TBM_FORMAT_RGB565:
> + bpp = 2;
> + break;
> + default:
> + bpp = 0;
> + break;
> + }
> +
> + return bpp;
> +}
> +
> +static int get_pitch(tbm_surface_h tbm_surface)
> +{
> + tbm_surface_info_s surf_info;
> +
> + if (tbm_surface_get_info(tbm_surface, &surf_info) != TBM_SURFACE_ERROR_NONE) {
> + return 0;
> + }
Nit: unneeded brackets
> +
> + return surf_info.planes[0].stride;
Please use consistent naming - function is called pitch, while you're
using .stride here.
> +}
> +
> +static int
> +get_native_buffer_name(tbm_surface_h tbm_surface)
> +{
> + uint32_t bo_name;
> +
> + bo_name = tbm_bo_export(tbm_surface_internal_get_bo(tbm_surface, 0));
> +
> + return (bo_name != 0 ) ? (int)bo_name : -1;
> +}
> +
> static EGLBoolean
> tizen_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
> {
> @@ -63,6 +110,33 @@ tizen_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
> if (dri2_surf->base.Width != width || dri2_surf->base.Height != height) {
> dri2_surf->base.Width = width;
> dri2_surf->base.Height = height;
> + dri2_egl_surface_free_local_buffers(dri2_surf);
> + }
The whole if seems pretty generic (and available elsewhere). Please
flesh out into a helper.
> +
> + /* Record all the buffers created by tpl_surface (tbm_surface_queue)
> + * and update back buffer * for updating buffer's age in swap_buffers.
> + */
> + EGLBoolean updated = EGL_FALSE;
> + for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> + if (!dri2_surf->color_buffers[i].tbm_surface) {
> + dri2_surf->color_buffers[i].tbm_surface = dri2_surf->tbm_surface;
> + dri2_surf->color_buffers[i].age = 0;
> + }
> + if (dri2_surf->color_buffers[i].tbm_surface == dri2_surf->tbm_surface) {
> + dri2_surf->back = &dri2_surf->color_buffers[i];
> + updated = EGL_TRUE;
> + break;
> + }
> + }
> +
> + if (!updated) {
> + /* In case of all the buffers were recreated , reset the color_buffers */
> + for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> + dri2_surf->color_buffers[i].tbm_surface = NULL;
> + dri2_surf->color_buffers[i].age = 0;
> + }
> + dri2_surf->color_buffers[0].tbm_surface = dri2_surf->tbm_surface;
> + dri2_surf->back = &dri2_surf->color_buffers[0];
> }
>
Same as these two - handy helpers would be appreciated. Let the helper
takes a buffer of type void * and enjoy.
> return EGL_TRUE;
> @@ -100,6 +174,7 @@ tizen_window_enqueue_buffer_with_damage(_EGLDisplay *disp,
>
> tbm_surface_internal_unref(dri2_surf->tbm_surface);
> dri2_surf->tbm_surface = NULL;
> + dri2_surf->back = NULL;
>
> mtx_lock(&disp->Mutex);
>
> @@ -200,7 +275,10 @@ tizen_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
> if (!dri2_surf->tpl_surface)
> goto cleanup_surface;
>
> - createNewDrawable = dri2_dpy->swrast->createNewDrawable;
> + if (dri2_dpy->dri2)
> + createNewDrawable = dri2_dpy->dri2->createNewDrawable;
> + else
> + createNewDrawable = dri2_dpy->swrast->createNewDrawable;
>
Ahh, yes this answers my earlier question; Thanks.
> dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, config,
> dri2_surf);
> @@ -243,6 +321,8 @@ tizen_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);
>
> + dri2_egl_surface_free_local_buffers(dri2_surf);
> +
> if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> if (dri2_surf->tbm_surface)
> tizen_window_cancel_buffer(disp, dri2_surf);
> @@ -272,6 +352,19 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> return 0;
> }
>
> +static EGLint
> +tizen_query_buffer_age(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surface)
> +{
> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surface);
> +
> + if (update_buffers(dri2_surf) < 0) {
> + _eglError(EGL_BAD_ALLOC, "tizen_query_buffer_age");
> + return -1;
> + }
> +
> + return dri2_surf->back ? dri2_surf->back->age : 0;
This seems broken. See my comments here [1].
I think there's a preexisting bug in the Android implementation that
this is papering over.
Bonus points (and brownie points) for a patch addressing the Android issue ;-)
[1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/158409.html
> +}
> +
> static EGLBoolean
> tizen_swap_buffers_with_damage(_EGLDriver *drv, _EGLDisplay *disp,
> _EGLSurface *draw, const EGLint *rects,
> @@ -283,10 +376,23 @@ tizen_swap_buffers_with_damage(_EGLDriver *drv, _EGLDisplay *disp,
> if (dri2_surf->base.Type != EGL_WINDOW_BIT)
> return EGL_TRUE;
>
> + for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
> + if (dri2_surf->color_buffers[i].age > 0)
> + dri2_surf->color_buffers[i].age++;
> + }
> +
> + if (dri2_surf->back)
I think the if check is a band-aid copied from the Android
implementation. See my discussion/reference above.
> + dri2_surf->back->age = 1;
> +
This whole hunk is fairly generic - can we make it a helper?
> if (dri2_surf->tbm_surface)
> tizen_window_enqueue_buffer_with_damage(disp, dri2_surf, rects, n_rects);
>
> - dri2_dpy->core->swapBuffers(dri2_surf->dri_drawable);
> + if (dri2_dpy->swrast) {
> + dri2_dpy->core->swapBuffers(dri2_surf->dri_drawable);
> + } else {
> + dri2_flush_drawable_for_swapbuffers(disp, draw);
> + dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
> + }
>
> return EGL_TRUE;
> }
> @@ -329,6 +435,87 @@ tizen_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf,
> return _eglQuerySurface(drv, dpy, surf, attribute, value);
> }
>
> +static void
> +tizen_flush_front_buffer(__DRIdrawable * driDrawable, void *loaderPrivate)
> +{
> +}
> +
> +static int
> +tizen_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf,
> + unsigned int *attachments, int count)
> +{
> + int num_buffers = 0;
> +
> + /* fill dri2_surf->buffers */
> + for (int i = 0; i < count * 2; i += 2) {
> + __DRIbuffer *buf, *local;
> +
> + assert(num_buffers < ARRAY_SIZE(dri2_surf->buffers));
> + buf = &dri2_surf->buffers[num_buffers];
> +
> + switch (attachments[i]) {
> + case __DRI_BUFFER_BACK_LEFT:
> + if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> + buf->attachment = attachments[i];
> + buf->name = get_native_buffer_name(dri2_surf->tbm_surface);
> + buf->cpp = get_format_bpp(tbm_surface_get_format(dri2_surf->tbm_surface));
> + buf->pitch = get_pitch(dri2_surf->tbm_surface);
> + buf->flags = 0;
> +
> + if (buf->name)
> + num_buffers++;
> +
> + break;
> + }
> + /* fall through for pbuffers */
> + case __DRI_BUFFER_DEPTH:
> + case __DRI_BUFFER_STENCIL:
> + case __DRI_BUFFER_ACCUM:
> + case __DRI_BUFFER_DEPTH_STENCIL:
> + case __DRI_BUFFER_HIZ:
> + local = dri2_egl_surface_alloc_local_buffer(dri2_surf, attachments[i],
> + attachments[i + 1]);
> +
> + if (local) {
> + *buf = *local;
> + num_buffers++;
> + }
> + break;
> + case __DRI_BUFFER_FRONT_LEFT:
> + case __DRI_BUFFER_FRONT_RIGHT:
> + case __DRI_BUFFER_FAKE_FRONT_LEFT:
> + case __DRI_BUFFER_FAKE_FRONT_RIGHT:
> + case __DRI_BUFFER_BACK_RIGHT:
> + default:
> + /* no front or right buffers */
> + break;
> + }
> + }
> +
> + return num_buffers;
> +}
> +
> +static __DRIbuffer *
> +tizen_get_buffers_with_format(__DRIdrawable * driDrawable,
> + int *width, int *height,
> + unsigned int *attachments, int count,
> + int *out_count, void *loaderPrivate)
> +{
> + struct dri2_egl_surface *dri2_surf = loaderPrivate;
> +
> + if (update_buffers(dri2_surf) < 0)
> + return NULL;
> +
> + *out_count = tizen_get_buffers_parse_attachments(dri2_surf, attachments, count);
> +
> + if (width)
> + *width = dri2_surf->base.Width;
> + if (height)
> + *height = dri2_surf->base.Height;
> +
> + return dri2_surf->buffers;
> +}
> +
> static int
> tizen_swrast_get_stride_for_format(tbm_format format, int w)
> {
> @@ -523,13 +710,21 @@ static const struct dri2_egl_display_vtbl tizen_display_vtbl = {
> .swap_buffers_region = dri2_fallback_swap_buffers_region,
> .post_sub_buffer = dri2_fallback_post_sub_buffer,
> .copy_buffers = dri2_fallback_copy_buffers,
> - .query_buffer_age = dri2_fallback_query_buffer_age,
> + .query_buffer_age = tizen_query_buffer_age,
> .query_surface = tizen_query_surface,
> .create_wayland_buffer_from_image = dri2_fallback_create_wayland_buffer_from_image,
> .get_sync_values = dri2_fallback_get_sync_values,
> .get_dri_drawable = dri2_surface_get_dri_drawable,
> };
>
> +static const __DRIdri2LoaderExtension tizen_dri2_loader_extension = {
> + .base = { __DRI_DRI2_LOADER, 3 },
> +
> + .getBuffers = NULL,
> + .getBuffersWithFormat = tizen_get_buffers_with_format,
> + .flushFrontBuffer = tizen_flush_front_buffer,
> +};
> +
> static const __DRIswrastLoaderExtension tizen_swrast_loader_extension = {
> .base = { __DRI_SWRAST_LOADER, 2 },
>
> @@ -539,6 +734,13 @@ static const __DRIswrastLoaderExtension tizen_swrast_loader_extension = {
> .putImage2 = tizen_swrast_put_image2,
> };
>
> +static const __DRIextension *tizen_dri2_loader_extensions[] = {
> + &tizen_dri2_loader_extension.base,
> + &image_lookup_extension.base,
> + &use_invalidate.base,
> + NULL,
> +};
> +
> static const __DRIextension *tizen_swrast_loader_extensions[] = {
> &tizen_swrast_loader_extension.base,
> NULL,
> @@ -551,6 +753,8 @@ dri2_initialize_tizen(_EGLDriver *drv, _EGLDisplay *dpy)
> tpl_display_t *tpl_display = NULL;
> const char *err;
> int tbm_bufmgr_fd = -1;
> + char *tbm_bufmgr_device_name = NULL;
> + int hw_accel = (getenv("LIBGL_ALWAYS_SOFTWARE") == NULL);
>
Please use env_var_as_boolean.
> loader_set_logger(_eglLog);
>
> @@ -579,13 +783,77 @@ dri2_initialize_tizen(_EGLDriver *drv, _EGLDisplay *dpy)
> goto cleanup;
> }
>
> - dri2_dpy->fd = tbm_bufmgr_fd;
> - dri2_dpy->driver_name = strdup("swrast");
> - if (!dri2_load_driver_swrast(dpy)) {
> - err = "DRI2: failed to load swrast driver";
> - goto cleanup;
> + if (hw_accel) {
> + int fd = -1;
> +
> + if (drmGetNodeTypeFromFd(tbm_bufmgr_fd) == DRM_NODE_RENDER) {
> +
> + tbm_bufmgr_device_name = loader_get_device_name_for_fd(tbm_bufmgr_fd);
> + if (tbm_bufmgr_device_name == NULL) {
> + err = "DRI2: failed to get tbm_bufmgr device name";
> + goto cleanup;
> + }
> + fd = loader_open_device(tbm_bufmgr_device_name);
> +
This looks strange:
You've got the fd already, only to get the device name and open the
device node ... to get the fd. Why?
> + } else {
> + if (!tbm_drm_helper_get_auth_info(&fd, NULL, NULL)) {
> +
> + /* FIXME: tbm_drm_helper_get_auth_info() does not support the case of
> + * display server for now. this code is fallback routine for
> + * that Enlightenment Server fails on tbm_drm_helper_get_auth_info.
> + * When tbm_drm_helper_get_auth_info() supports display server
> + * case, then remove below routine.
> + */
Hmm in this case I'm inclined to suggest:
Don't bother with that, simply drop the dri2 codepath and have the
dri3/image one ;-)
The HW platforms in question have render node enabled kernel+DRM
modules I'd assume?
Both of these have been a feature for a few years now.
-Emil
More information about the mesa-dev
mailing list