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

[Emil Velikov]

On 5 April 2018 at 14:44, Thierry Reding  wrote:
> From: Thierry Reding 
>
> 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 .
>
> Signed-off-by: Thierry Reding 
> ---
>  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_XRGB;
>
> -   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, , 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_XRGB)
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();
> +   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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Thierry Reding]

From: Thierry Reding 

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 .

Signed-off-by: Thierry Reding 
---
 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_XRGB;
 
-   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, , 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_XRGB)
+   format_index = i;
+   }
+
+   for (i = 0; i < drm->plane->props->count_props; i++) {
+   if (!strcmp(drm->plane->props_info[i]->name, "IN_FORMATS")) {
+   struct drm_format_modifier *mods;
+   drmModePropertyBlobPtr blob;
+   unsigned int count;
+
+   blob = drmModeGetPropertyBlob(drm->fd,
+ 
drm->plane->props->prop_values[i]);
+   if (!blob) {
+   printf("failed to get blob for property %s\n",
+  drm->plane->props_info[i]->name);
+   return -ENOMEM;
+   }
+
+   count = get_modifiers(blob, );
+
+   for (j = 0; j < count; j++) {
+   if (mods[j].formats & (1ULL << format_index))
+   drm->num_modifiers++;
+   }
+
+   drm->modifiers = calloc(drm->num_modifiers,
+   sizeof(uint64_t));
+   if (!drm->modifiers) {
+   printf("failed to allocate modifiers\n");
+   return -ENOMEM;
+   }
+
+   for (j = 0; j < count; j++) {
+   if (mods[j].formats & (1ULL << format_index))
+   drm->modifiers[j] = mods[j].modifier;
+   }
+   }
+   }
+
+   return 0;
+}
+
+const struct drm *init_drm_atomic(const char *device, uint64_t *modifiers,
+ unsigned int num_modifiers)
 {
uint32_t plane_id;
int ret;
@@ -346,6 +405,9 @@ const struct drm * init_drm_atomic(const char *device)
if (ret)
return NULL;
 
+   drm.num_modifiers = num_modifiers;
+