[PATCH 5/6] drm/mediatek: Convert to use CMA helpers

[Laurent Pinchart]

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 at mediatek.com>
> > > > > > Cc: Philipp Zabel <p.zabel at pengutronix.de>
> > > > > > Cc: David Airlie <airlied at linux.ie>
> > > > > > Cc: Daniel Vetter <daniel at ffwll.ch>
> > > > > > Cc: Matthias Brugger <matthias.bgg at gmail.com>
> > > > > > Cc: linux-arm-kernel at lists.infradead.org
> > > > > > Cc: linux-mediatek at lists.infradead.org
> > > > > > Signed-off-by: Rob Herring <robh at 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.
> 

-- 
Regards,

Laurent Pinchart