From 9940d9d93406f41ad4dc69fa2cda1e059a7ca108 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Thu, 23 Apr 2020 14:43:49 +0900 Subject: [PATCH] drm/exynos: gem: Get rid of the internal 'pages' array MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Marek Szyprowski Signed-off-by: Inki Dae --- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 28 +------ drivers/gpu/drm/exynos/exynos_drm_gem.c | 124 +++++++++--------------------- drivers/gpu/drm/exynos/exynos_drm_gem.h | 13 +--- 3 files changed, 42 insertions(+), 123 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index e6ceaf3..56a2b47 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -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; diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index f136efb..0df57ee 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -17,28 +17,23 @@ #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) diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h index f00044c..6ef001f 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h @@ -21,20 +21,15 @@ * @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 -- 2.7.4