On Tue, 2023-09-12 at 11:05 -0400, Nicolas Dufresne wrote:

 	 
External email : Please do not click links or open attachments until
you have verified the sender or the content.
 Le mardi 12 septembre 2023 à 08:47 +0000, Yong Wu (吴勇) a écrit :

quoted

On Mon, 2023-09-11 at 12:12 -0400, Nicolas Dufresne wrote:

quoted

  
External email : Please do not click links or open attachments

until

quoted

quoted

you have verified the sender or the content.
 Hi,

Le lundi 11 septembre 2023 à 10:30 +0800, Yong Wu a écrit :

quoted

From: John Stultz <jstultz@google.com>

This allows drivers who don't want to create their own
DMA-BUF exporter to be able to allocate DMA-BUFs directly
from existing DMA-BUF Heaps.

There is some concern that the premise of DMA-BUF heaps is
that userland knows better about what type of heap memory
is needed for a pipeline, so it would likely be best for
drivers to import and fill DMA-BUFs allocated by userland
instead of allocating one themselves, but this is still
up for debate.

Would be nice for the reviewers to provide the information about

the

quoted

quoted

user of
this new in-kernel API. I noticed it because I was CCed, but
strangely it didn't
make it to the mailing list yet and its not clear in the cover

what

quoted

quoted

this is used
with. 

I can explain in my words though, my read is that this is used to
allocate both
user visible and driver internal memory segments in MTK VCODEC
driver.

I'm somewhat concerned that DMABuf objects are used to abstract
secure memory
allocation from tee. For framebuffers that are going to be

exported

quoted

quoted

and shared
its probably fair use, but it seems that internal shared memory

and

quoted

quoted

codec
specific reference buffers also endup with a dmabuf fd (often

called

quoted

quoted

a secure fd
in the v4l2 patchset) for data that is not being shared, and

requires

quoted

quoted

a 1:1
mapping to a tee handle anyway. Is that the design we'd like to
follow ? 

Yes. basically this is right.

quoted

Can't
we directly allocate from the tee, adding needed helper to make

this

quoted

quoted

as simple
as allocating from a HEAP ?

If this happens, the memory will always be inside TEE. Here we

create a

quoted

new _CMA heap, it will cma_alloc/free dynamically. Reserve it

before

quoted

SVP start, and release to kernel after SVP done.

Ok, I see the benefit of having a common driver then. It would add to
the
complexity, but having a driver for the tee allocator and v4l2/heaps
would be
another option?

It's ok for v4l2. But our DRM also use this new heap and it will be
sent upstream in the next few days.

quoted

  
Secondly. the v4l2/drm has the mature driver control flow, like
drm_gem_prime_import_dev that always use dma_buf ops. So we can use

the

quoted

current flow as much as possible without having to re-plan a flow

in

quoted

the TEE.

From what I've read from Yunfei series, this is only partially true
for V4L2.
The vb2 queue MMAP feature have dmabuf exportation as optional, but
its not a
problem to always back it up with a dmabuf object. But for internal
SHM buffers
used for firmware communication, I've never seen any driver use a
DMABuf.

Same applies for primary decode buffers when frame buffer compression
or post-
processing it used (or reconstruction buffer in encoders), these are
not user
visible and are usually not DMABuf.

If they aren't dmabuf, of course it is ok. I guess we haven't used
these. The SHM buffer is got by tee_shm_register_kernel_buf in this
case and we just use the existed dmabuf ops to complete SVP.

In our case, the vcodec input/output/working buffers and DRM input
buffer all use this new secure heap during secure video play.

quoted

quoted

Nicolas

quoted

Signed-off-by: John Stultz <jstultz@google.com>
Signed-off-by: T.J. Mercier <tjmercier@google.com>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
[Yong: Fix the checkpatch alignment warning]
---
 drivers/dma-buf/dma-heap.c | 60 ++++++++++++++++++++++++++++

----

quoted

quoted

------

quoted

 include/linux/dma-heap.h   | 25 ++++++++++++++++
 2 files changed, 69 insertions(+), 16 deletions(-)

[snip]