On 10/5/20 4:29 AM, Christoph Hellwig wrote:
On Fri, Oct 02, 2020 at 08:46:35AM -0400, Jonathan Marek wrote:
+void msm_gem_sync_cache(struct drm_gem_object *obj, uint32_t flags,
+ size_t range_start, size_t range_end)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+ /* TODO: sync only the required range, and don't invalidate on clean */
+
+ if (flags & MSM_GEM_SYNC_CACHE_CLEAN)
+ sync_for_device(msm_obj);
+
+ if (flags & MSM_GEM_SYNC_CACHE_INVALIDATE)
+ sync_for_cpu(msm_obj);
And make to these ones as well. They are complete abuses of the DMA
API, and while we had to live with that for now to not cause regressions
they absoutely must not be exposed in a userspace ABI like this.
How do you propose that cached non-coherent memory be implemented? It is a
useful feature for userspace.
If the driver is using the DMA API you need to use dma_alloc_noncoherent
and friends as of 5.10 (see the iommu list for the discussion).
If you use the raw IOMMU API (which I think the msm drm driver does) you
need to work with the maintainers to implement a cache synchronization
API that is not tied to the DMA API.
The cache synchronization doesn't have anything to do with IOMMU (for
example: cache synchronization would be useful in cases where drm/msm
doesn't use IOMMU).
What is needed is to call arch_sync_dma_for_{cpu,device} (which is what
I went with initially, but then decided to re-use drm/msm's
sync_for_{cpu,device}). But you are also saying those functions aren't
for driver use, and I doubt IOMMU maintainers will want to add wrappers
for these functions just to satisfy this "not for driver use" requirement.
