Hi
Am 16.10.25 um 11:40 schrieb Boris Brezillon:
Hi Thomas,
On Thu, 16 Oct 2025 10:32:46 +0200
Thomas Zimmermann <[email protected]> wrote:
Hi,
on patches 2 to 4: sync is really begin/end access wrapped into one
interface, which I find questionable. I also don't like that these
patches add generic infrastructure for a single driver.
It's actually two drivers (panfrost and panthor), and the interface is
here so other drivers relying on drm_gem_shmem don't have to hand-roll
these things in the future.
Ok.
But what about the unrelated drivers? Most GEM SHMEM drivers don't need
this. Looking at patch 4, there's a for loop over the SG table, which
appears to affect all drivers.
You know, there are two types of GEM SHMEM users. The ones like panthor
that build upon it with additional features. And the other type that
only use it as simple buffer storage. The former type what's building
blocks to combine as needed; that latter (actually the majority) wants a
simple solution. And TBH, I've considered spitting GEM SHMEM into two
for this reason.
My proposal is to make your own dma_buf exporter in panthor and set the
begin/end_cpu_access functions as you need. Panthor already contains its
own GEM export helper at [1]. If you inline drm_gem_prime_export() [2]
you can set the cpu_access callbacks to panthor-specific code. The other
dma-buf helpers' symbols should be exported and can be re-used. That is
a lot less intrusive and should provide what you need.
I can of course do that in panthor, but then I'll have to duplicate
the same logic in panfrost. Also, the whole point of making it generic
is so that people don't forget that begin/end_cpu_access() is a thing
they should consider (like happened to me if you look at v2 of this
patchset), otherwise importers of their buffers might have unpleasant
side effects because of missing flush/invalidates. This, IMHO, is a good
reason to have it as a drm_gem_funcs::sync() callback. That, or we
decide that the default dma_buf_ops is not a thing, and we force
developers to think twice when they select the default hooks to pick
for their dma_buf implementation.
See my comment above. Panthor and panfrost should treat GEM SHMEM like a
set of building blocks rather and a full solution.
I found that a hand full of other DRM drivers implement dma-buf's
begin/end access callbacks. If you want a generic begin/end interface
for GEM, you certainly want to get them on board. If there's something
common to share, this should be done in a separate series.
Fair enough. I'll try to convert freedreno and imagination to this new
interface, and gather some feedback.
I did 'git grep \.begin_cpu_access' in the drm directory. This returned,
amdgpu, i915, tegra, omap, and xe. These where the ones I had in mind.
Best regards
Thomas
Regards,
Boris
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
