mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lucas Stach <l.stach@pengutronix.de>
To: "Christian König" <christian.koenig@amd.com>,
	"Pekka Paalanen" <ppaalanen@gmail.com>
Cc: "Sharma, Shashank" <Shashank.Sharma@amd.com>,
	lkml <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Nicolas Dufresne <nicolas@ndufresne.ca>,
	linaro-mm-sig@lists.linaro.org,
	Sumit Semwal <sumit.semwal@linaro.org>,
	linux-media <linux-media@vger.kernel.org>
Subject: Re: DMA-buf and uncached system memory
Date: Thu, 23 Jun 2022 12:13:16 +0200	[thread overview]
Message-ID: <708e27755317a7650ca08ba2e4c14691ac0d6ba2.camel@pengutronix.de> (raw)
In-Reply-To: <05814ddb-4f3e-99d8-025a-c31db7b2c46b@amd.com>

Am Donnerstag, dem 23.06.2022 um 11:46 +0200 schrieb Christian König:
> Am 23.06.22 um 11:33 schrieb Lucas Stach:
> > [SNIP]
> > > > > > In the DMA API keeping things mapped is also a valid use-case, but then
> > > > > > you need to do explicit domain transfers via the dma_sync_* family,
> > > > > > which DMA-buf has not inherited. Again those sync are no-ops on cache
> > > > > > coherent architectures, but do any necessary cache maintenance on non
> > > > > > coherent arches.
> > > > > Correct, yes. Coherency is mandatory for DMA-buf, you can't use
> > > > > dma_sync_* on it when you are the importer.
> > > > > 
> > > > > The exporter could of course make use of that because he is the owner of
> > > > > the buffer.
> > > > In the example given here with UVC video, you don't know that the
> > > > buffer will be exported and needs to be coherent without
> > > > synchronization points, due to the mapping cache at the DRM side. So
> > > > V4L2 naturally allocates the buffers from CPU cached memory. If the
> > > > expectation is that those buffers are device coherent without relying
> > > > on the map/unmap_attachment calls, then V4L2 needs to always
> > > > synchronize caches on DQBUF when the  buffer is allocated from CPU
> > > > cached memory and a single DMA-buf attachment exists. And while writing
> > > > this I realize that this is probably exactly what V4L2 should do...
> > > No, the expectation is that the importer can deal with whatever the
> > > exporter provides.
> > > 
> > > If the importer can't access the DMA-buf coherently it's his job to
> > > handle that gracefully.
> > How does the importer know that the memory behind the DMA-buf is in CPU
> > cached memory?
> > 
> > If you now tell me that an importer always needs to assume this and
> > reject the import if it can't do snooping, then any DMA-buf usage on
> > most ARM SoCs is currently invalid usage.
> 
> Yes, exactly that. I've pointed out a couple of times now that a lot of 
> ARM SoCs don't implement that the way we need it.
> 
> We already had tons of bug reports because somebody attached a random 
> PCI root complex to an ARM SoC and expected it to work with for example 
> an AMD GPU.
> 
> Non-cache coherent applications are currently not really supported by 
> the DMA-buf framework in any way.
> 
I'm not talking about bolting on a PCIe root complex, with its implicit
inherited "PCI is cache coherent" expectations to a ARM SoC, but just
the standard VPU/GPU/display engines are not snooping on most ARM SoCs.

> > On most of the multimedia
> > targeted ARM SoCs being unable to snoop the cache is the norm, not an
> > exception.
> > 
> > > See for example on AMD/Intel hardware most of the engines can perfectly
> > > deal with cache coherent memory accesses. Only the display engines can't.
> > > 
> > > So on import time we can't even say if the access can be coherent and
> > > snoop the CPU cache or not because we don't know how the imported
> > > DMA-buf will be used later on.
> > > 
> > So for those mixed use cases, wouldn't it help to have something
> > similar to the dma_sync in the DMA-buf API, so your scanout usage can
> > tell the exporter that it's going to do non-snoop access and any dirty
> > cache lines must be cleaned? Signaling this to the exporter would allow
> > to skip the cache maintenance if the buffer is in CPU uncached memory,
> > which again is a default case for the ARM SoC world.
> 
> Well for the AMD and Intel use cases we at least have the opportunity to 
> signal cache flushing, but I'm not sure if that counts for everybody.
> 
Sure, all the non-coherent arches have some way to do the cache
maintenance in some explicit way. Non coherent and no cache maintenance
instruction would be a recipe for desaster. ;)

