drm/exynos: gem: Get rid of the internal 'pages' array
authorMarek Szyprowski <m.szyprowski@samsung.com>
Thu, 23 Apr 2020 05:43:49 +0000 (14:43 +0900)
committerInki Dae <inki.dae@samsung.com>
Mon, 18 May 2020 02:36:00 +0000 (11:36 +0900)
Internal pages array and scatter-list for them is not really needed for
anything. FBDev emulation can simply rely on the DMA-mapping framework
to create a proper kernel mapping for the buffer, while all other buffer
use cases don't really need that array at all.

Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Inki Dae <inki.dae@samsung.com>
drivers/gpu/drm/exynos/exynos_drm_fbdev.c
drivers/gpu/drm/exynos/exynos_drm_gem.c
drivers/gpu/drm/exynos/exynos_drm_gem.h

index e6ceaf3..56a2b47 100644 (file)
@@ -76,7 +76,6 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
        struct fb_info *fbi;
        struct drm_framebuffer *fb = helper->fb;
        unsigned int size = fb->width * fb->height * fb->format->cpp[0];
-       unsigned int nr_pages;
        unsigned long offset;
 
        fbi = drm_fb_helper_alloc_fbi(helper);
@@ -90,16 +89,6 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper,
 
        drm_fb_helper_fill_info(fbi, helper, sizes);
 
-       nr_pages = exynos_gem->size >> PAGE_SHIFT;
-
-       exynos_gem->kvaddr = (void __iomem *) vmap(exynos_gem->pages, nr_pages,
-                               VM_MAP, pgprot_writecombine(PAGE_KERNEL));
-       if (!exynos_gem->kvaddr) {
-               DRM_DEV_ERROR(to_dma_dev(helper->dev),
-                             "failed to map pages to kernel space.\n");
-               return -EIO;
-       }
-
        offset = fbi->var.xoffset * fb->format->cpp[0];
        offset += fbi->var.yoffset * fb->pitches[0];
 
@@ -133,18 +122,7 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 
        size = mode_cmd.pitches[0] * mode_cmd.height;
 
-       exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_CONTIG, size);
-       /*
-        * If physically contiguous memory allocation fails and if IOMMU is
-        * supported then try to get buffer from non physically contiguous
-        * memory area.
-        */
-       if (IS_ERR(exynos_gem) && is_drm_iommu_supported(dev)) {
-               dev_warn(dev->dev, "contiguous FB allocation failed, falling back to non-contiguous\n");
-               exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_NONCONTIG,
-                                                  size);
-       }
-
+       exynos_gem = exynos_drm_gem_create(dev, EXYNOS_BO_WC, size, true);
        if (IS_ERR(exynos_gem))
                return PTR_ERR(exynos_gem);
 
