Hi Rob, On 21/10/2019 23:45, Rob Herring wrote: > The only reason the Mediatek driver doesn't use the CMA helpers is it > sets DMA_ATTR_NO_KERNEL_MAPPING and does a vmap() on demand. Using > vmap() is not even guaranteed to work as DMA buffers may not have a > struct page. Now that the CMA helpers support setting > DMA_ATTR_NO_KERNEL_MAPPING as needed or not, convert Mediatek driver to > use CMA helpers. > > Cc: CK Hu <ck.hu@xxxxxxxxxxxx> > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > Cc: David Airlie <airlied@xxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Cc: Matthias Brugger <matthias.bgg@xxxxxxxxx> > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Cc: linux-mediatek@xxxxxxxxxxxxxxxxxxx > Signed-off-by: Rob Herring <robh@xxxxxxxxxx> > --- I tested this on my Chromebook with some patches on top of v5.4-rc1 [1], which work. If I add your patches on top of that, the system does not boot up. Unfortunately I don't have a serial console, so I wasn't able to see if there is any error message. I checked the config and didn't see any suspicious I added Uli and some guys from chromium, maybe someone can provide some logs/insights. Regards, Matthias [1] https://github.com/uli/kernel/tree/elm-working-5.4 > drivers/gpu/drm/mediatek/Makefile | 1 - > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 28 +-- > drivers/gpu/drm/mediatek/mtk_drm_fb.c | 1 - > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 289 ----------------------- > drivers/gpu/drm/mediatek/mtk_drm_gem.h | 51 ---- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 7 +- > 7 files changed, 15 insertions(+), 364 deletions(-) > delete mode 100644 drivers/gpu/drm/mediatek/mtk_drm_gem.c > delete mode 100644 drivers/gpu/drm/mediatek/mtk_drm_gem.h > > diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile > index 82ae49c64221..50a50e86738f 100644 > --- a/drivers/gpu/drm/mediatek/Makefile > +++ b/drivers/gpu/drm/mediatek/Makefile > @@ -8,7 +8,6 @@ mediatek-drm-y := mtk_disp_color.o \ > mtk_drm_ddp_comp.o \ > mtk_drm_drv.o \ > mtk_drm_fb.o \ > - mtk_drm_gem.o \ > mtk_drm_plane.o \ > mtk_dsi.o \ > mtk_mipi_tx.o \ > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 34a731755791..638d57e8ac12 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -11,6 +11,7 @@ > > #include <drm/drm_atomic_helper.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_print.h> > #include <drm/drm_probe_helper.h> > #include <drm/drm_vblank.h> > > @@ -18,7 +19,6 @@ > #include "mtk_drm_crtc.h" > #include "mtk_drm_ddp.h" > #include "mtk_drm_ddp_comp.h" > -#include "mtk_drm_gem.h" > #include "mtk_drm_plane.h" > > /** > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index 352b81a7a670..36f32507e5fb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -28,7 +28,6 @@ > #include "mtk_drm_ddp_comp.h" > #include "mtk_drm_drv.h" > #include "mtk_drm_fb.h" > -#include "mtk_drm_gem.h" > > #define DRIVER_NAME "mediatek" > #define DRIVER_DESC "Mediatek SoC DRM" > @@ -335,16 +334,14 @@ static void mtk_drm_kms_deinit(struct drm_device *drm) > drm_mode_config_cleanup(drm); > } > > -static const struct file_operations mtk_drm_fops = { > - .owner = THIS_MODULE, > - .open = drm_open, > - .release = drm_release, > - .unlocked_ioctl = drm_ioctl, > - .mmap = mtk_drm_gem_mmap, > - .poll = drm_poll, > - .read = drm_read, > - .compat_ioctl = drm_compat_ioctl, > -}; > +DEFINE_DRM_GEM_CMA_FOPS(mtk_drm_fops); > + > +static int mtk_drm_gem_dumb_create(struct drm_file *file_priv, > + struct drm_device *dev, > + struct drm_mode_create_dumb *args) > +{ > + return drm_gem_cma_dumb_create_internal(file_priv, dev, args); > +} > > /* > * We need to override this because the device used to import the memory is > @@ -361,18 +358,15 @@ struct drm_gem_object *mtk_drm_gem_prime_import(struct drm_device *dev, > static struct drm_driver mtk_drm_driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC, > > - .gem_free_object_unlocked = mtk_drm_gem_free_object, > .gem_vm_ops = &drm_gem_cma_vm_ops, > + .gem_create_object = drm_cma_gem_create_object_default_funcs, > .dumb_create = mtk_drm_gem_dumb_create, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > .gem_prime_import = mtk_drm_gem_prime_import, > - .gem_prime_get_sg_table = mtk_gem_prime_get_sg_table, > - .gem_prime_import_sg_table = mtk_gem_prime_import_sg_table, > - .gem_prime_mmap = mtk_drm_gem_mmap_buf, > - .gem_prime_vmap = mtk_drm_gem_prime_vmap, > - .gem_prime_vunmap = mtk_drm_gem_prime_vunmap, > + .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, > + .gem_prime_mmap = drm_gem_prime_mmap, > .fops = &mtk_drm_fops, > > .name = DRIVER_NAME, > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c b/drivers/gpu/drm/mediatek/mtk_drm_fb.c > index 3f230a28a2dc..596b4d5ed002 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c > @@ -14,7 +14,6 @@ > > #include "mtk_drm_drv.h" > #include "mtk_drm_fb.h" > -#include "mtk_drm_gem.h" > > static const struct drm_framebuffer_funcs mtk_drm_fb_funcs = { > .create_handle = drm_gem_fb_create_handle, > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > deleted file mode 100644 > index ca672f1d140d..000000000000 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > +++ /dev/null > @@ -1,289 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Copyright (c) 2015 MediaTek Inc. > - */ > - > -#include <linux/dma-buf.h> > - > -#include <drm/drm.h> > -#include <drm/drm_device.h> > -#include <drm/drm_gem.h> > -#include <drm/drm_prime.h> > - > -#include "mtk_drm_drv.h" > -#include "mtk_drm_gem.h" > - > -static struct mtk_drm_gem_obj *mtk_drm_gem_init(struct drm_device *dev, > - unsigned long size) > -{ > - struct mtk_drm_gem_obj *mtk_gem_obj; > - int ret; > - > - size = round_up(size, PAGE_SIZE); > - > - mtk_gem_obj = kzalloc(sizeof(*mtk_gem_obj), GFP_KERNEL); > - if (!mtk_gem_obj) > - return ERR_PTR(-ENOMEM); > - > - ret = drm_gem_object_init(dev, &mtk_gem_obj->base, size); > - if (ret < 0) { > - DRM_ERROR("failed to initialize gem object\n"); > - kfree(mtk_gem_obj); > - return ERR_PTR(ret); > - } > - > - return mtk_gem_obj; > -} > - > -struct mtk_drm_gem_obj *mtk_drm_gem_create(struct drm_device *dev, > - size_t size, bool alloc_kmap) > -{ > - struct mtk_drm_private *priv = dev->dev_private; > - struct mtk_drm_gem_obj *mtk_gem; > - struct drm_gem_object *obj; > - int ret; > - > - mtk_gem = mtk_drm_gem_init(dev, size); > - if (IS_ERR(mtk_gem)) > - return ERR_CAST(mtk_gem); > - > - obj = &mtk_gem->base; > - > - mtk_gem->dma_attrs = DMA_ATTR_WRITE_COMBINE; > - > - if (!alloc_kmap) > - mtk_gem->dma_attrs |= DMA_ATTR_NO_KERNEL_MAPPING; > - > - mtk_gem->cookie = dma_alloc_attrs(priv->dma_dev, obj->size, > - &mtk_gem->dma_addr, GFP_KERNEL, > - mtk_gem->dma_attrs); > - if (!mtk_gem->cookie) { > - DRM_ERROR("failed to allocate %zx byte dma buffer", obj->size); > - ret = -ENOMEM; > - goto err_gem_free; > - } > - > - if (alloc_kmap) > - mtk_gem->kvaddr = mtk_gem->cookie; > - > - DRM_DEBUG_DRIVER("cookie = %p dma_addr = %pad size = %zu\n", > - mtk_gem->cookie, &mtk_gem->dma_addr, > - size); > - > - return mtk_gem; > - > -err_gem_free: > - drm_gem_object_release(obj); > - kfree(mtk_gem); > - return ERR_PTR(ret); > -} > - > -void mtk_drm_gem_free_object(struct drm_gem_object *obj) > -{ > - struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > - struct mtk_drm_private *priv = obj->dev->dev_private; > - > - if (mtk_gem->sg) > - drm_prime_gem_destroy(obj, mtk_gem->sg); > - else > - dma_free_attrs(priv->dma_dev, obj->size, mtk_gem->cookie, > - mtk_gem->dma_addr, mtk_gem->dma_attrs); > - > - /* release file pointer to gem object. */ > - drm_gem_object_release(obj); > - > - kfree(mtk_gem); > -} > - > -int mtk_drm_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, > - struct drm_mode_create_dumb *args) > -{ > - struct mtk_drm_gem_obj *mtk_gem; > - int ret; > - > - args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8); > - args->size = args->pitch * args->height; > - > - mtk_gem = mtk_drm_gem_create(dev, args->size, false); > - if (IS_ERR(mtk_gem)) > - return PTR_ERR(mtk_gem); > - > - /* > - * allocate a id of idr table where the obj is registered > - * and handle has the id what user can see. > - */ > - ret = drm_gem_handle_create(file_priv, &mtk_gem->base, &args->handle); > - if (ret) > - goto err_handle_create; > - > - /* drop reference from allocate - handle holds it now. */ > - drm_gem_object_put_unlocked(&mtk_gem->base); > - > - return 0; > - > -err_handle_create: > - mtk_drm_gem_free_object(&mtk_gem->base); > - return ret; > -} > - > -static int mtk_drm_gem_object_mmap(struct drm_gem_object *obj, > - struct vm_area_struct *vma) > - > -{ > - int ret; > - struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > - struct mtk_drm_private *priv = obj->dev->dev_private; > - > - /* > - * dma_alloc_attrs() allocated a struct page table for mtk_gem, so clear > - * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > - */ > - vma->vm_flags &= ~VM_PFNMAP; > - > - ret = dma_mmap_attrs(priv->dma_dev, vma, mtk_gem->cookie, > - mtk_gem->dma_addr, obj->size, mtk_gem->dma_attrs); > - if (ret) > - drm_gem_vm_close(vma); > - > - return ret; > -} > - > -int mtk_drm_gem_mmap_buf(struct drm_gem_object *obj, struct vm_area_struct *vma) > -{ > - int ret; > - > - ret = drm_gem_mmap_obj(obj, obj->size, vma); > - if (ret) > - return ret; > - > - return mtk_drm_gem_object_mmap(obj, vma); > -} > - > -int mtk_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) > -{ > - struct drm_gem_object *obj; > - int ret; > - > - ret = drm_gem_mmap(filp, vma); > - if (ret) > - return ret; > - > - obj = vma->vm_private_data; > - > - /* > - * Set vm_pgoff (used as a fake buffer offset by DRM) to 0 and map the > - * whole buffer from the start. > - */ > - vma->vm_pgoff = 0; > - > - return mtk_drm_gem_object_mmap(obj, vma); > -} > - > -/* > - * Allocate a sg_table for this GEM object. > - * Note: Both the table's contents, and the sg_table itself must be freed by > - * the caller. > - * Returns a pointer to the newly allocated sg_table, or an ERR_PTR() error. > - */ > -struct sg_table *mtk_gem_prime_get_sg_table(struct drm_gem_object *obj) > -{ > - struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > - struct mtk_drm_private *priv = obj->dev->dev_private; > - struct sg_table *sgt; > - int ret; > - > - sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); > - if (!sgt) > - return ERR_PTR(-ENOMEM); > - > - ret = dma_get_sgtable_attrs(priv->dma_dev, sgt, mtk_gem->cookie, > - mtk_gem->dma_addr, obj->size, > - mtk_gem->dma_attrs); > - if (ret) { > - DRM_ERROR("failed to allocate sgt, %d\n", ret); > - kfree(sgt); > - return ERR_PTR(ret); > - } > - > - return sgt; > -} > - > -struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev, > - struct dma_buf_attachment *attach, struct sg_table *sg) > -{ > - struct mtk_drm_gem_obj *mtk_gem; > - int ret; > - struct scatterlist *s; > - unsigned int i; > - dma_addr_t expected; > - > - mtk_gem = mtk_drm_gem_init(dev, attach->dmabuf->size); > - > - if (IS_ERR(mtk_gem)) > - return ERR_CAST(mtk_gem); > - > - expected = sg_dma_address(sg->sgl); > - for_each_sg(sg->sgl, s, sg->nents, i) { > - if (sg_dma_address(s) != expected) { > - DRM_ERROR("sg_table is not contiguous"); > - ret = -EINVAL; > - goto err_gem_free; > - } > - expected = sg_dma_address(s) + sg_dma_len(s); > - } > - > - mtk_gem->dma_addr = sg_dma_address(sg->sgl); > - mtk_gem->sg = sg; > - > - return &mtk_gem->base; > - > -err_gem_free: > - kfree(mtk_gem); > - return ERR_PTR(ret); > -} > - > -void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj) > -{ > - struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > - struct sg_table *sgt; > - struct sg_page_iter iter; > - unsigned int npages; > - unsigned int i = 0; > - > - if (mtk_gem->kvaddr) > - return mtk_gem->kvaddr; > - > - sgt = mtk_gem_prime_get_sg_table(obj); > - if (IS_ERR(sgt)) > - return NULL; > - > - npages = obj->size >> PAGE_SHIFT; > - mtk_gem->pages = kcalloc(npages, sizeof(*mtk_gem->pages), GFP_KERNEL); > - if (!mtk_gem->pages) > - goto out; > - > - for_each_sg_page(sgt->sgl, &iter, sgt->orig_nents, 0) { > - mtk_gem->pages[i++] = sg_page_iter_page(&iter); > - if (i > npages) > - break; > - } > - mtk_gem->kvaddr = vmap(mtk_gem->pages, npages, VM_MAP, > - pgprot_writecombine(PAGE_KERNEL)); > - > -out: > - kfree((void *)sgt); > - > - return mtk_gem->kvaddr; > -} > - > -void mtk_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr) > -{ > - struct mtk_drm_gem_obj *mtk_gem = to_mtk_gem_obj(obj); > - > - if (!mtk_gem->pages) > - return; > - > - vunmap(vaddr); > - mtk_gem->kvaddr = 0; > - kfree((void *)mtk_gem->pages); > -} > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.h b/drivers/gpu/drm/mediatek/mtk_drm_gem.h > deleted file mode 100644 > index ff9f976d9807..000000000000 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.h > +++ /dev/null > @@ -1,51 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-only */ > -/* > - * Copyright (c) 2015 MediaTek Inc. > - */ > - > -#ifndef _MTK_DRM_GEM_H_ > -#define _MTK_DRM_GEM_H_ > - > -#include <drm/drm_gem.h> > - > -/* > - * mtk drm buffer structure. > - * > - * @base: a gem object. > - * - a new handle to this gem object would be created > - * by drm_gem_handle_create(). > - * @cookie: the return value of dma_alloc_attrs(), keep it for dma_free_attrs() > - * @kvaddr: kernel virtual address of gem buffer. > - * @dma_addr: dma address of gem buffer. > - * @dma_attrs: dma attributes of gem buffer. > - * > - * P.S. this object would be transferred to user as kms_bo.handle so > - * user can access the buffer through kms_bo.handle. > - */ > -struct mtk_drm_gem_obj { > - struct drm_gem_object base; > - void *cookie; > - void *kvaddr; > - dma_addr_t dma_addr; > - unsigned long dma_attrs; > - struct sg_table *sg; > - struct page **pages; > -}; > - > -#define to_mtk_gem_obj(x) container_of(x, struct mtk_drm_gem_obj, base) > - > -void mtk_drm_gem_free_object(struct drm_gem_object *gem); > -struct mtk_drm_gem_obj *mtk_drm_gem_create(struct drm_device *dev, size_t size, > - bool alloc_kmap); > -int mtk_drm_gem_dumb_create(struct drm_file *file_priv, struct drm_device *dev, > - struct drm_mode_create_dumb *args); > -int mtk_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma); > -int mtk_drm_gem_mmap_buf(struct drm_gem_object *obj, > - struct vm_area_struct *vma); > -struct sg_table *mtk_gem_prime_get_sg_table(struct drm_gem_object *obj); > -struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device *dev, > - struct dma_buf_attachment *attach, struct sg_table *sg); > -void *mtk_drm_gem_prime_vmap(struct drm_gem_object *obj); > -void mtk_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); > - > -#endif > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index 584a9ecadce6..8f256602f075 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -8,13 +8,14 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_plane_helper.h> > +#include <drm/drm_print.h> > +#include <drm/drm_gem_cma_helper.h> > #include <drm/drm_gem_framebuffer_helper.h> > > #include "mtk_drm_crtc.h" > #include "mtk_drm_ddp_comp.h" > #include "mtk_drm_drv.h" > #include "mtk_drm_fb.h" > -#include "mtk_drm_gem.h" > #include "mtk_drm_plane.h" > > static const u32 formats[] = { > @@ -108,7 +109,6 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, > struct drm_crtc *crtc = plane->state->crtc; > struct drm_framebuffer *fb = plane->state->fb; > struct drm_gem_object *gem; > - struct mtk_drm_gem_obj *mtk_gem; > unsigned int pitch, format; > dma_addr_t addr; > > @@ -116,8 +116,7 @@ static void mtk_plane_atomic_update(struct drm_plane *plane, > return; > > gem = fb->obj[0]; > - mtk_gem = to_mtk_gem_obj(gem); > - addr = mtk_gem->dma_addr; > + addr = to_drm_gem_cma_obj(gem)->paddr; > pitch = fb->pitches[0]; > format = fb->format->format; > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
