Re: [Mesa-dev] [PATCH kmscube 2/2] Use weak functions to handle lack of gbm modifiers

[Rob Clark]

On Fri, Mar 30, 2018 at 10:17 AM, Emil Velikov  wrote:
> From: Emil Velikov 
>
> Add weak function declaration and check if they're valid prior
> to calling the functions.
>
> This allows us to remove conditional compilation, yet allowing the
> modifiers codepath to work if API is available.
>
> Cc: Christian Gmeiner 
> Cc: Rob Clark 
> Signed-off-by: Emil Velikov 

Reviewed-by: Rob Clark 

> ---
>  common.c | 32 +---
>  common.h |  2 ++
>  configure.ac | 16 
>  drm-common.c | 49 -
>  4 files changed, 55 insertions(+), 44 deletions(-)
>
> diff --git a/common.c b/common.c
> index 332e3e9..1d97c91 100644
> --- a/common.c
> +++ b/common.c
> @@ -33,23 +33,33 @@
>
>  static struct gbm gbm;
>
> +WEAK struct gbm_surface *
> +gbm_surface_create_with_modifiers(struct gbm_device *gbm,
> +  uint32_t width, uint32_t height,
> +  uint32_t format,
> +  const uint64_t *modifiers,
> +  const unsigned int count);
> +
>  const struct gbm * init_gbm(int drm_fd, int w, int h, uint64_t modifier)
>  {
> gbm.dev = gbm_create_device(drm_fd);
> gbm.format = GBM_FORMAT_XRGB;
>
> -#ifndef HAVE_GBM_MODIFIERS
> -   if (modifier != DRM_FORMAT_MOD_LINEAR) {
> -   fprintf(stderr, "Modifiers requested but support isn't 
> available\n");
> -   return NULL;
> +   if (gbm_surface_create_with_modifiers) {
> +   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
> +   gbm.format,
> +   , 1);
> +
> +   } else {
> +   if (modifier != DRM_FORMAT_MOD_LINEAR) {
> +   fprintf(stderr, "Modifiers requested but support 
> isn't available\n");
> +   return NULL;
> +   }
> +   gbm.surface = gbm_surface_create(gbm.dev, w, h,
> +   gbm.format,
> +   GBM_BO_USE_SCANOUT | 
> GBM_BO_USE_RENDERING);
> +
> }
> -   gbm.surface = gbm_surface_create(gbm.dev, w, h,
> -   gbm.format,
> -   GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING);
> -#else
> -   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
> -   gbm.format, , 1);
> -#endif
>
> if (!gbm.surface) {
> printf("failed to create gbm surface\n");
> diff --git a/common.h b/common.h
> index dc87825..42eb236 100644
> --- a/common.h
> +++ b/common.h
> @@ -57,6 +57,8 @@ EGLAPI EGLSurface EGLAPIENTRY 
> eglCreatePlatformPixmapSurfaceEXT (EGLDisplay dpy,
>  #endif
>  #endif /* EGL_EXT_platform_base */
>
> +#define WEAK __attribute__((weak))
> +
>  struct gbm {
> struct gbm_device *dev;
> struct gbm_surface *surface;
> diff --git a/configure.ac b/configure.ac
> index 8397f7b..5a63afb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -49,21 +49,5 @@ if test "x$HAVE_GST" = "xyes"; then
>  fi
>  AM_CONDITIONAL(ENABLE_GST, [test "x$HAVE_GST" = "xyes"])
>
> -AC_CHECK_LIB([gbm], [gbm_bo_get_modifier], [gbm_modifiers=yes], [])
> -
> -AC_ARG_ENABLE([gbm-modifiers],
> - [AS_HELP_STRING([--enable-gbm-modifiers],
> - [enable using GBM modifiers @<:@default=auto@:>@])],
> - [enable_gbm_modifiers="$enableval"],
> - [enable_gbm_modifiers=auto])
> -
> -if test "x$enable_gbm_modifiers" = xyes -a "x$gbm_modifiers" != xyes; then
> -   AC_MSG_ERROR([Cannot find gbm modifier supported mesa])
> -fi
> -
> -if test "x$enable_gbm_modifiers" = xyes; then
> -   AC_DEFINE(HAVE_GBM_MODIFIERS, 1, [Define if you can use GBM 
> properties.])
> -fi
> -
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
> diff --git a/drm-common.c b/drm-common.c
> index 1ec2820..136fe69 100644
> --- a/drm-common.c
> +++ b/drm-common.c
> @@ -31,6 +31,18 @@
>  #include "common.h"
>  #include "drm-common.h"
>
> +WEAK uint64_t
> +gbm_bo_get_modifier(struct gbm_bo *bo);
> +
> +WEAK int
> +gbm_bo_get_plane_count(struct gbm_bo *bo);
> +
> +WEAK uint32_t
> +gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int plane);
> +
> +WEAK uint32_t
> +gbm_bo_get_offset(struct gbm_bo *bo, int plane);
> +
>  static void
>  drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
>  {
> @@ -62,26 +74,29 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
> height = gbm_bo_get_height(bo);
> format = gbm_bo_get_format(bo);
>
> -#ifdef HAVE_GBM_MODIFIERS
> -   uint64_t modifiers[4] = {0};
> -   modifiers[0] = gbm_bo_get_modifier(bo);
> -   const int 

Re: [Mesa-dev] [PATCH kmscube 2/2] Use weak functions to handle lack of gbm modifiers

[Christian Gmeiner]

2018-03-30 16:17 GMT+02:00 Emil Velikov :
> From: Emil Velikov 
>
> Add weak function declaration and check if they're valid prior
> to calling the functions.
>
> This allows us to remove conditional compilation, yet allowing the
> modifiers codepath to work if API is available.
>
> Cc: Christian Gmeiner 
> Cc: Rob Clark 
> Signed-off-by: Emil Velikov 

Reviewed-by: Christian Gmeiner 

> ---
>  common.c | 32 +---
>  common.h |  2 ++
>  configure.ac | 16 
>  drm-common.c | 49 -
>  4 files changed, 55 insertions(+), 44 deletions(-)
>
> diff --git a/common.c b/common.c
> index 332e3e9..1d97c91 100644
> --- a/common.c
> +++ b/common.c
> @@ -33,23 +33,33 @@
>
>  static struct gbm gbm;
>
> +WEAK struct gbm_surface *
> +gbm_surface_create_with_modifiers(struct gbm_device *gbm,
> +  uint32_t width, uint32_t height,
> +  uint32_t format,
> +  const uint64_t *modifiers,
> +  const unsigned int count);
> +
>  const struct gbm * init_gbm(int drm_fd, int w, int h, uint64_t modifier)
>  {
> gbm.dev = gbm_create_device(drm_fd);
> gbm.format = GBM_FORMAT_XRGB;
>
> -#ifndef HAVE_GBM_MODIFIERS
> -   if (modifier != DRM_FORMAT_MOD_LINEAR) {
> -   fprintf(stderr, "Modifiers requested but support isn't 
> available\n");
> -   return NULL;
> +   if (gbm_surface_create_with_modifiers) {
> +   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
> +   gbm.format,
> +   , 1);
> +
> +   } else {
> +   if (modifier != DRM_FORMAT_MOD_LINEAR) {
> +   fprintf(stderr, "Modifiers requested but support 
> isn't available\n");
> +   return NULL;
> +   }
> +   gbm.surface = gbm_surface_create(gbm.dev, w, h,
> +   gbm.format,
> +   GBM_BO_USE_SCANOUT | 
> GBM_BO_USE_RENDERING);
> +
> }
> -   gbm.surface = gbm_surface_create(gbm.dev, w, h,
> -   gbm.format,
> -   GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING);
> -#else
> -   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
> -   gbm.format, , 1);
> -#endif
>
> if (!gbm.surface) {
> printf("failed to create gbm surface\n");
> diff --git a/common.h b/common.h
> index dc87825..42eb236 100644
> --- a/common.h
> +++ b/common.h
> @@ -57,6 +57,8 @@ EGLAPI EGLSurface EGLAPIENTRY 
> eglCreatePlatformPixmapSurfaceEXT (EGLDisplay dpy,
>  #endif
>  #endif /* EGL_EXT_platform_base */
>
> +#define WEAK __attribute__((weak))
> +
>  struct gbm {
> struct gbm_device *dev;
> struct gbm_surface *surface;
> diff --git a/configure.ac b/configure.ac
> index 8397f7b..5a63afb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -49,21 +49,5 @@ if test "x$HAVE_GST" = "xyes"; then
>  fi
>  AM_CONDITIONAL(ENABLE_GST, [test "x$HAVE_GST" = "xyes"])
>
> -AC_CHECK_LIB([gbm], [gbm_bo_get_modifier], [gbm_modifiers=yes], [])
> -
> -AC_ARG_ENABLE([gbm-modifiers],
> - [AS_HELP_STRING([--enable-gbm-modifiers],
> - [enable using GBM modifiers @<:@default=auto@:>@])],
> - [enable_gbm_modifiers="$enableval"],
> - [enable_gbm_modifiers=auto])
> -
> -if test "x$enable_gbm_modifiers" = xyes -a "x$gbm_modifiers" != xyes; then
> -   AC_MSG_ERROR([Cannot find gbm modifier supported mesa])
> -fi
> -
> -if test "x$enable_gbm_modifiers" = xyes; then
> -   AC_DEFINE(HAVE_GBM_MODIFIERS, 1, [Define if you can use GBM 
> properties.])
> -fi
> -
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
> diff --git a/drm-common.c b/drm-common.c
> index 1ec2820..136fe69 100644
> --- a/drm-common.c
> +++ b/drm-common.c
> @@ -31,6 +31,18 @@
>  #include "common.h"
>  #include "drm-common.h"
>
> +WEAK uint64_t
> +gbm_bo_get_modifier(struct gbm_bo *bo);
> +
> +WEAK int
> +gbm_bo_get_plane_count(struct gbm_bo *bo);
> +
> +WEAK uint32_t
> +gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int plane);
> +
> +WEAK uint32_t
> +gbm_bo_get_offset(struct gbm_bo *bo, int plane);
> +
>  static void
>  drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
>  {
> @@ -62,26 +74,29 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
> height = gbm_bo_get_height(bo);
> format = gbm_bo_get_format(bo);
>
> -#ifdef HAVE_GBM_MODIFIERS
> -   uint64_t modifiers[4] = {0};
> -   modifiers[0] = gbm_bo_get_modifier(bo);
> -   const int 

[Mesa-dev] [PATCH kmscube 2/2] Use weak functions to handle lack of gbm modifiers

[Emil Velikov]

From: Emil Velikov 

Add weak function declaration and check if they're valid prior
to calling the functions.

This allows us to remove conditional compilation, yet allowing the
modifiers codepath to work if API is available.

Cc: Christian Gmeiner 
Cc: Rob Clark 
Signed-off-by: Emil Velikov 
---
 common.c | 32 +---
 common.h |  2 ++
 configure.ac | 16 
 drm-common.c | 49 -
 4 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/common.c b/common.c
index 332e3e9..1d97c91 100644
--- a/common.c
+++ b/common.c
@@ -33,23 +33,33 @@
 
 static struct gbm gbm;
 
+WEAK struct gbm_surface *
+gbm_surface_create_with_modifiers(struct gbm_device *gbm,
+  uint32_t width, uint32_t height,
+  uint32_t format,
+  const uint64_t *modifiers,
+  const unsigned int count);
+
 const struct gbm * init_gbm(int drm_fd, int w, int h, uint64_t modifier)
 {
gbm.dev = gbm_create_device(drm_fd);
gbm.format = GBM_FORMAT_XRGB;
 
-#ifndef HAVE_GBM_MODIFIERS
-   if (modifier != DRM_FORMAT_MOD_LINEAR) {
-   fprintf(stderr, "Modifiers requested but support isn't 
available\n");
-   return NULL;
+   if (gbm_surface_create_with_modifiers) {
+   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
+   gbm.format,
+   , 1);
+
+   } else {
+   if (modifier != DRM_FORMAT_MOD_LINEAR) {
+   fprintf(stderr, "Modifiers requested but support isn't 
available\n");
+   return NULL;
+   }
+   gbm.surface = gbm_surface_create(gbm.dev, w, h,
+   gbm.format,
+   GBM_BO_USE_SCANOUT | 
GBM_BO_USE_RENDERING);
+
}
-   gbm.surface = gbm_surface_create(gbm.dev, w, h,
-   gbm.format,
-   GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING);
-#else
-   gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
-   gbm.format, , 1);
-#endif
 
if (!gbm.surface) {
printf("failed to create gbm surface\n");
diff --git a/common.h b/common.h
index dc87825..42eb236 100644
--- a/common.h
+++ b/common.h
@@ -57,6 +57,8 @@ EGLAPI EGLSurface EGLAPIENTRY 
eglCreatePlatformPixmapSurfaceEXT (EGLDisplay dpy,
 #endif
 #endif /* EGL_EXT_platform_base */
 
+#define WEAK __attribute__((weak))
+
 struct gbm {
struct gbm_device *dev;
struct gbm_surface *surface;
diff --git a/configure.ac b/configure.ac
index 8397f7b..5a63afb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,21 +49,5 @@ if test "x$HAVE_GST" = "xyes"; then
 fi
 AM_CONDITIONAL(ENABLE_GST, [test "x$HAVE_GST" = "xyes"])
 
-AC_CHECK_LIB([gbm], [gbm_bo_get_modifier], [gbm_modifiers=yes], [])
-
-AC_ARG_ENABLE([gbm-modifiers],
- [AS_HELP_STRING([--enable-gbm-modifiers],
- [enable using GBM modifiers @<:@default=auto@:>@])],
- [enable_gbm_modifiers="$enableval"],
- [enable_gbm_modifiers=auto])
-
-if test "x$enable_gbm_modifiers" = xyes -a "x$gbm_modifiers" != xyes; then
-   AC_MSG_ERROR([Cannot find gbm modifier supported mesa])
-fi
-
-if test "x$enable_gbm_modifiers" = xyes; then
-   AC_DEFINE(HAVE_GBM_MODIFIERS, 1, [Define if you can use GBM 
properties.])
-fi
-
 AC_CONFIG_FILES([Makefile])
 AC_OUTPUT
diff --git a/drm-common.c b/drm-common.c
index 1ec2820..136fe69 100644
--- a/drm-common.c
+++ b/drm-common.c
@@ -31,6 +31,18 @@
 #include "common.h"
 #include "drm-common.h"
 
+WEAK uint64_t
+gbm_bo_get_modifier(struct gbm_bo *bo);
+
+WEAK int
+gbm_bo_get_plane_count(struct gbm_bo *bo);
+
+WEAK uint32_t
+gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int plane);
+
+WEAK uint32_t
+gbm_bo_get_offset(struct gbm_bo *bo, int plane);
+
 static void
 drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
 {
@@ -62,26 +74,29 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
height = gbm_bo_get_height(bo);
format = gbm_bo_get_format(bo);
 
-#ifdef HAVE_GBM_MODIFIERS
-   uint64_t modifiers[4] = {0};
-   modifiers[0] = gbm_bo_get_modifier(bo);
-   const int num_planes = gbm_bo_get_plane_count(bo);
-   for (int i = 0; i < num_planes; i++) {
-   strides[i] = gbm_bo_get_stride_for_plane(bo, i);
-   handles[i] = gbm_bo_get_handle(bo).u32;
-   offsets[i] = gbm_bo_get_offset(bo, i);
-   modifiers[i] = modifiers[0];
-   }
+   if (gbm_bo_get_modifier && gbm_bo_get_plane_count &&
+