OSDN Git Service

minigbm: dri,amdgpu: Modifier support.
authorChromeOS Developer <basni@chromium.org>
Mon, 2 Mar 2020 12:08:53 +0000 (13:08 +0100)
committerCommit Bot <commit-bot@chromium.org>
Tue, 10 Mar 2020 15:39:10 +0000 (15:39 +0000)
This adds modifier support to minigbm in the amdgpu driver.

This includes

(a) creation of images with modifiers
(b) imports
  - Had to distinguish between legacy and non-legacy.
(c) Support for planes > format planes

BUG=b:149819940
TEST=Login on a Zork device + play a YT video.

Change-Id: If58ada081aa254932e299536c48c18937266c2e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2093212
Tested-by: Bas Nieuwenhuizen <basni@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Bas Nieuwenhuizen <basni@chromium.org>

amdgpu.c
dri.c
dri.h
gbm.c

index 8670087..bb9342e 100644 (file)
--- a/amdgpu.c
+++ b/amdgpu.c
@@ -142,14 +142,49 @@ static void amdgpu_close(struct driver *drv)
        drv->priv = NULL;
 }
 
-static int amdgpu_create_bo(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
-                           uint64_t use_flags)
+static int amdgpu_create_bo_linear(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
+                                  uint64_t use_flags)
 {
        int ret;
        uint32_t plane, stride;
-       struct combination *combo;
        union drm_amdgpu_gem_create gem_create;
 
+       stride = drv_stride_from_format(format, width, 0);
+       stride = ALIGN(stride, 256);
+
+       drv_bo_from_format(bo, stride, height, format);
+
+       memset(&gem_create, 0, sizeof(gem_create));
+       gem_create.in.bo_size = bo->meta.total_size;
+       gem_create.in.alignment = 256;
+       gem_create.in.domain_flags = 0;
+
+       if (use_flags & (BO_USE_LINEAR | BO_USE_SW_MASK))
+               gem_create.in.domain_flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+
+       gem_create.in.domains = AMDGPU_GEM_DOMAIN_GTT;
+       if (!(use_flags & (BO_USE_SW_READ_OFTEN | BO_USE_SCANOUT)))
+               gem_create.in.domain_flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+
+       /* Allocate the buffer with the preferred heap. */
+       ret = drmCommandWriteRead(drv_get_fd(bo->drv), DRM_AMDGPU_GEM_CREATE, &gem_create,
+                                 sizeof(gem_create));
+       if (ret < 0)
+               return ret;
+
+       for (plane = 0; plane < bo->meta.num_planes; plane++)
+               bo->handles[plane].u32 = gem_create.out.handle;
+
+       bo->meta.format_modifiers[0] = DRM_FORMAT_MOD_LINEAR;
+
+       return 0;
+}
+
+static int amdgpu_create_bo(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
+                           uint64_t use_flags)
+{
+       struct combination *combo;
+
        combo = drv_get_combination(bo->drv, format, use_flags);
        if (!combo)
                return -EINVAL;
@@ -179,45 +214,38 @@ static int amdgpu_create_bo(struct bo *bo, uint32_t width, uint32_t height, uint
                return dri_bo_create(bo, width, height, format, use_flags);
        }
 
-       stride = drv_stride_from_format(format, width, 0);
-       stride = ALIGN(stride, 256);
-
-       drv_bo_from_format(bo, stride, height, format);
-
-       memset(&gem_create, 0, sizeof(gem_create));
-       gem_create.in.bo_size = bo->meta.total_size;
-       gem_create.in.alignment = 256;
-       gem_create.in.domain_flags = 0;
-
-       if (use_flags & (BO_USE_LINEAR | BO_USE_SW_MASK))
-               gem_create.in.domain_flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
-
-       gem_create.in.domains = AMDGPU_GEM_DOMAIN_GTT;
-       if (!(use_flags & (BO_USE_SW_READ_OFTEN | BO_USE_SCANOUT)))
-               gem_create.in.domain_flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+       return amdgpu_create_bo_linear(bo, width, height, format, use_flags);
+}
 
