Re: [PATCH v3 1/1] drm: msm: Replace dma_map_sg with dma_sync_sg*

From: Christoph Hellwig <hch@lst.de>
To: Rob Clark <robdclark@gmail.com>
Cc: hch@lst.de, Daniel Vetter <daniel@ffwll.ch>,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Tomasz Figa <tfiga@chromium.org>,
	Sean Paul <seanpaul@chromium.org>,
	Vivek Gautam <vivek.gautam@codeaurora.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH v3 1/1] drm: msm: Replace dma_map_sg with dma_sync_sg*
Date: Fri, 30 Nov 2018 10:35:41 +0100	[thread overview]
Message-ID: <20181130093541.GA21436@lst.de> (raw)
In-Reply-To: <CAF6AEGsS6LnuoL9+_LsKLOWxKF6P0nTrxs7b0j0Kqnqz6tmocw@mail.gmail.com>

On Thu, Nov 29, 2018 at 08:15:23PM -0500, Rob Clark wrote:
> As far as hiding cache ops behind iommu layer, I guess I'd been
> thinking more of just letting the drivers that want to bypass dma
> layer take things into their own hands.. tbh I think/assume/hope
> drm/msm is more the exception than the rule as far as needing to
> bypass the dma layer.  Or at least I guess the # of drivers having
> problems w/ the dma layer is less than the # of iommu drivers.

So the whole bypass thing has already been a contentious discussion
in the past.  One thing that the API aready enforces is that we pass
a DMA direction, which I want to keep.  The other is that we need
a struct device (or something similiar) that is used to check if the
device is cache coherent or not.  In your MSM case you might know that,
but in general it really is a platform issue that I don't want every
driver to know about.

> (Not sure if that changes my thoughts on this patch, it isn't like
> what this patch replaces isn't also a problematic hack around the
> inability to bypass the dma layer.  In the short term I just want
> *something* that works, I'm totally happy to refactor later when there
> are better options.)

The current patch is simply broken.  You can't just construct your
own S/G list and pass it to the DMA API, and we enforce that very
strictly with dma debug enabled.

So your only options are: a) actually use the DMA API for creating
the mapping, by e.g. using dma_direct_ops ontop of an actual IOMMU
managed in the background, or b) figure out a way to do cache
maintainance for raw IOMMU API drivers.