Re: please revert "drm/msm: Use the correct dma_sync calls in msm_gem"

From: Rob Clark <robdclark@chromium.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Stephan Gerhold <stephan@gerhold.net>,
	Sean Paul <seanpaul@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	torvalds@linux-foundation.org
Subject: Re: please revert "drm/msm: Use the correct dma_sync calls in msm_gem"
Date: Fri, 2 Aug 2019 15:06:10 -0700	[thread overview]
Message-ID: <CAJs_Fx45VtKe52eTuAUcqSUTrY=892OwhCZNrLGoQHHBwETCdQ@mail.gmail.com> (raw)
In-Reply-To: <20190802213756.GA20033@lst.de>

On Fri, Aug 2, 2019 at 2:37 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi Rob,
>
> I just saw your above commit in Linus' tree, which is completely
> bogus and misunderstand the DMA API. Next time you have any issues
> please Cc the relevant maintainers and mailing lists.  But even
> more importantly get_dma_ops is an completely internal API, and
> whatevet your helpers are trying to do is bad - if the dma wasn't
> mapped at the time you call the new sync_for_device helper, this
> is broken because you can't call dma_sync_sg_for_device on unmapped
> address.  If it was mapped it is bogus as well as you can't double
> map it.  Please describe what you're actually trying to fix and we're
> going to work on a proper solution, but this shot from the hip is just
> going to make things worse.

Sorry, this is just a temporary band-aid for v5.3 to get things
working again.  Yes, I realize it is a complete hack.

The root problem is that I'm using the DMA API in the first place.  I
don't actually use the DMA API to map buffers, for various reasons,
but instead manage the iommu_domain directly.

Because arm/arm64 cache ops are not exported to modules, so currently
I need to abuse the DMA API for cache operations (really just to clean
pages if I need to mmap them uncached/writecombine).  Originally I was
doing that w/ dma_{map,unmap}_sg.  But to avoid debug splats I
switched that to dma_sync_sg (see
0036bc73ccbe7e600a3468bf8e8879b122252274).  But now it seems the
dma-direct ops are unhappy w/ dma_sync without a dma_map (AFAICT).

(On some generations of hw, the iommu is attached to the device node
that maps to the drm device, which is passed to dma_map/dma_sync.  On
other generations the iommu is attached to a sub-device.  Changing
this would break dtb compatibility.. so for now I need to handle both
iommu-ops and direct-ops cases.)

For v5.4, my plan was to revisit (at least for arm64) exposing cache
ops to drivers, so I can use drm_clflush_* instead, and stop abusing
DMA API.

BR,
-R