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

[CK Hu <ck.hu@mediatek.com>]

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



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek