[Mesa-dev] [PATCH kmscube 3/4] Automatically select modifiers

[Emil Velikov]

On 5 April 2018 at 14:44, Thierry Reding <thierry.reding at gmail.com> wrote:
> From: Thierry Reding <treding at nvidia.com>
>
> If available, use the formats/modifiers blob from a DRM/KMS device to
> automatically detect which modifiers to use. In the absence of the blob,
> leave it up to the implementation to choose an appropriate format.
>
> Based on work by Lucas Stach <l.stach at pengutronix.de>.
>
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
>  common.c     | 10 +++++----
>  common.h     |  2 +-
>  drm-atomic.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  drm-common.h |  5 ++++-
>  kmscube.c    |  8 +++++--
>  5 files changed, 86 insertions(+), 9 deletions(-)
>
> diff --git a/common.c b/common.c
> index 3dcd9bd3d8f4..a886962ea7b1 100644
> --- a/common.c
> +++ b/common.c
> @@ -41,17 +41,19 @@ gbm_surface_create_with_modifiers(struct gbm_device *gbm,
>                                    const uint64_t *modifiers,
>                                    const unsigned int count);
>
> -const struct gbm * init_gbm(const struct drm *drm, uint64_t modifier)
> +const struct gbm * init_gbm(const struct drm *drm)
>  {
>         gbm.dev = gbm_create_device(drm->fd);
>         gbm.format = GBM_FORMAT_XRGB8888;
>
> -       if (gbm_surface_create_with_modifiers) {
> +       if (gbm_surface_create_with_modifiers && drm->num_modifiers > 0) {
>                 gbm.surface = gbm_surface_create_with_modifiers(gbm.dev,
>                                 drm->mode->hdisplay, drm->mode->vdisplay,
> -                               gbm.format, &modifier, 1);
> +                               gbm.format, drm->modifiers,
> +                               drm->num_modifiers);
>         } else {
> -               if (modifier != DRM_FORMAT_MOD_LINEAR) {
> +               if (drm->num_modifiers > 0 &&
> +                   drm->modifiers[0] != DRM_FORMAT_MOD_LINEAR) {
>                         fprintf(stderr, "Modifiers requested but support isn't available\n");
>                         return NULL;
>                 }
> diff --git a/common.h b/common.h
> index 8ff1ed3a6aa3..bed316786557 100644
> --- a/common.h
> +++ b/common.h
> @@ -84,7 +84,7 @@ struct gbm {
>  };
>
>  struct drm;
> -const struct gbm *init_gbm(const struct drm *drm, uint64_t modifier);
> +const struct gbm *init_gbm(const struct drm *drm);
>
>  struct egl {
>         EGLDisplay display;
> diff --git a/drm-atomic.c b/drm-atomic.c
> index 99ac33d6a686..a68f036a9aab 100644
> --- a/drm-atomic.c
> +++ b/drm-atomic.c
> @@ -337,7 +337,66 @@ static int get_plane_id(void)
>         return ret;
>  }
>
> -const struct drm * init_drm_atomic(const char *device)
> +static unsigned int
> +get_modifiers(drmModePropertyBlobPtr blob, struct drm_format_modifier **modsp)
> +{
> +       struct drm_format_modifier_blob *data = blob->data;
> +
> +       *modsp = blob->data + data->modifiers_offset;
> +
> +       return data->count_modifiers;
> +}
> +
> +static int
> +drm_atomic_get_modifiers(struct drm *drm)
> +{
> +       unsigned int i, j, format_index = 0;
> +
> +       for (i = 0; i < drm->plane->plane->count_formats; i++) {
> +               if (drm->plane->plane->formats[i] == DRM_FORMAT_XRGB8888)
Use gbm.format? The {DRM,GBM}_FORMAT defines are identical.

> +                       format_index = i;
Shouldn't we bail out if no plane has the requested format?


> @@ -402,6 +464,12 @@ const struct drm * init_drm_atomic(const char *device)
>         get_properties(crtc, CRTC, drm.crtc_id);
>         get_properties(connector, CONNECTOR, drm.connector_id);
>
> +       if (num_modifiers == 0) {
> +               ret = drm_atomic_get_modifiers(&drm);
> +               if (ret < 0)
> +                       return NULL;
Just print an {info,error} message and continue. As-is this will break
the (very odd) legacy case.

With the above suggestions (or some clarification) the series is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil