[Mesa-dev] [PATCH v13 11/36] st/dri: implement createImageWithModifiers in DRIimage

[Emil Velikov]

On 19 May 2017 at 10:37, Daniel Stone <daniels at collabora.com> wrote:
> From: Varad Gautam <varad.gautam at collabora.com>
>
> adds a pscreen->resource_create_with_modifiers() to create textures
> with modifier.
>
> Signed-off-by: Varad Gautam <varad.gautam at collabora.com>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/gallium/include/pipe/p_screen.h   | 18 ++++++++
>  src/gallium/state_trackers/dri/dri2.c | 79 ++++++++++++++++++++++++++++-------
>  2 files changed, 81 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
> index 8b4239c61a..8eddf355d7 100644
> --- a/src/gallium/include/pipe/p_screen.h
> +++ b/src/gallium/include/pipe/p_screen.h
> @@ -328,6 +328,24 @@ struct pipe_screen {
>      * driver doesn't support an on-disk shader cache.
>      */
>     struct disk_cache *(*get_disk_shader_cache)(struct pipe_screen *screen);
> +
> +   /**
> +    * Create a new texture object from the given template info, taking
> +    * format modifiers into account. \p modifiers specifies a list of format
> +    * modifier tokens, as defined in drm_fourcc.h. The driver then picks the
> +    * best modifier among these and creates the resource. \p count must
> +    * contain the size of \p modifiers array. The selected modifier is
> +    * returned via \p modifier after a successful call.
> +    *
> +    * Returns NULL if an entry in \p modifiers is unsupported by the driver,
> +    * or if only DRM_FORMAT_MOD_INVALID is provided.
> +    */
> +   struct pipe_resource * (*resource_create_with_modifiers)(
> +                           struct pipe_screen *,
Style: move this to previous line.


> diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
> index 7614474e4a..713f482181 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -977,27 +977,38 @@ dri2_create_image_from_renderbuffer(__DRIcontext *context,

>     tex_usage = PIPE_BIND_RENDER_TARGET | PIPE_BIND_SAMPLER_VIEW;
> -   if (use & __DRI_IMAGE_USE_SCANOUT)
> -      tex_usage |= PIPE_BIND_SCANOUT;
> -   if (use & __DRI_IMAGE_USE_SHARE)
> -      tex_usage |= PIPE_BIND_SHARED;
> -   if (use & __DRI_IMAGE_USE_LINEAR)
> -      tex_usage |= PIPE_BIND_LINEAR;
> -   if (use & __DRI_IMAGE_USE_CURSOR) {
> -      if (width != 64 || height != 64)
> -         return NULL;
> -      tex_usage |= PIPE_BIND_CURSOR;
> +   if (use) {
> +      if (use & __DRI_IMAGE_USE_SCANOUT)
> +         tex_usage |= PIPE_BIND_SCANOUT;
> +      if (use & __DRI_IMAGE_USE_SHARE)
> +         tex_usage |= PIPE_BIND_SHARED;
> +      if (use & __DRI_IMAGE_USE_LINEAR)
> +         tex_usage |= PIPE_BIND_LINEAR;
> +      if (use & __DRI_IMAGE_USE_CURSOR) {
> +         if (width != 64 || height != 64)
> +            return NULL;
> +         tex_usage |= PIPE_BIND_CURSOR;
> +      }
Unrelated micro optimisation?

>     }
>
>     pf = dri2_format_to_pipe_format (format);
> @@ -1018,7 +1029,16 @@ dri2_create_image(__DRIscreen *_screen,
>     templ.depth0 = 1;
>     templ.array_size = 1;
>
> -   img->texture = screen->base.screen->resource_create(screen->base.screen, &templ);
> +   if (modifiers)
> +      img->texture = screen->base.screen
> +                        ->resource_create_with_modifiers(screen->base.screen,
> +                                                         &templ,
> +                                                         modifiers,
> +                                                         count,
> +                                                         &modifier);
> +   else
> +      img->texture = screen->base.screen
> +                        ->resource_create(screen->base.screen, &templ);
Style:
   foo =
      screen->.....

>     if (!img->texture) {
>        FREE(img);
>        return NULL;
> @@ -1029,12 +1049,34 @@ dri2_create_image(__DRIscreen *_screen,
>     img->dri_format = format;
>     img->dri_components = 0;
>     img->use = use;
> -   img->modifier = DRM_FORMAT_MOD_INVALID;
> +   img->modifier = modifier;
>
>     img->loader_private = loaderPrivate;
>     return img;
>  }
>
> +static __DRIimage *
> +dri2_create_image(__DRIscreen *_screen,
> +                   int width, int height, int format,
> +                   unsigned int use, void *loaderPrivate)
> +{
> +   return dri2_create_image_common(_screen, width, height, format, use,
> +                                   NULL /* modifiers */, 0 /* count */,
> +                                   loaderPrivate);
> +}
> +
> +static __DRIimage *
> +dri2_create_image_with_modifiers(__DRIscreen *dri_screen,
> +                                 int width, int height, int format,
> +                                 const uint64_t *modifiers,
> +                                 const unsigned count,
> +                                 void *loaderPrivate)
> +{
> +   return dri2_create_image_common(dri_screen, width, height, format,
> +                                   0 /* use */, modifiers, count,
> +                                   loaderPrivate);
> +}
> +
>  static GLboolean
>  dri2_query_image(__DRIimage *image, int attrib, int *value)
>  {
> @@ -1432,7 +1474,7 @@ dri2_get_capabilities(__DRIscreen *_screen)
>
>  /* The extension is modified during runtime if DRI_PRIME is detected */
>  static __DRIimageExtension dri2ImageExtension = {
> -    .base = { __DRI_IMAGE, 12 },
> +    .base = { __DRI_IMAGE, 14 },
I'm split whether setting the version here or alongside the function
pointer is better.

>
>      .createImageFromName          = dri2_create_image_from_name,
>      .createImageFromRenderbuffer  = dri2_create_image_from_renderbuffer,
> @@ -1450,6 +1492,7 @@ static __DRIimageExtension dri2ImageExtension = {
>      .getCapabilities              = dri2_get_capabilities,
>      .mapImage                     = dri2_map_image,
>      .unmapImage                   = dri2_unmap_image,
> +    .createImageWithModifiers     = NULL,
No need - C99 dictates that this will be NULL.

-Emil