-       /* Allocate the buffer with the preferred heap. */
-       ret = drmCommandWriteRead(drv_get_fd(bo->drv), DRM_AMDGPU_GEM_CREATE, &gem_create,
-                                 sizeof(gem_create));
-       if (ret < 0)
-               return ret;
+static int amdgpu_create_bo_with_modifiers(struct bo *bo, uint32_t width, uint32_t height,
+                                          uint32_t format, const uint64_t *modifiers,
+                                          uint32_t count)
+{
+       bool only_use_linear = true;
 
-       for (plane = 0; plane < bo->meta.num_planes; plane++)
-               bo->handles[plane].u32 = gem_create.out.handle;
+       for (uint32_t i = 0; i < count; ++i)
+               if (modifiers[i] != DRM_FORMAT_MOD_LINEAR)
+                       only_use_linear = false;
 
-       bo->meta.format_modifiers[0] = DRM_FORMAT_MOD_LINEAR;
+       if (only_use_linear)
+               return amdgpu_create_bo_linear(bo, width, height, format, BO_USE_SCANOUT);
 
-       return 0;
+       return dri_bo_create_with_modifiers(bo, width, height, format, modifiers, count);
 }
 
 static int amdgpu_import_bo(struct bo *bo, struct drv_import_fd_data *data)
 {
-       struct combination *combo;
-       combo = drv_get_combination(bo->drv, data->format, data->use_flags);
-       if (!combo)
-               return -EINVAL;
+       bool dri_tiling = data->format_modifiers[0] != DRM_FORMAT_MOD_LINEAR;
+       if (data->format_modifiers[0] == DRM_FORMAT_MOD_INVALID) {
+               struct combination *combo;
+               combo = drv_get_combination(bo->drv, data->format, data->use_flags);
+               if (!combo)
+                       return -EINVAL;
+
+               dri_tiling = combo->metadata.tiling == TILE_TYPE_DRI;
+       }
 
-       if (combo->metadata.tiling == TILE_TYPE_DRI)
+       if (dri_tiling)
                return dri_bo_import(bo, data);
        else
                return drv_prime_bo_import(bo, data);
@@ -309,6 +337,7 @@ const struct backend backend_amdgpu = {
        .init = amdgpu_init,
        .close = amdgpu_close,
        .bo_create = amdgpu_create_bo,
+       .bo_create_with_modifiers = amdgpu_create_bo_with_modifiers,
        .bo_destroy = amdgpu_destroy_bo,
        .bo_import = amdgpu_import_bo,
        .bo_map = amdgpu_map_bo,
diff --git a/dri.c b/dri.c
index 8492385..6d7c9a0 100644 (file)
--- a/dri.c
+++ b/dri.c
@@ -65,41 +65,109 @@ static bool lookup_extension(const __DRIextension *const *extensions, const char
 }
 
 /*
+ * Close Gem Handle
+ */
+static void close_gem_handle(uint32_t handle, int fd)
+{
+       struct drm_gem_close gem_close;
+       int ret = 0;
+
+       memset(&gem_close, 0, sizeof(gem_close));
+       gem_close.handle = handle;
+       ret = drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &gem_close);
+       if (ret)
+               drv_log("DRM_IOCTL_GEM_CLOSE failed (handle=%x) error %d\n", handle, ret);
+}
+
+/*
  * The DRI GEM namespace may be different from the minigbm's driver GEM namespace. We have
  * to import into minigbm.
  */
 static int import_into_minigbm(struct dri_driver *dri, struct bo *bo)
 {
        uint32_t handle;
-       int prime_fd, ret;
+       int ret, modifier_upper, modifier_lower, num_planes, i, j;
+       __DRIimage *plane_image = NULL;
 
-       if (!dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_FD, &prime_fd))
+       if (dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_MODIFIER_UPPER,
+                                            &modifier_upper) &&
+           dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_MODIFIER_LOWER,
+                                            &modifier_lower)) {
+               bo->meta.format_modifiers[0] =
+                   ((uint64_t)modifier_upper << 32) | (uint32_t)modifier_lower;
+       } else {
+               bo->meta.format_modifiers[0] = DRM_FORMAT_MOD_INVALID;
+       }
+
+       if (!dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_NUM_PLANES,
+                                             &num_planes)) {
                return -errno;
+       }
 
