[Mesa-dev] [PATCH 3/3] panfrost: Add support for modifiers

Tomeu Vizoso tomeu.vizoso at collabora.com
Thu Jul 4 08:47:02 UTC 2019


On Thu, 4 Jul 2019 at 10:05, Tomeu Vizoso <tomeu.vizoso at collabora.com> wrote:
>
> Implement query_dmabuf_modifiers and resource_create_with_modifiers so
> Wayland clients can share AFBC buffers with the compositor.
>
> For now this is guarded behind the PAN_MESA_DEBUG=modifiers env var
> because implementing those callbacks causes Weston to try to pass
> modifiers to the Rockchip KMS driver, which currently doesn't support
> modifiers, thus failing the modeset.
>
> This has been fixed in Weston 6.0, so we can enable unconditionally once
> we are confident that most people testing panfrost have upgraded.
>
> This lays the ground for scanning out AFBC buffers, if the KMS driver
> supports it.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> ---
>  src/gallium/drivers/panfrost/pan_drm.c      |  1 +
>  src/gallium/drivers/panfrost/pan_resource.c | 74 ++++++++++++++++++---
>  src/gallium/drivers/panfrost/pan_screen.c   | 37 +++++++++++
>  src/gallium/drivers/panfrost/pan_util.h     |  7 ++
>  4 files changed, 108 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_drm.c b/src/gallium/drivers/panfrost/pan_drm.c
> index 9648ac1d452d..623793a84411 100644
> --- a/src/gallium/drivers/panfrost/pan_drm.c
> +++ b/src/gallium/drivers/panfrost/pan_drm.c
> @@ -26,6 +26,7 @@
>  #include <fcntl.h>
>  #include <xf86drm.h>
>
> +#include "drm-uapi/drm_fourcc.h"
>  #include "drm-uapi/panfrost_drm.h"
>
>  #include "util/u_memory.h"
> diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
> index 54497d3de2bb..d901d43168fb 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.c
> +++ b/src/gallium/drivers/panfrost/pan_resource.c
> @@ -34,6 +34,7 @@
>  #include "drm-uapi/drm_fourcc.h"
>
>  #include "state_tracker/winsys_handle.h"
> +#include "util/u_drm.h"
>  #include "util/u_format.h"
>  #include "util/u_memory.h"
>  #include "util/u_surface.h"
> @@ -91,6 +92,18 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen,
>
>         rsc->bo = panfrost_drm_import_bo(screen, whandle->handle);
>
> +       switch(whandle->modifier) {
> +       case DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP):
> +               rsc->layout = PAN_AFBC;
> +               break;
> +       case DRM_FORMAT_MOD_LINEAR:
> +               rsc->layout = PAN_LINEAR;
> +               break;
> +       default:
> +                fprintf(stderr, "unknown modifier: 0x%"PRIx64"\n", whandle->modifier);
> +               assert(0);
> +       }
> +
>          size_t bo_size;
>          panfrost_setup_slices(rsc, &bo_size);
>          assert(bo_size == rsc->bo->size);
> @@ -106,6 +119,21 @@ panfrost_resource_from_handle(struct pipe_screen *pscreen,
>          return prsc;
>  }
>
> +static uint64_t
> +panfrost_resource_modifier(struct panfrost_resource *rsrc)
> +{
> +        switch (rsrc->layout) {
> +        case PAN_AFBC:
> +                return DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP);
> +        case PAN_TILED:
> +                return DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED;
> +        case PAN_LINEAR:
> +                return DRM_FORMAT_MOD_LINEAR;
> +        default:
> +                return DRM_FORMAT_MOD_INVALID;
> +        }
> +}
> +
>  static boolean
>  panfrost_resource_get_handle(struct pipe_screen *pscreen,
>                               struct pipe_context *ctx,
> @@ -117,7 +145,7 @@ panfrost_resource_get_handle(struct pipe_screen *pscreen,
>          struct panfrost_resource *rsrc = (struct panfrost_resource *) pt;
>          struct renderonly_scanout *scanout = rsrc->scanout;
>
> -        handle->modifier = DRM_FORMAT_MOD_INVALID;
> +        handle->modifier = panfrost_resource_modifier(rsrc);
>
>         if (handle->type == WINSYS_HANDLE_TYPE_SHARED) {
>                 return FALSE;
> @@ -341,7 +369,9 @@ panfrost_setup_slices(struct panfrost_resource *pres, size_t *bo_size)
>  }
>
>  static void
> -panfrost_resource_create_bo(struct panfrost_screen *screen, struct panfrost_resource *pres)
> +panfrost_resource_create_bo(struct panfrost_screen *screen,
> +                            struct panfrost_resource *pres,
> +                            const uint64_t *modifiers, int count)
>  {
>         struct pipe_resource *res = &pres->base;
>
> @@ -362,14 +392,19 @@ panfrost_resource_create_bo(struct panfrost_screen *screen, struct panfrost_reso
>
>          /* Tiling textures is almost always faster, unless we only use it once */
>
> +        bool can_afbc = panfrost_format_supports_afbc(res->format);
> +        bool want_afbc = drm_find_modifier(DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP), modifiers, count);
> +        bool want_linear = drm_find_modifier(DRM_FORMAT_MOD_LINEAR, modifiers, count);
> +
>  #define SCANOUT (PIPE_BIND_SCANOUT | PIPE_BIND_SHARED | PIPE_BIND_DISPLAY_TARGET)
>
>          bool is_scanout = (res->bind & SCANOUT);
>          bool is_texture = (res->bind & PIPE_BIND_SAMPLER_VIEW);
> +        bool is_renderable = (res->bind & PIPE_BIND_RENDER_TARGET);
>          bool is_2d = res->depth0 == 1 && res->array_size == 1;
>          bool is_streaming = (res->usage == PIPE_USAGE_STREAM);
>
> -        bool should_tile = !is_streaming && is_texture && is_2d && !is_scanout;
> +        bool should_tile = !is_streaming && is_texture && is_2d && !want_linear && !is_scanout;
>
>          /* Depth/stencil can't be tiled, only linear or AFBC */
>          should_tile &= !(res->bind & PIPE_BIND_DEPTH_STENCIL);
> @@ -381,17 +416,20 @@ panfrost_resource_create_bo(struct panfrost_screen *screen, struct panfrost_reso
>          pres->checksummed = can_checksum && should_checksum;
>
>          /* Set the layout appropriately */
> -        pres->layout = should_tile ? PAN_TILED : PAN_LINEAR;
> +       if (want_afbc || (is_renderable && can_afbc && !is_texture))
> +                pres->layout = PAN_AFBC;

We regress here because we seem to have some bugs regarding AFBC usage
with some formats such as rgba4 and rgb5_a1:

https://gitlab.freedesktop.org/tomeu/mesa/-/jobs/399237

I'm looking at only enabling AFBC when explicitly asked to by the winsys.

Cheers,

Tomeu

> +        else
> +                pres->layout = should_tile ? PAN_TILED : PAN_LINEAR;
>
>          size_t bo_size;
> -
>          panfrost_setup_slices(pres, &bo_size);
>          pres->bo = panfrost_drm_create_bo(screen, bo_size, 0);
>  }
>
>  static struct pipe_resource *
> -panfrost_resource_create(struct pipe_screen *screen,
> -                         const struct pipe_resource *template)
> +panfrost_resource_create_with_modifiers(struct pipe_screen *screen,
> +                                        const struct pipe_resource *template,
> +                                        const uint64_t *modifiers, int count)
>  {
>          /* Make sure we're familiar */
>          switch (template->target) {
> @@ -418,13 +456,16 @@ panfrost_resource_create(struct pipe_screen *screen,
>
>          util_range_init(&so->valid_buffer_range);
>
> -        panfrost_resource_create_bo(pscreen, so);
> +        panfrost_resource_create_bo(pscreen, so, modifiers, count);
>
>          /* Set up the "scanout resource" (the dmabuf export of our buffer to
>           * the KMS handle) if the buffer might ever have
>           * resource_get_handle(WINSYS_HANDLE_TYPE_KMS) called on it.
> +         * create_with_modifiers() doesn't give us usage flags, so we have to
> +         * assume that all calls with modifiers are scanout-possible.
>           */
> -        if (template->bind & PIPE_BIND_SCANOUT) {
> +        if (((template->bind & PIPE_BIND_SCANOUT) ||
> +             !(count == 1 && modifiers[0] == DRM_FORMAT_MOD_INVALID))) {
>                  so->scanout =
>                          renderonly_scanout_for_resource(&so->base, pscreen->ro, NULL);
>                  if (!so->scanout) {
> @@ -436,6 +477,14 @@ panfrost_resource_create(struct pipe_screen *screen,
>          return (struct pipe_resource *)so;
>  }
>
> +static struct pipe_resource *
> +panfrost_resource_create(struct pipe_screen *screen,
> +                         const struct pipe_resource *template)
> +{
> +       const uint64_t mod = DRM_FORMAT_MOD_INVALID;
> +       return panfrost_resource_create_with_modifiers(screen, template, &mod, 1);
> +}
> +
>  void
>  panfrost_bo_reference(struct panfrost_bo *bo)
>  {
> @@ -779,8 +828,11 @@ static const struct u_transfer_vtbl transfer_vtbl = {
>  void
>  panfrost_resource_screen_init(struct panfrost_screen *pscreen)
>  {
> -        //pscreen->base.resource_create_with_modifiers =
> -        //        panfrost_resource_create_with_modifiers;
> +        if (pan_debug & PAN_DBG_MODIFIERS) {
> +                pscreen->base.resource_create_with_modifiers =
> +                        panfrost_resource_create_with_modifiers;
> +        }
> +
>          pscreen->base.resource_create = u_transfer_helper_resource_create;
>          pscreen->base.resource_destroy = u_transfer_helper_resource_destroy;
>          pscreen->base.resource_from_handle = panfrost_resource_from_handle;
> diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c
> index d53a906838eb..02b65e068c30 100644
> --- a/src/gallium/drivers/panfrost/pan_screen.c
> +++ b/src/gallium/drivers/panfrost/pan_screen.c
> @@ -57,6 +57,7 @@
>  static const struct debug_named_value debug_options[] = {
>         {"msgs",      PAN_DBG_MSGS,     "Print debug messages"},
>         {"trace",     PAN_DBG_TRACE,    "Trace the command stream"},
> +       {"modifiers", PAN_DBG_MODIFIERS,"Enable modifiers support"},
>         DEBUG_NAMED_VALUE_END
>  };
>
> @@ -559,6 +560,38 @@ panfrost_screen_get_compiler_options(struct pipe_screen *pscreen,
>          return &midgard_nir_options;
>  }
>
> +const uint64_t supported_modifiers[] = {
> +   DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_ROCKCHIP),
> +   DRM_FORMAT_MOD_LINEAR,
> +};
> +
> +static void
> +panfrost_query_dmabuf_modifiers(struct pipe_screen *pscreen,
> +                                enum pipe_format format, int max,
> +                                uint64_t *modifiers,
> +                                unsigned int *external_only, int *count)
> +{
> +   int i, num_modifiers = 0;
> +
> +   if (max > ARRAY_SIZE(supported_modifiers))
> +      max = ARRAY_SIZE(supported_modifiers);
> +
> +   if (!max) {
> +      modifiers = NULL;
> +      max = ARRAY_SIZE(supported_modifiers);
> +   }
> +
> +   for (i = 0; num_modifiers < max; i++) {
> +      if (modifiers)
> +         modifiers[num_modifiers] = supported_modifiers[i];
> +      if (external_only)
> +         external_only[num_modifiers] = util_format_is_yuv(format) ? 1 : 0;
> +      num_modifiers++;
> +   }
> +
> +   *count = num_modifiers;
> +}
> +
>  struct pipe_screen *
>  panfrost_create_screen(int fd, struct renderonly *ro)
>  {
> @@ -599,6 +632,10 @@ panfrost_create_screen(int fd, struct renderonly *ro)
>          screen->base.fence_reference = panfrost_fence_reference;
>          screen->base.fence_finish = panfrost_fence_finish;
>
> +        if (pan_debug & PAN_DBG_MODIFIERS) {
> +                screen->base.query_dmabuf_modifiers = panfrost_query_dmabuf_modifiers;
> +        }
> +
>         screen->last_fragment_flushed = true;
>          screen->last_job = NULL;
>
> diff --git a/src/gallium/drivers/panfrost/pan_util.h b/src/gallium/drivers/panfrost/pan_util.h
> index 8fd41420a486..a3a7ec3067e6 100644
> --- a/src/gallium/drivers/panfrost/pan_util.h
> +++ b/src/gallium/drivers/panfrost/pan_util.h
> @@ -30,6 +30,7 @@
>
>  #define PAN_DBG_MSGS           0x0001
>  #define PAN_DBG_TRACE           0x0002
> +#define PAN_DBG_MODIFIERS       0x0004
>
>  extern int pan_debug;
>
> @@ -38,4 +39,10 @@ extern int pan_debug;
>                         fprintf(stderr, "%s:%d: "fmt, \
>                                 __FUNCTION__, __LINE__, ##__VA_ARGS__); } while (0)
>
> +/* TODO: Pick these two up from kernel header */
> +#define AFBC_FORMAT_MOD_ROCKCHIP (1ULL <<  20)
> +
> +#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
> +       fourcc_mod_code(ARM, ((1ULL << 55) | 1))
> +
>  #endif /* PAN_UTIL_H */
> --
> 2.20.1
>


More information about the mesa-dev mailing list