@@ -229,12 +207,8 @@ err_init:
 static void exynos_drm_fbdev_destroy(struct drm_device *dev,
                                      struct drm_fb_helper *fb_helper)
 {
-       struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(fb_helper);
-       struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
        struct drm_framebuffer *fb;
 
-       vunmap(exynos_gem->kvaddr);
-
        /* release drm framebuffer and real buffer */
        if (fb_helper->fb && fb_helper->fb->funcs) {
                fb = fb_helper->fb;
index f136efb..0df57ee 100644 (file)
 #include "exynos_drm_drv.h"
 #include "exynos_drm_gem.h"
 
-static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
+static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem, bool kvmap)
 {
        struct drm_device *dev = exynos_gem->base.dev;
-       unsigned long attr;
-       unsigned int nr_pages;
-       struct sg_table sgt;
-       int ret = -ENOMEM;
+       unsigned long attr = 0;
 
        if (exynos_gem->dma_addr) {
                DRM_DEV_DEBUG_KMS(to_dma_dev(dev), "already allocated.\n");
                return 0;
        }
 
-       exynos_gem->dma_attrs = 0;
-
        /*
         * if EXYNOS_BO_CONTIG, fully physically contiguous memory
         * region will be allocated else physically contiguous
         * as possible.
         */
        if (!(exynos_gem->flags & EXYNOS_BO_NONCONTIG))
-               exynos_gem->dma_attrs |= DMA_ATTR_FORCE_CONTIGUOUS;
+               attr |= DMA_ATTR_FORCE_CONTIGUOUS;
 
        /*
         * if EXYNOS_BO_WC or EXYNOS_BO_NONCACHABLE, writecombine mapping
@@ -46,61 +41,29 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem *exynos_gem)
         */
        if (exynos_gem->flags & EXYNOS_BO_WC ||
                        !(exynos_gem->flags & EXYNOS_BO_CACHABLE))
-               attr = DMA_ATTR_WRITE_COMBINE;
+               attr |= DMA_ATTR_WRITE_COMBINE;
        else
-               attr = DMA_ATTR_NON_CONSISTENT;
-
-       exynos_gem->dma_attrs |= attr;
-       exynos_gem->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
-
-       nr_pages = exynos_gem->size >> PAGE_SHIFT;
+               attr |= DMA_ATTR_NON_CONSISTENT;
 
-       exynos_gem->pages = kvmalloc_array(nr_pages, sizeof(struct page *),
-                       GFP_KERNEL | __GFP_ZERO);
-       if (!exynos_gem->pages) {
-               DRM_DEV_ERROR(to_dma_dev(dev), "failed to allocate pages.\n");
-               return -ENOMEM;
-       }
+       /* FBDev emulation requires kernel mapping */
+       if (!kvmap)
+               attr |= DMA_ATTR_NO_KERNEL_MAPPING;
 
+       exynos_gem->dma_attrs = attr;
        exynos_gem->cookie = dma_alloc_attrs(to_dma_dev(dev), exynos_gem->size,
                                             &exynos_gem->dma_addr, GFP_KERNEL,
                                             exynos_gem->dma_attrs);
        if (!exynos_gem->cookie) {
                DRM_DEV_ERROR(to_dma_dev(dev), "failed to allocate buffer.\n");
-               goto err_free;
-       }
-
-       ret = dma_get_sgtable_attrs(to_dma_dev(dev), &sgt, exynos_gem->cookie,
-                                   exynos_gem->dma_addr, exynos_gem->size,
-                                   exynos_gem->dma_attrs);
-       if (ret < 0) {
-               DRM_DEV_ERROR(to_dma_dev(dev), "failed to get sgtable.\n");
-               goto err_dma_free;
-       }
-
-       if (drm_prime_sg_to_page_addr_arrays(&sgt, exynos_gem->pages, NULL,
-                                            nr_pages)) {
-               DRM_DEV_ERROR(to_dma_dev(dev), "invalid sgtable.\n");
-               ret = -EINVAL;
-               goto err_sgt_free;
+               return -ENOMEM;
        }
 
-       sg_free_table(&sgt);
+       if (kvmap)
+               exynos_gem->kvaddr = exynos_gem->cookie;
 
        DRM_DEV_DEBUG_KMS(to_dma_dev(dev), "dma_addr(0x%lx), size(0x%lx)\n",
                        (unsigned long)exynos_gem->dma_addr, exynos_gem->size);
-
        return 0;
-
-err_sgt_free:
-       sg_free_table(&sgt);
-err_dma_free:
-       dma_free_attrs(to_dma_dev(dev), exynos_gem->size, exynos_gem->cookie,
-                      exynos_gem->dma_addr, exynos_gem->dma_attrs);
-err_free:
-       kvfree(exynos_gem->pages);
-
-       return ret;
 }
 
 static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
@@ -118,8 +81,6 @@ static void exynos_drm_free_buf(struct exynos_drm_gem *exynos_gem)
        dma_free_attrs(to_dma_dev(dev), exynos_gem->size, exynos_gem->cookie,
                        (dma_addr_t)exynos_gem->dma_addr,
                        exynos_gem->dma_attrs);
-
-       kvfree(exynos_gem->pages);
 }
 
 static int exynos_drm_gem_handle_create(struct drm_gem_object *obj,
@@ -203,7 +164,8 @@ static struct exynos_drm_gem *exynos_drm_gem_init(struct drm_device *dev,
 
 struct exynos_drm_gem *exynos_drm_gem_create(struct drm_device *dev,
                                             unsigned int flags,
-                                            unsigned long size)
+                                            unsigned long size,
+                                            bool kvmap)
 {
        struct exynos_drm_gem *exynos_gem;
        int ret;
@@ -237,7 +199,7 @@ struct exynos_drm_gem *exynos_drm_gem_create(struct drm_device *dev,
        /* set memory type and cache attribute from user side. */
        exynos_gem->flags = flags;
 
-       ret = exynos_drm_alloc_buf(exynos_gem);
+       ret = exynos_drm_alloc_buf(exynos_gem, kvmap);
        if (ret < 0) {
                drm_gem_object_release(&exynos_gem->base);
                kfree(exynos_gem);
@@ -254,7 +216,7 @@ int exynos_drm_gem_create_ioctl(struct drm_device *dev, void *data,
        struct exynos_drm_gem *exynos_gem;
        int ret;
 
-       exynos_gem = exynos_drm_gem_create(dev, args->flags, args->size);
+       exynos_gem = exynos_drm_gem_create(dev, args->flags, args->size, false);
        if (IS_ERR(exynos_gem))
                return PTR_ERR(exynos_gem);
 
@@ -365,7 +327,7 @@ int exynos_drm_gem_dumb_create(struct drm_file *file_priv,
        else
                flags = EXYNOS_BO_CONTIG | EXYNOS_BO_WC;
 
-       exynos_gem = exynos_drm_gem_create(dev, flags, args->size);
+       exynos_gem = exynos_drm_gem_create(dev, flags, args->size, false);
        if (IS_ERR(exynos_gem)) {
                dev_warn(dev->dev, "FB allocation failed.\n");
                return PTR_ERR(exynos_gem);
@@ -442,11 +404,24 @@ struct drm_gem_object *exynos_drm_gem_prime_import(struct drm_device *dev,
 struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
 {
        struct exynos_drm_gem *exynos_gem = to_exynos_gem(obj);
-       int npages;
+       struct drm_device *drm_dev = obj->dev;
+       struct sg_table *sgt;
+       int ret;
+
+       sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+       if (!sgt)
+               return ERR_PTR(-ENOMEM);
 
-       npages = exynos_gem->size >> PAGE_SHIFT;
+       ret = dma_get_sgtable_attrs(to_dma_dev(drm_dev), sgt, exynos_gem->cookie,
+                                   exynos_gem->dma_addr, exynos_gem->size,
+                                   exynos_gem->dma_attrs);
+       if (ret) {
+               DRM_ERROR("failed to get sgtable, %d\n", ret);
+               kfree(sgt);
+               return ERR_PTR(ret);
+       }
 
-       return drm_prime_pages_to_sg(exynos_gem->pages, npages);
+       return sgt;
 }
 
 struct drm_gem_object *
@@ -455,8 +430,6 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
                                     struct sg_table *sgt)
 {
        struct exynos_drm_gem *exynos_gem;
-       int npages;
-       int ret;
 
        if (sgt->nents < 1)
                return ERR_PTR(-EINVAL);
@@ -482,26 +455,8 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
        }
 
        exynos_gem = exynos_drm_gem_init(dev, attach->dmabuf->size);
-       if (IS_ERR(exynos_gem)) {
-               ret = PTR_ERR(exynos_gem);
-               return ERR_PTR(ret);
-       }
-
-       exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
-
-       npages = exynos_gem->size >> PAGE_SHIFT;
-       exynos_gem->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
-       if (!exynos_gem->pages) {
-               ret = -ENOMEM;
-               goto err;
-       }
-
-       ret = drm_prime_sg_to_page_addr_arrays(sgt, exynos_gem->pages, NULL,
-                                              npages);
-       if (ret < 0)
-               goto err_free_large;
-
-       exynos_gem->sgt = sgt;
+       if (IS_ERR(exynos_gem))
+               return ERR_CAST(exynos_gem);
 
        /*
         * Buffer has been mapped as contiguous into DMA address space,
@@ -513,14 +468,9 @@ exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
        else
                exynos_gem->flags |= EXYNOS_BO_CONTIG;
 
+       exynos_gem->dma_addr = sg_dma_address(sgt->sgl);
+       exynos_gem->sgt = sgt;
        return &exynos_gem->base;
-
-err_free_large:
-       kvfree(exynos_gem->pages);
-err:
-       drm_gem_object_release(&exynos_gem->base);
-       kfree(exynos_gem);
-       return ERR_PTR(ret);
 }
 
 void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj)
index f00044c..6ef001f 100644 (file)
  * @base: a gem object.
  *     - a new handle to this gem object would be created
  *     by drm_gem_handle_create().
- * @buffer: a pointer to exynos_drm_gem_buffer object.
- *     - contain the information to memory region allocated
- *     by user request or at framebuffer creation.
- *     continuous memory region allocated by user request
- *     or at framebuffer creation.
  * @flags: indicate memory type to allocated buffer and cache attruibute.
  * @size: size requested from user, in bytes and this size is aligned
  *     in page unit.
  * @cookie: cookie returned by dma_alloc_attrs
- * @kvaddr: kernel virtual address to allocated memory region.
+ * @kvaddr: kernel virtual address to allocated memory region (for fbdev)
  * @dma_addr: bus address(accessed by dma) to allocated memory region.
  *     - this address could be physical address without IOMMU and
  *     device address with IOMMU.
- * @pages: Array of backing pages.
+ * @dma_attrs: attrs passed dma mapping framework
  * @sgt: Imported sg_table.
  *
  * P.S. this object would be transferred to user as kms_bo.handle so
@@ -48,7 +43,6 @@ struct exynos_drm_gem {
        void __iomem            *kvaddr;
        dma_addr_t              dma_addr;
        unsigned long           dma_attrs;
-       struct page             **pages;
        struct sg_table         *sgt;
 };
 
@@ -58,7 +52,8 @@ void exynos_drm_gem_destroy(struct exynos_drm_gem *exynos_gem);
 /* create a new buffer with gem object */
 struct exynos_drm_gem *exynos_drm_gem_create(struct drm_device *dev,
                                             unsigned int flags,
-                                            unsigned long size);
+                                            unsigned long size,
+                                            bool kvmap);
 
 /*
  * request gem object creation and buffer allocation as the size