-       ret = drmPrimeFDToHandle(bo->drv->fd, prime_fd, &handle);
-       if (ret) {
-               drv_log("drmPrimeFDToHandle failed with %s\n", strerror(errno));
-               return ret;
+       bo->meta.num_planes = num_planes;
+
+       for (i = 0; i < num_planes; ++i) {
+               int prime_fd, stride, offset;
+               plane_image = dri->image_extension->fromPlanar(bo->priv, i, NULL);
+               __DRIimage *image = plane_image ? plane_image : bo->priv;
+
+               if (i)
+                       bo->meta.format_modifiers[i] = bo->meta.format_modifiers[0];
+
+               if (!dri->image_extension->queryImage(image, __DRI_IMAGE_ATTRIB_STRIDE, &stride) ||
+                   !dri->image_extension->queryImage(image, __DRI_IMAGE_ATTRIB_OFFSET, &offset)) {
+                       ret = -errno;
+                       goto cleanup;
+               }
+
+               if (!dri->image_extension->queryImage(image, __DRI_IMAGE_ATTRIB_FD, &prime_fd)) {
+                       ret = -errno;
+                       goto cleanup;
+               }
+
+               ret = drmPrimeFDToHandle(bo->drv->fd, prime_fd, &handle);
+
+               close(prime_fd);
+
+               if (ret) {
+                       drv_log("drmPrimeFDToHandle failed with %s\n", strerror(errno));
+                       goto cleanup;
+               }
+
+               bo->handles[i].u32 = handle;
+
+               bo->meta.strides[i] = stride;
+               bo->meta.offsets[i] = offset;
+
+               /* Not setting sizes[i] and total_size as these are not
+                * provided by DRI. The best way to "approximate" would be
+                * what drv_bo_import does, but I see nothing in the minigbm
+                * core that needs it. */
+
+               if (plane_image)
+                       dri->image_extension->destroyImage(plane_image);
        }
 
-       bo->handles[0].u32 = handle;
-       close(prime_fd);
        return 0;
-}
-
-/*
- * Close Gem Handle
- */
-static void close_gem_handle(uint32_t handle, int fd)
-{
-       struct drm_gem_close gem_close;
-       int ret = 0;
 
-       memset(&gem_close, 0, sizeof(gem_close));
-       gem_close.handle = handle;
-       ret = drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &gem_close);
-       if (ret)
-               drv_log("DRM_IOCTL_GEM_CLOSE failed (handle=%x) error %d\n", handle, ret);
+cleanup:
+       if (plane_image)
+               dri->image_extension->destroyImage(plane_image);
+       while (--i >= 0) {
+               for (j = 0; j <= i; ++j)
+                       if (bo->handles[j].u32 == bo->handles[i].u32)
+                               break;
+
+               /* Multiple equivalent handles) */
+               if (i == j)
+                       break;
+
+               /* This kind of goes horribly wrong when we already imported
+                * the same handles earlier, as we should really reference
+                * count handles. */
+               close_gem_handle(bo->handles[i].u32, bo->drv->fd);
+       }
+       return ret;
 }
 
 /*
@@ -190,11 +258,9 @@ int dri_bo_create(struct bo *bo, uint32_t width, uint32_t height, uint32_t forma
                  uint64_t use_flags)
 {
        unsigned int dri_use;
-       int ret, dri_format, stride, offset;
-       int modifier_upper, modifier_lower;
+       int ret, dri_format;
        struct dri_driver *dri = bo->drv->priv;
 
-       assert(bo->meta.num_planes == 1);
        dri_format = drm_format_to_dri_format(format);
 
        /* Gallium drivers require shared to get the handle and stride. */
@@ -217,34 +283,38 @@ int dri_bo_create(struct bo *bo, uint32_t width, uint32_t height, uint32_t forma
        if (ret)
                goto free_image;
 
-       if (!dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_STRIDE, &stride)) {
-               ret = -errno;
-               goto close_handle;
+       return 0;
+
+free_image:
+       dri->image_extension->destroyImage(bo->priv);
+       return ret;
+}
+
+int dri_bo_create_with_modifiers(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
+                                const uint64_t *modifiers, uint32_t modifier_count)
+{
+       int ret, dri_format;
+       struct dri_driver *dri = bo->drv->priv;
+
+       if (!dri->image_extension->createImageWithModifiers) {
+               return -ENOENT;
        }
 
-       if (!dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_OFFSET, &offset)) {
+       dri_format = drm_format_to_dri_format(format);
+
+       bo->priv = dri->image_extension->createImageWithModifiers(
+           dri->device, width, height, dri_format, modifiers, modifier_count, NULL);
+       if (!bo->priv) {
                ret = -errno;
-               goto close_handle;
+               return ret;
        }
 
-       if (dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_MODIFIER_UPPER,
-                                            &modifier_upper) &&
-           dri->image_extension->queryImage(bo->priv, __DRI_IMAGE_ATTRIB_MODIFIER_LOWER,
-                                            &modifier_lower)) {
-               bo->meta.format_modifiers[0] =
-                   ((uint64_t)modifier_upper << 32) | (uint32_t)modifier_lower;
-       } else {
-               bo->meta.format_modifiers[0] = DRM_FORMAT_MOD_INVALID;
-       }
+       ret = import_into_minigbm(dri, bo);
+       if (ret)
+               goto free_image;
 