> What we would rather do for those use cases is an indicator on the 
> DMA-buf if the underlying backing store is CPU cached or not. The 
> importer can then cleanly reject the use cases where it can't support 
> CPU cache snooping.
> 
> This then results in the normal fallback paths which we have anyway for 
> those use cases because DMA-buf sharing is not always possible.
> 
That's a very x86 centric world view you have there. 99% of DMA-buf
uses on those cheap ARM SoCs is non-snooping. We can not do any
fallbacks here, as the whole graphics world on those SoCs with their
different IP cores mixed together depends on DMA-buf sharing working
efficiently even when the SoC is mostly non coherent.

In fact DMA-buf sharing works fine on most of those SoCs because
everyone just assumes that all the accelerators don't snoop, so the
memory shared via DMA-buf is mostly CPU uncached. It only falls apart
for uses like the UVC cameras, where the shared buffer ends up being
CPU cached.

Non-coherent without explicit domain transfer points is just not going
to work. So why can't we solve the issue for DMA-buf in the same way as
the DMA API already solved it years ago: by adding the equivalent of
the dma_sync calls that do cache maintenance when necessary? On x86 (or
any system where things are mostly coherent) you could still no-op them
for the common case and only trigger cache cleaning if the importer
explicitly says that is going to do a non-snooping access.

Regards,
Lucas


  reply	other threads:[~2022-06-23 10:13 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-15  8:58 Christian König
2021-02-15  9:06 ` Simon Ser
2021-02-15  9:34   ` Christian König
2021-02-15 11:53     ` Lucas Stach
2021-02-15 12:04       ` Christian König
2021-02-15 12:16         ` Lucas Stach
2021-02-15 12:25           ` Christian König
2021-02-15 14:41         ` David Laight
2021-02-15 14:54           ` [Linaro-mm-sig] " Christian König
2021-02-15  9:49 ` Thomas Zimmermann
2021-02-15 12:00   ` Thomas Zimmermann
2021-02-15 12:10     ` Christian König
2021-02-15 20:46       ` Nicolas Dufresne
2021-02-15 20:39 ` Nicolas Dufresne
     [not found]   ` <5822b325-766e-ce3c-50eb-d7f54f14fd0b@mediatek.com>
     [not found]     ` <39f7f41a-af8d-4700-37da-9401455afb98@amd.com>
2022-06-21 15:42       ` Nicolas Dufresne
2022-06-22  9:05         ` [Linaro-mm-sig] " Christian König
2021-02-16  9:25 ` Daniel Vetter
2022-06-22 19:39   ` Nicolas Dufresne
2022-06-22 23:34     ` Daniel Stone
2022-06-23  6:59       ` Christian König
2022-06-23  7:13         ` Pekka Paalanen
2022-06-23  7:26           ` Christian König
2022-06-23  8:04             ` Lucas Stach
2022-06-23  8:14               ` Christian König
2022-06-23  8:58                 ` Lucas Stach
2022-06-23  9:09                   ` Christian König
2022-06-23  9:33                     ` Lucas Stach
2022-06-23  9:46                       ` Christian König
2022-06-23 10:13                         ` Lucas Stach [this message]
2022-06-23 11:10                           ` Christian König
2022-06-23 11:27                             ` Daniel Stone
2022-06-23 11:32                               ` Christian König
2022-06-24 22:02                                 ` [Linaro-mm-sig] " Daniel Vetter
2022-07-04 13:48                                   ` Christian König
2022-08-09 14:46                                     ` Daniel Vetter
2022-08-10  5:55                                       ` Christian König
2022-06-23 11:29                             ` Lucas Stach
2022-06-23 11:54                               ` Christian König
2022-06-23 12:14                                 ` Lucas Stach
2022-06-23 12:52                                   ` Christian König
2022-06-23 15:26                                     ` Lucas Stach
2022-06-24  6:54                                       ` Christian König
2022-06-24  8:10                                         ` Lucas Stach
2022-06-27 13:54                       ` Nicolas Dufresne
2022-06-27 14:06                         ` Lucas Stach
2022-06-27 14:30                           ` Nicolas Dufresne
2022-06-27 13:51                   ` Nicolas Dufresne
2022-06-23  8:13 ` Thomas Zimmermann
2022-06-23  8:26   ` Christian König
2022-06-23  8:42     ` Thomas Zimmermann
2022-08-09 15:01 ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=708e27755317a7650ca08ba2e4c14691ac0d6ba2.camel@pengutronix.de \
    --to=l.stach@pengutronix.de \
    --cc=Shashank.Sharma@amd.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=nicolas@ndufresne.ca \
    --cc=ppaalanen@gmail.com \
    --cc=sumit.semwal@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®