[Mesa-dev] [PATCH mesa 3/6] vulkan/wsi/display: pass the plane's modifiers to the image

[andrey simiklit]

Hi,

Please find my comment below:

Regards,
Andrii.

On Wed, Sep 26, 2018 at 6:39 PM Eric Engestrom <eric.engestrom at intel.com>
wrote:

> Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> ---
>  src/vulkan/wsi/wsi_common_display.c | 95 ++++++++++++++++++++++++++++-
>  1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c
> b/src/vulkan/wsi/wsi_common_display.c
> index 6c9160a445c8f25a8ad5..2a30b1139f06cbb56769 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -958,6 +958,87 @@ wsi_display_destroy_buffer(struct wsi_display *wsi,
>                     &((struct drm_mode_destroy_dumb) { .handle = buffer
> }));
>  }
>
> +static uint64_t*
> +wsi_get_modifiers_for_format(const struct wsi_display * const wsi,
> +                             const uint32_t plane_id,
> +                             const uint32_t drm_format,
> +                             uint32_t * const modifiers_count)
> +{
> +   /* Get the properties of the plane */
> +   drmModeObjectProperties *props =
> +      drmModeObjectGetProperties(wsi->fd, plane_id,
> +                                 DRM_MODE_OBJECT_PLANE);
> +   if (!props)
> +      return NULL;
> +
> +   /* Find the blob the contains the formats and their modifiers */
> +   uint32_t blob_id = 0;
> +   for (size_t i = 0; i< props->count_props; i++) {
> +      const drmModePropertyPtr prop =
> +         drmModeGetProperty(wsi->fd, props->props[i]);
> +
> +      if (!strcmp(prop->name, "IN_FORMATS")) {
> +         blob_id = props->prop_values[i];
> +         drmModeFreeProperty(prop);
> +         break;
> +      }
> +
> +      drmModeFreeProperty(prop);
> +   }
> +
> +   /* Property not found, which means old kernel, so definitely no
> +    * modifiers support */
> +   if (blob_id == 0)
> +      return NULL;
> +
> +   /* Grab the IN_FORMATS blob */
> +   drmModePropertyBlobRes *blob = drmModeGetPropertyBlob(wsi->fd,
> blob_id);
> +   if (!blob)
> +      return NULL;
> +
> +   /* Get the formats and modifiers out of the blob */
> +   struct drm_format_modifier_blob *fmt_mod_blob = blob->data;
> +   uint32_t *blob_formats = (uint32_t*)((char*)fmt_mod_blob +
> +                                        fmt_mod_blob->formats_offset);
> +   struct drm_format_modifier *blob_modifiers =
> +      (struct drm_format_modifier *)((char*)fmt_mod_blob +
> +                                     fmt_mod_blob->modifiers_offset);
> +
> +   /* Find the format we care about in the list */
> +   size_t format_index = 0;
> +   for (size_t i = 0; i < fmt_mod_blob->count_formats; i++) {
> +      if (blob_formats[i] == drm_format) {
> +         format_index = i;
> +         break;
> +      }
> +   }
> +
> +   /* Get the list of modifiers supported by that format */
> +   uint32_t count_modifiers = 0;
> +   uint64_t *modifiers = NULL;
> +   for (size_t i = 0; i < fmt_mod_blob->count_modifiers; i++) {
> +      struct drm_format_modifier *mod = &blob_modifiers[i];
> +
> +      if ((format_index < mod->offset) || (format_index > mod->offset +
> 63))
> +         continue;
> +      if (!(mod->formats & (1 << (format_index - mod->offset))))
>

Would it be better to use '1ull << ... ' here?
The 'formats' filed is defined as '__u64 formats'.

+         continue;
> +
> +      modifiers = realloc(modifiers,
> +                          (count_modifiers + 1) *
> +                          sizeof(modifiers[0]));
> +      assert(modifiers);
> +      modifiers[count_modifiers++] = mod->modifier;
> +   }
> +
> +   drmModeFreePropertyBlob(blob);
> +
> +   drmModeFreeObjectProperties(props);
> +
> +   *modifiers_count = count_modifiers;
> +   return modifiers;
> +}
> +
>  static VkResult
>  wsi_display_image_init(VkDevice device_h,
>                         struct wsi_swapchain *drv_chain,
> @@ -969,6 +1050,10 @@ wsi_display_image_init(VkDevice device_h,
>        (struct wsi_display_swapchain *) drv_chain;
>     struct wsi_display *wsi = chain->wsi;
>     uint32_t drm_format = 0;
> +   VkIcdSurfaceDisplay *surface = chain->surface;
> +   wsi_display_mode *display_mode =
> +      wsi_display_mode_from_handle(surface->displayMode);
> +   wsi_display_connector *connector = display_mode->connector;
>
>     for (unsigned i = 0; i < ARRAY_SIZE(available_surface_formats); i++) {
>        if (create_info->imageFormat ==
> available_surface_formats[i].format) {
> @@ -981,9 +1066,17 @@ wsi_display_image_init(VkDevice device_h,
>     if (drm_format == 0)
>        return VK_ERROR_DEVICE_LOST;
>
> +   uint32_t modifiers_count = 0;
> +   uint64_t *modifiers = wsi_get_modifiers_for_format(wsi,
> connector->plane_id, drm_format,
> +                                                      &modifiers_count);
> +   assume(!modifiers || modifiers_count > 0);
> +
>     VkResult result = wsi_create_native_image(&chain->base, create_info,
> -                                             0, NULL, NULL,
> +                                             !!modifiers_count,
> +                                             &modifiers_count,
> +                                             (const uint64_t * const
> *)&modifiers,
>                                               &image->base);
> +   free(modifiers);
>     if (result != VK_SUCCESS)
>        return result;
>
> --
> Cheers,
>   Eric
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180927/7550da36/attachment-0001.html>