| Message ID | 20191021214550.1461-6-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Support CMA per allocation kernel mappings | expand |
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@mediatek.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-mediatek@lists.infradead.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- 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; > >
On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger <matthias.bgg@gmail.com> wrote: > > 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@mediatek.com> > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Cc: David Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-mediatek@lists.infradead.org > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > 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. Thanks for testing. I'm based on drm-misc-next, but don't see anything obvious there that would matter. There are some mmap changes, but I think they shouldn't matter. Did you have fbcon enabled? That may give more clues about where the problem is. Rob
Hi, Rob: On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote: > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger > <matthias.bgg@gmail.com> wrote: > > > > 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@mediatek.com> > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > Cc: David Airlie <airlied@linux.ie> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > > Cc: linux-arm-kernel@lists.infradead.org > > > Cc: linux-mediatek@lists.infradead.org > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > --- > > > > 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. > > Thanks for testing. I'm based on drm-misc-next, but don't see anything > obvious there that would matter. There are some mmap changes, but I > think they shouldn't matter. > > Did you have fbcon enabled? That may give more clues about where the problem is. There are priv->dma_dev for dma device, but it is not drm device. In mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is drm device and ovl device is mmsys's sub device which provide dma function, so ovl is the priv->dma_dev. I think your patch directly use drm device for dma operation and this would cause dma function fail. Please use priv->dma_dev for dma operation. Regards, CK [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.4-rc4 > > Rob
On Wed, Oct 23, 2019 at 4:06 PM CK Hu <ck.hu@mediatek.com> wrote: > > Hi, Rob: > > On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote: > > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger > > <matthias.bgg@gmail.com> wrote: > > > > > > 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@mediatek.com> > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > Cc: David Airlie <airlied@linux.ie> > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > > > Cc: linux-arm-kernel@lists.infradead.org > > > > Cc: linux-mediatek@lists.infradead.org > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > > --- > > > > > > 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. > > > > Thanks for testing. I'm based on drm-misc-next, but don't see anything > > obvious there that would matter. There are some mmap changes, but I > > think they shouldn't matter. > > > > Did you have fbcon enabled? That may give more clues about where the problem is. > > There are priv->dma_dev for dma device, but it is not drm device. In > mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is > drm device and ovl device is mmsys's sub device which provide dma > function, so ovl is the priv->dma_dev. I think your patch directly use > drm device for dma operation and this would cause dma function fail. > Please use priv->dma_dev for dma operation. Right, thanks for catching that. Either we'll need to make CMA GEM object have a struct device ptr or adjust the drm_device.dev to have the necessary DMA setup. One question though, why do you use CMA when you have an IOMMU? That's not optimal as CMA size may be limited. Or you don't always have an IOMMU? Rob
Hi, Rob: On Wed, 2019-10-23 at 17:56 -0500, Rob Herring wrote: > On Wed, Oct 23, 2019 at 4:06 PM CK Hu <ck.hu@mediatek.com> wrote: > > > > Hi, Rob: > > > > On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote: > > > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger > > > <matthias.bgg@gmail.com> wrote: > > > > > > > > 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@mediatek.com> > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > > Cc: David Airlie <airlied@linux.ie> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > > > > Cc: linux-arm-kernel@lists.infradead.org > > > > > Cc: linux-mediatek@lists.infradead.org > > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > > > --- > > > > > > > > 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. > > > > > > Thanks for testing. I'm based on drm-misc-next, but don't see anything > > > obvious there that would matter. There are some mmap changes, but I > > > think they shouldn't matter. > > > > > > Did you have fbcon enabled? That may give more clues about where the problem is. > > > > There are priv->dma_dev for dma device, but it is not drm device. In > > mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is > > drm device and ovl device is mmsys's sub device which provide dma > > function, so ovl is the priv->dma_dev. I think your patch directly use > > drm device for dma operation and this would cause dma function fail. > > Please use priv->dma_dev for dma operation. > > Right, thanks for catching that. Either we'll need to make CMA GEM > object have a struct device ptr or adjust the drm_device.dev to have > the necessary DMA setup. > > One question though, why do you use CMA when you have an IOMMU? That's > not optimal as CMA size may be limited. Or you don't always have an > IOMMU? For all upstreamed mediatek SoC, all has IOMMU, so it does not need CMA. I think we use CMA just because we refer to other drm driver to implement mediatek drm driver and we misused CMA helper function but it works. I think we should change to more accurate implementation. If you want, you could modify it in this series. Regards, CK > > Rob
Hi Rob, On Thu, Oct 24, 2019 at 03:02:57PM +0800, CK Hu wrote: > On Wed, 2019-10-23 at 17:56 -0500, Rob Herring wrote: > > On Wed, Oct 23, 2019 at 4:06 PM CK Hu wrote: > > > On Wed, 2019-10-23 at 12:42 -0500, Rob Herring wrote: > > > > On Tue, Oct 22, 2019 at 12:07 PM Matthias Brugger wrote: > > > > > 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@mediatek.com> > > > > > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > > > > > Cc: David Airlie <airlied@linux.ie> > > > > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > > > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > > > > > Cc: linux-arm-kernel@lists.infradead.org > > > > > > Cc: linux-mediatek@lists.infradead.org > > > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > > > > --- > > > > > > > > > > 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. > > > > > > > > Thanks for testing. I'm based on drm-misc-next, but don't see anything > > > > obvious there that would matter. There are some mmap changes, but I > > > > think they shouldn't matter. > > > > > > > > Did you have fbcon enabled? That may give more clues about where the problem is. > > > > > > There are priv->dma_dev for dma device, but it is not drm device. In > > > mt8173.dtsi [1], there are mmsys device and ovl device, mmsys device is > > > drm device and ovl device is mmsys's sub device which provide dma > > > function, so ovl is the priv->dma_dev. I think your patch directly use > > > drm device for dma operation and this would cause dma function fail. > > > Please use priv->dma_dev for dma operation. > > > > Right, thanks for catching that. Either we'll need to make CMA GEM > > object have a struct device ptr or adjust the drm_device.dev to have > > the necessary DMA setup. > > > > One question though, why do you use CMA when you have an IOMMU? That's > > not optimal as CMA size may be limited. Or you don't always have an > > IOMMU? Please note that the DRM GEM CMA helpers are misnamed, they use the DMA coherent allocation API, and thus don't use CMA if the device is backed by an IOMMU. They should really have been named DRM GEM DMA helpers. > For all upstreamed mediatek SoC, all has IOMMU, so it does not need CMA. > I think we use CMA just because we refer to other drm driver to > implement mediatek drm driver and we misused CMA helper function but it > works. I think we should change to more accurate implementation. If you > want, you could modify it in this series. >
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;
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@mediatek.com> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Matthias Brugger <matthias.bgg@gmail.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-mediatek@lists.infradead.org Signed-off-by: Rob Herring <robh@kernel.org> --- 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