-       bo->meta.strides[0] = stride;
-       bo->meta.sizes[0] = stride * height;
-       bo->meta.offsets[0] = offset;
-       bo->meta.total_size = offset + bo->meta.sizes[0];
        return 0;
 
-close_handle:
-       close_gem_handle(bo->handles[0].u32, bo->drv->fd);
 free_image:
        dri->image_extension->destroyImage(bo->priv);
        return ret;
@@ -255,17 +325,41 @@ int dri_bo_import(struct bo *bo, struct drv_import_fd_data *data)
        int ret;
        struct dri_driver *dri = bo->drv->priv;
 
-       assert(bo->meta.num_planes == 1);
-
-       // clang-format off
-       bo->priv = dri->image_extension->createImageFromFds(dri->device, data->width, data->height,
-                                                           data->format, data->fds,
-                                                           bo->meta.num_planes,
-                                                           (int *)data->strides,
-                                                           (int *)data->offsets, NULL);
-       // clang-format on
-       if (!bo->priv)
-               return -errno;
+       if (data->format_modifiers[0] != DRM_FORMAT_MOD_INVALID) {
+               unsigned error;
+
+               if (!dri->image_extension->createImageFromDmaBufs2)
+                       return -ENOSYS;
+
+               // clang-format off
+               bo->priv = dri->image_extension->createImageFromDmaBufs2(dri->device, data->width, data->height,
+                                                                        data->format,
+                                                                        data->format_modifiers[0],
+                                                                        data->fds,
+                                                                        bo->meta.num_planes,
+                                                                        (int *)data->strides,
+                                                                        (int *)data->offsets,
+                                                                        __DRI_YUV_COLOR_SPACE_UNDEFINED,
+                                                                        __DRI_YUV_RANGE_UNDEFINED,
+                                                                        __DRI_YUV_CHROMA_SITING_UNDEFINED,
+                                                                        __DRI_YUV_CHROMA_SITING_UNDEFINED,
+                                                                        &error, NULL);
+               // clang-format on
+
+               /* Could translate the DRI error, but the Mesa GBM also returns ENOSYS. */
+               if (!bo->priv)
+                       return -ENOSYS;
+       } else {
+               // clang-format off
+               bo->priv = dri->image_extension->createImageFromFds(dri->device, data->width, data->height,
+                                                                   data->format, data->fds,
+                                                                   bo->meta.num_planes,
+                                                                   (int *)data->strides,
+                                                                   (int *)data->offsets, NULL);
+               // clang-format on
+               if (!bo->priv)
+                       return -errno;
+       }
 
        ret = import_into_minigbm(dri, bo);
        if (ret) {
diff --git a/dri.h b/dri.h
index d424dfc..6218e82 100644 (file)
--- a/dri.h
+++ b/dri.h
@@ -30,6 +30,8 @@ int dri_init(struct driver *drv, const char *dri_so_path, const char *driver_suf
 void dri_close(struct driver *drv);
 int dri_bo_create(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
                  uint64_t use_flags);
+int dri_bo_create_with_modifiers(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
+                                const uint64_t *modifiers, uint32_t modifier_count);
 int dri_bo_import(struct bo *bo, struct drv_import_fd_data *data);
 int dri_bo_destroy(struct bo *bo);
 void *dri_bo_map(struct bo *bo, struct vma *vma, size_t plane, uint32_t map_flags);
diff --git a/gbm.c b/gbm.c
index 705acff..ab5b3f7 100644 (file)
--- a/gbm.c
+++ b/gbm.c
@@ -210,6 +210,9 @@ PUBLIC struct gbm_bo *gbm_bo_import(struct gbm_device *gbm, uint32_t type, void
                drv_data.format = fd_data->format;
                drv_data.fds[0] = fd_data->fd;
                drv_data.strides[0] = fd_data->stride;
+
+               for (i = 0; i < GBM_MAX_PLANES; ++i)
+                       drv_data.format_modifiers[i] = DRM_FORMAT_MOD_INVALID;
                break;
        case GBM_BO_IMPORT_FD_MODIFIER:
                gbm_format = fd_modifier_data->format;