On Wednesday, 2018-02-07 23:17:48 -0500, Ilia Mirkin wrote:
> The GBM surface format has to match the DRM mode. Both are used in a
> couple of places, so move the defines to a common place so that they can
> be adjusted easily.
> 
> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
> ---
> 
> Tested with both RGB565 and ARGB2101010. Predictably the RGB565 cube has
> a lot of artifacts, while the 10bpc one avoids the occasional artifacts
> that show up at 8bpc.
> 
>  common.c     | 76 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  common.h     |  3 +++
>  drm-common.c |  4 ++--
>  3 files changed, 76 insertions(+), 7 deletions(-)
> 
> diff --git a/common.c b/common.c
> index b76c994..2ee3b85 100644
> --- a/common.c
> +++ b/common.c
> @@ -54,7 +54,7 @@ const struct gbm * init_gbm(int drm_fd, int w, int h, 
> uint64_t modifier)
>               return NULL;
>       }
>       gbm.surface = gbm_surface_create(gbm.dev, w, h,
> -                     GBM_FORMAT_XRGB8888,
> +                     GBM_FORMAT,
>                       GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING);
>  #else
>       uint64_t *mods;
> @@ -66,7 +66,7 @@ const struct gbm * init_gbm(int drm_fd, int w, int h, 
> uint64_t modifier)
>               count = get_modifiers(&mods);
>       }
>       gbm.surface = gbm_surface_create_with_modifiers(gbm.dev, w, h,
> -                     GBM_FORMAT_XRGB8888, mods, count);
> +                     GBM_FORMAT, mods, count);
>  #endif
>  
>       if (!gbm.surface) {
> @@ -100,9 +100,75 @@ static bool has_ext(const char *extension_list, const 
> char *ext)
>       }
>  }
>  
> +static int
> +match_config_to_visual(EGLDisplay egl_display,
> +                    EGLint visual_id,
> +                    EGLConfig *configs,
> +                    int count)
> +{
> +     int i;
> +
> +     for (i = 0; i < count; ++i) {
> +             EGLint id;
> +
> +             if (!eglGetConfigAttrib(egl_display,
> +                             configs[i], EGL_NATIVE_VISUAL_ID,
> +                             &id))
> +                     continue;
> +
> +             if (id == visual_id)
> +                     return i;
> +     }
> +
> +     return -1;
> +}
> +
> +static int
> +egl_choose_config(struct egl *egl, const EGLint *attribs, EGLint visual_id,

`display` is the only member of `egl` used; pass `egl->display` directly?

> +               EGLConfig *config_out)
> +{
> +     EGLint count = 0;
> +     EGLint matched = 0;
> +     EGLConfig *configs;
> +     int config_index = -1;
> +
> +     if (!eglGetConfigs(egl->display, NULL, 0, &count) || count < 1) {
> +             printf("No EGL configs to choose from.\n");
> +             return -1;
> +     }
> +     configs = calloc(count, sizeof *configs);

Nit: no need to calloc, malloc is enough; we'll never read past
how many configs eglChooseConfig() says it returned, and barring
any implementation bug, that should mean all indices up to `matched`
are initialised.

> +     if (!configs)
> +             return -1;
> +
> +     if (!eglChooseConfig(egl->display, attribs, configs,
> +                           count, &matched) || !matched) {
> +             printf("No EGL configs with appropriate attributes.\n");
> +             goto out;
> +     }
> +
> +     if (!visual_id)
> +             config_index = 0;
> +
> +     if (config_index == -1)
> +             config_index = match_config_to_visual(egl->display,
> +                                                   visual_id,
> +                                                   configs,
> +                                                   matched);
> +
> +     if (config_index != -1)
> +             *config_out = configs[config_index];
> +
> +out:
> +     free(configs);
> +     if (config_index == -1)
> +             return -1;
> +
> +     return 0;

can we make that `return true/false` ?

Other than these nit-picks, lgtm:
Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>

> +}
> +
>  int init_egl(struct egl *egl, const struct gbm *gbm)
>  {
> -     EGLint major, minor, n;
> +     EGLint major, minor;
>  
>       static const EGLint context_attribs[] = {
>               EGL_CONTEXT_CLIENT_VERSION, 2,
> @@ -174,8 +240,8 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
>               return -1;
>       }
>  
> -     if (!eglChooseConfig(egl->display, config_attribs, &egl->config, 1, &n) 
> || n != 1) {
> -             printf("failed to choose config: %d\n", n);
> +     if (egl_choose_config(egl, config_attribs, GBM_FORMAT, &egl->config)) {
> +             printf("failed to choose config\n");
>               return -1;
>       }
>  
> diff --git a/common.h b/common.h
> index 11ec26e..0c9cd62 100644
> --- a/common.h
> +++ b/common.h
> @@ -40,6 +40,9 @@
>  #define DRM_FORMAT_MOD_INVALID ((((__u64)0) << 56) | ((1ULL << 56) - 1))
>  #endif
>  
> +#define GBM_FORMAT GBM_FORMAT_XRGB8888
> +#define DRM_FORMAT DRM_FORMAT_XRGB8888
> +
>  #ifndef EGL_KHR_platform_gbm
>  #define EGL_KHR_platform_gbm 1
>  #define EGL_PLATFORM_GBM_KHR              0x31D7
> diff --git a/drm-common.c b/drm-common.c
> index 4b55745..f022eb0 100644
> --- a/drm-common.c
> +++ b/drm-common.c
> @@ -78,7 +78,7 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
>       }
>  
>       ret = drmModeAddFB2WithModifiers(drm_fd, width, height,
> -                     DRM_FORMAT_XRGB8888, handles, strides, offsets,
> +                     DRM_FORMAT, handles, strides, offsets,
>                       modifiers, &fb->fb_id, flags);
>  #endif
>       if (ret) {
> @@ -88,7 +88,7 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
>               memcpy(handles, (uint32_t 
> [4]){gbm_bo_get_handle(bo).u32,0,0,0}, 16);
>               memcpy(strides, (uint32_t [4]){gbm_bo_get_stride(bo),0,0,0}, 
> 16);
>               memset(offsets, 0, 16);
> -             ret = drmModeAddFB2(drm_fd, width, height, DRM_FORMAT_XRGB8888,
> +             ret = drmModeAddFB2(drm_fd, width, height, DRM_FORMAT,
>                               handles, strides, offsets, &fb->fb_id, 0);
>       }
>  
> -- 
> 2.13.6
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to