2023-12-06 08:16:14

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,00/21] add driver to support secure video decoder

The patch series used to enable secure video playback (SVP) on MediaTek
hardware in the Linux kernel.

Memory Definitions:
secure memory - Memory allocated in the TEE (Trusted Execution
Environment) which is inaccessible in the REE (Rich Execution
Environment, i.e. linux kernel/userspace).
secure handle - Integer value which acts as reference to 'secure
memory'. Used in communication between TEE and REE to reference
'secure memory'.
secure buffer - 'secure memory' that is used to store decrypted,
compressed video or for other general purposes in the TEE.
secure surface - 'secure memory' that is used to store graphic buffers.

Memory Usage in SVP:
The overall flow of SVP starts with encrypted video coming in from an
outside source into the REE. The REE will then allocate a 'secure
buffer' and send the corresponding 'secure handle' along with the
encrypted, compressed video data to the TEE. The TEE will then decrypt
the video and store the result in the 'secure buffer'. The REE will
then allocate a 'secure surface'. The REE will pass the 'secure
handles' for both the 'secure buffer' and 'secure surface' into the
TEE for video decoding. The video decoder HW will then decode the
contents of the 'secure buffer' and place the result in the 'secure
surface'. The REE will then attach the 'secure surface' to the overlay
plane for rendering of the video.

Everything relating to ensuring security of the actual contents of the
'secure buffer' and 'secure surface' is out of scope for the REE and
is the responsibility of the TEE.

This patch series is consists of four parts. The first is from Jeffrey,
adding secure memory flag in v4l2 framework to support request secure
buffer.

The second and third parts are from John and T.J, adding some heap
interfaces, then our kernel users could allocate buffer from special
heap. The patch v1 is inside below dmabuf link.
https://lore.kernel.org/linux-mediatek/[email protected]/
To avoid confusing, move them into vcodec patchset since we use the
new interfaces directly.

The fourth part is mediatek decoder driver, adding tee interface to
support secure video decoder.

---
Changed in v3:
- rewrite the cover-letter of this patch series
- disable irq for svp mode
- rebase the driver based on the latest media stage

Changed in v2:
- remove setting decoder mode and getting secure handle from decode
- add Jeffrey's patch
- add John and T.J's patch
- getting secure flag with request buffer
- fix some comments from patch v1
---
Jeffrey Kardatzke (4):
v4l2: add secure memory flags
v4l2: handle secure memory flags in queue setup
v4l2: verify secure dmabufs are used in secure queue
v4l: add documentation for secure memory flag

John Stultz (2):
dma-heap: Add proper kref handling on dma-buf heaps
dma-heap: Provide accessors so that in-kernel drivers can allocate
dmabufs from specific heaps

T.J. Mercier (1):
dma-buf: heaps: Deduplicate docs and adopt common format

Yunfei Dong (14):
media: mediatek: vcodec: add tee client interface to communiate with
optee-os
media: mediatek: vcodec: allocate tee share memory
media: mediatek: vcodec: send share memory data to optee
media: mediatek: vcodec: initialize msg and vsi information
media: mediatek: vcodec: add interface to allocate/free secure memory
media: mediatek: vcodec: using shared memory as vsi address
media: mediatek: vcodec: Add capture format to support one plane
memory
media: mediatek: vcodec: Add one plane format
media: medkatek: vcodec: support one plane capture buffer
media: medkatek: vcodec: re-construct h264 driver to support svp mode
media: medkatek: vcodec: remove parse nal_info in kernel
media: medkatek: vcodec: disable wait interrupt for svp mode
media: medkatek: vcodec: support tee decoder
media: mediatek: vcodec: move vdec init interface to setup callback

.../userspace-api/media/v4l/buffer.rst | 8 +-
.../media/v4l/pixfmt-reserved.rst | 8 +
drivers/dma-buf/dma-heap.c | 139 +++++--
.../media/common/videobuf2/videobuf2-core.c | 23 ++
.../common/videobuf2/videobuf2-dma-contig.c | 6 +
.../media/common/videobuf2/videobuf2-dma-sg.c | 6 +
.../media/common/videobuf2/videobuf2-v4l2.c | 34 +-
.../media/platform/mediatek/vcodec/Kconfig | 1 +
.../mediatek/vcodec/common/mtk_vcodec_util.c | 122 +++++-
.../mediatek/vcodec/common/mtk_vcodec_util.h | 3 +
.../platform/mediatek/vcodec/decoder/Makefile | 1 +
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 155 ++++---
.../vcodec/decoder/mtk_vcodec_dec_drv.c | 8 +
.../vcodec/decoder/mtk_vcodec_dec_drv.h | 7 +
.../vcodec/decoder/mtk_vcodec_dec_hw.c | 34 +-
.../vcodec/decoder/mtk_vcodec_dec_optee.c | 383 ++++++++++++++++++
.../vcodec/decoder/mtk_vcodec_dec_optee.h | 156 +++++++
.../vcodec/decoder/mtk_vcodec_dec_pm.c | 6 +-
.../vcodec/decoder/mtk_vcodec_dec_stateless.c | 35 +-
.../decoder/vdec/vdec_h264_req_common.c | 11 +-
.../decoder/vdec/vdec_h264_req_multi_if.c | 334 +++++++++------
.../mediatek/vcodec/decoder/vdec_drv_if.c | 4 +-
.../mediatek/vcodec/decoder/vdec_msg_queue.c | 9 +-
.../mediatek/vcodec/decoder/vdec_vpu_if.c | 57 ++-
.../mediatek/vcodec/decoder/vdec_vpu_if.h | 4 +
drivers/media/v4l2-core/v4l2-common.c | 2 +
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/linux/dma-heap.h | 29 +-
include/media/videobuf2-core.h | 8 +-
include/uapi/linux/videodev2.h | 3 +
30 files changed, 1296 insertions(+), 301 deletions(-)
create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h

--
2.18.0


2023-12-06 08:16:25

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,01/21] v4l2: add secure memory flags

From: Jeffrey Kardatzke <[email protected]>

Adds a V4L2 flag which indicates that a queue is using secure dmabufs
and the corresponding capability flag.

Signed-off-by: Jeffrey Kardatzke <[email protected]>
Signed-off-by: Yunfei Dong <[email protected]>
---
include/media/videobuf2-core.h | 8 +++++++-
include/uapi/linux/videodev2.h | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5557d78b6f20..98eba43661c1 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -518,6 +518,9 @@ struct vb2_buf_ops {
* ->finish().
* @non_coherent_mem: when set queue will attempt to allocate buffers using
* non-coherent memory.
+ * @allow_secure_mem: when set user-space can pass the %V4L2_MEMORY_FLAG_SECURE
+ * flag to indicate the dma bufs are secure.
+ * @secure_mem: when set queue will verify that the dma bufs are secure.
* @lock: pointer to a mutex that protects the &struct vb2_queue. The
* driver can set this to a mutex to let the v4l2 core serialize
* the queuing ioctls. If the driver wants to handle locking
@@ -601,6 +604,8 @@ struct vb2_queue {
unsigned int uses_requests:1;
unsigned int allow_cache_hints:1;
unsigned int non_coherent_mem:1;
+ unsigned int allow_secure_mem:1;
+ unsigned int secure_mem:1;

struct mutex *lock;
void *owner;
@@ -770,7 +775,8 @@ void vb2_core_querybuf(struct vb2_queue *q, struct vb2_buffer *vb, void *pb);
* @q: pointer to &struct vb2_queue with videobuf2 queue.
* @memory: memory type, as defined by &enum vb2_memory.
* @flags: auxiliary queue/buffer management flags. Currently, the only
- * used flag is %V4L2_MEMORY_FLAG_NON_COHERENT.
+ * used flags are %V4L2_MEMORY_FLAG_NON_COHERENT and
+ * %V4L2_MEMORY_FLAG_SECURE.
* @count: requested buffer count.
*
* Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 68e7ac178cc2..570b11552818 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1026,6 +1026,7 @@ struct v4l2_requestbuffers {
};

#define V4L2_MEMORY_FLAG_NON_COHERENT (1 << 0)
+#define V4L2_MEMORY_FLAG_SECURE (1 << 1)

/* capabilities for struct v4l2_requestbuffers and v4l2_create_buffers */
#define V4L2_BUF_CAP_SUPPORTS_MMAP (1 << 0)
@@ -1036,6 +1037,7 @@ struct v4l2_requestbuffers {
#define V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF (1 << 5)
#define V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS (1 << 6)
#define V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS (1 << 7)
+#define V4L2_BUF_CAP_SUPPORTS_SECURE_MEM (1 << 8)

/**
* struct v4l2_plane - plane info for multi-planar buffers
--
2.18.0

2023-12-06 08:16:38

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,02/21] v4l2: handle secure memory flags in queue setup

From: Jeffrey Kardatzke <[email protected]>

Validates the secure memory flags when setting up a queue and ensures
the queue has the proper capability.

Signed-off-by: Jeffrey Kardatzke <[email protected]>
Signed-off-by: Yunfei Dong <[email protected]>
---
.../media/common/videobuf2/videobuf2-core.c | 23 +++++++++++++
.../media/common/videobuf2/videobuf2-v4l2.c | 34 +++++++++++++------
2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 8c1df829745b..09dc030484be 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -813,6 +813,15 @@ static bool verify_coherency_flags(struct vb2_queue *q, bool non_coherent_mem)
return true;
}

+static bool verify_secure_mem_flags(struct vb2_queue *q, bool secure_mem)
+{
+ if (secure_mem != q->secure_mem) {
+ dprintk(q, 1, "secure memory model mismatch\n");
+ return false;
+ }
+ return true;
+}
+
int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int flags, unsigned int *count)
{
@@ -820,6 +829,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned int q_num_bufs = vb2_get_num_buffers(q);
unsigned plane_sizes[VB2_MAX_PLANES] = { };
bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
+ bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
unsigned int i;
int ret = 0;

@@ -836,6 +846,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
if (*count == 0 || q_num_bufs != 0 ||
(q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) ||
!verify_coherency_flags(q, non_coherent_mem)) {
+ bool no_previous_buffers = !q->num_buffers;
+
/*
* We already have buffers allocated, so first check if they
* are not in use and can be freed.
@@ -854,6 +866,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
__vb2_queue_free(q, q_num_bufs);
mutex_unlock(&q->mmap_lock);

+ /*
+ * Do not allow switching secure buffer mode.
+ */
+ if (!no_previous_buffers && !verify_secure_mem_flags(q, secure_mem))
+ return -EINVAL;
+
/*
* In case of REQBUFS(0) return immediately without calling
* driver's queue_setup() callback and allocating resources.
@@ -882,6 +900,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
if (ret)
return ret;
set_queue_coherency(q, non_coherent_mem);
+ q->secure_mem = secure_mem;

/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -986,6 +1005,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
unsigned plane_sizes[VB2_MAX_PLANES] = { };
bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
unsigned int q_num_bufs = vb2_get_num_buffers(q);
+ bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
bool no_previous_buffers = !q_num_bufs;
int ret = 0;

@@ -1015,6 +1035,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return ret;
q->waiting_for_buffers = !q->is_output;
set_queue_coherency(q, non_coherent_mem);
+ q->secure_mem = secure_mem;
} else {
if (q->memory != memory) {
dprintk(q, 1, "memory model mismatch\n");
@@ -1022,6 +1043,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
}
if (!verify_coherency_flags(q, non_coherent_mem))
return -EINVAL;
+ if (!verify_secure_mem_flags(q, secure_mem))
+ return -EINVAL;
}

num_buffers = min(*count, q->max_num_buffers - q_num_bufs);
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 54d572c3b515..0a530830276c 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -686,22 +686,30 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
*caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
if (q->supports_requests)
*caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
+ if (q->allow_secure_mem && q->io_modes & VB2_DMABUF)
+ *caps |= V4L2_BUF_CAP_SUPPORTS_SECURE_MEM;
}

-static void validate_memory_flags(struct vb2_queue *q,
+static bool validate_memory_flags(struct vb2_queue *q,
int memory,
u32 *flags)
{
+ if (*flags & V4L2_MEMORY_FLAG_SECURE &&
+ (!q->allow_secure_mem || memory != V4L2_MEMORY_DMABUF)) {
+ return false;
+ }
+
if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) {
/*
- * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only,
- * but in order to avoid bugs we zero out all bits.
+ * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only.
*/
- *flags = 0;
- } else {
- /* Clear all unknown flags. */
- *flags &= V4L2_MEMORY_FLAG_NON_COHERENT;
+ *flags &= ~V4L2_MEMORY_FLAG_NON_COHERENT;
}
+
+ /* Clear all unknown flags. */
+ *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | V4L2_MEMORY_FLAG_SECURE;
+
+ return true;
}

int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
@@ -710,7 +718,8 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
u32 flags = req->flags;

fill_buf_caps(q, &req->capabilities);
- validate_memory_flags(q, req->memory, &flags);
+ if (!validate_memory_flags(q, req->memory, &flags))
+ return -EINVAL;
req->flags = flags;
return ret ? ret : vb2_core_reqbufs(q, req->memory,
req->flags, &req->count);
@@ -752,7 +761,8 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
unsigned i;

fill_buf_caps(q, &create->capabilities);
- validate_memory_flags(q, create->memory, &create->flags);
+ if (!validate_memory_flags(q, create->memory, &create->flags))
+ return -EINVAL;
create->index = vb2_get_num_buffers(q);
create->max_num_buffers = q->max_num_buffers;
create->capabilities |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS;
@@ -1007,7 +1017,8 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
u32 flags = p->flags;

fill_buf_caps(vdev->queue, &p->capabilities);
- validate_memory_flags(vdev->queue, p->memory, &flags);
+ if (!validate_memory_flags(vdev->queue, p->memory, &flags))
+ return -EINVAL;
p->flags = flags;
if (res)
return res;
@@ -1031,7 +1042,8 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,

p->index = vdev->queue->num_buffers;
fill_buf_caps(vdev->queue, &p->capabilities);
- validate_memory_flags(vdev->queue, p->memory, &p->flags);
+ if (!validate_memory_flags(vdev->queue, p->memory, &p->flags))
+ return -EINVAL;
/*
* If count == 0, then just check if memory and type are valid.
* Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
--
2.18.0

2023-12-06 08:16:42

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,07/21] dma-heap: Provide accessors so that in-kernel drivers can allocate dmabufs from specific heaps

From: John Stultz <[email protected]>

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.

Signed-off-by: John Stultz <[email protected]>
Signed-off-by: T.J. Mercier <[email protected]>
Signed-off-by: Yong Wu <[email protected]>
[Yong: Fix the checkpatch alignment warning]
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/dma-buf/dma-heap.c | 83 ++++++++++++++++++++++++++++++--------
include/linux/dma-heap.h | 6 +++
2 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 97025ee8500f..6efe833a4b10 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -51,12 +51,24 @@ static dev_t dma_heap_devt;
static struct class *dma_heap_class;
static DEFINE_XARRAY_ALLOC(dma_heap_minors);

-static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
- unsigned int fd_flags,
- unsigned int heap_flags)
+/**
+ * dma_heap_buffer_alloc - Allocate dma-buf from a dma_heap
+ * @heap: DMA-Heap to allocate from
+ * @len: size to allocate in bytes
+ * @fd_flags: flags to set on returned dma-buf fd
+ * @heap_flags: flags to pass to the dma heap
+ *
+ * This is for internal dma-buf allocations only. Free returned buffers with dma_buf_put().
+ */
+struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
+ unsigned int fd_flags,
+ unsigned int heap_flags)
{
- struct dma_buf *dmabuf;
- int fd;
+ if (fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
+ return ERR_PTR(-EINVAL);
+
+ if (heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
+ return ERR_PTR(-EINVAL);

/*
* Allocations from all heaps have to begin
@@ -64,9 +76,20 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
*/
len = PAGE_ALIGN(len);
if (!len)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
+
+ return heap->ops->allocate(heap, len, fd_flags, heap_flags);
+}
+EXPORT_SYMBOL_GPL(dma_heap_buffer_alloc);

- dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
+static int dma_heap_bufferfd_alloc(struct dma_heap *heap, size_t len,
+ unsigned int fd_flags,
+ unsigned int heap_flags)
+{
+ struct dma_buf *dmabuf;
+ int fd;
+
+ dmabuf = dma_heap_buffer_alloc(heap, len, fd_flags, heap_flags);
if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);

@@ -104,15 +127,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
if (heap_allocation->fd)
return -EINVAL;

- if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
- return -EINVAL;
-
- if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
- return -EINVAL;
-
- fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
- heap_allocation->fd_flags,
- heap_allocation->heap_flags);
+ fd = dma_heap_bufferfd_alloc(heap, heap_allocation->len,
+ heap_allocation->fd_flags,
+ heap_allocation->heap_flags);
if (fd < 0)
return fd;

@@ -205,6 +222,7 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
{
return heap->priv;
}
+EXPORT_SYMBOL_GPL(dma_heap_get_drvdata);

/**
* dma_heap_get_name - get heap name
@@ -217,6 +235,7 @@ const char *dma_heap_get_name(struct dma_heap *heap)
{
return heap->name;
}
+EXPORT_SYMBOL_GPL(dma_heap_get_name);

/**
* dma_heap_add - adds a heap to dmabuf heaps
@@ -307,6 +326,37 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
kfree(heap);
return err_ret;
}
+EXPORT_SYMBOL_GPL(dma_heap_add);
+
+/**
+ * dma_heap_find - get the heap registered with the specified name
+ * @name: Name of the DMA-Heap to find
+ *
+ * Returns:
+ * The DMA-Heap with the provided name.
+ *
+ * NOTE: DMA-Heaps returned from this function MUST be released using
+ * dma_heap_put() when the user is done to enable the heap to be unloaded.
+ */
+struct dma_heap *dma_heap_find(const char *name)
+{
+ struct dma_heap *h;
+
+ mutex_lock(&heap_list_lock);
+ list_for_each_entry(h, &heap_list, list) {
+ if (!kref_get_unless_zero(&h->refcount))
+ continue;
+
+ if (!strcmp(h->name, name)) {
+ mutex_unlock(&heap_list_lock);
+ return h;
+ }
+ dma_heap_put(h);
+ }
+ mutex_unlock(&heap_list_lock);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(dma_heap_find);

static void dma_heap_release(struct kref *ref)
{
@@ -332,6 +382,7 @@ void dma_heap_put(struct dma_heap *heap)
{
kref_put(&heap->refcount, dma_heap_release);
}
+EXPORT_SYMBOL_GPL(dma_heap_put);

static char *dma_heap_devnode(const struct device *dev, umode_t *mode)
{
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index d57593f8a1bc..3cbf9bff2346 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -46,6 +46,12 @@ const char *dma_heap_get_name(struct dma_heap *heap);

struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);

+struct dma_heap *dma_heap_find(const char *name);
+
void dma_heap_put(struct dma_heap *heap);

+struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
+ unsigned int fd_flags,
+ unsigned int heap_flags);
+
#endif /* _DMA_HEAPS_H */
--
2.18.0

2023-12-06 08:16:43

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,06/21] dma-heap: Add proper kref handling on dma-buf heaps

From: John Stultz <[email protected]>

Add proper refcounting on the dma_heap structure.
While existing heaps are built-in, we may eventually
have heaps loaded from modules, and we'll need to be
able to properly handle the references to the heaps

Signed-off-by: John Stultz <[email protected]>
Signed-off-by: T.J. Mercier <[email protected]>
Signed-off-by: Yong Wu <[email protected]>
[Yong: Just add comment for "minor" and "refcount"]
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/dma-buf/dma-heap.c | 29 +++++++++++++++++++++++++++++
include/linux/dma-heap.h | 2 ++
2 files changed, 31 insertions(+)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 22f6c193db0d..97025ee8500f 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -11,6 +11,7 @@
#include <linux/dma-buf.h>
#include <linux/dma-heap.h>
#include <linux/err.h>
+#include <linux/kref.h>
#include <linux/list.h>
#include <linux/nospec.h>
#include <linux/syscalls.h>
@@ -30,6 +31,7 @@
* @heap_devt: heap device node
* @list: list head connecting to list of heaps
* @heap_cdev: heap char device
+ * @refcount: reference counter for this heap device
*
* Represents a heap of memory from which buffers can be made.
*/
@@ -40,6 +42,7 @@ struct dma_heap {
dev_t heap_devt;
struct list_head list;
struct cdev heap_cdev;
+ struct kref refcount;
};

static LIST_HEAD(heap_list);
@@ -240,6 +243,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
if (!heap)
return ERR_PTR(-ENOMEM);

+ kref_init(&heap->refcount);
heap->name = exp_info->name;
heap->ops = exp_info->ops;
heap->priv = exp_info->priv;
@@ -304,6 +308,31 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
return err_ret;
}

+static void dma_heap_release(struct kref *ref)
+{
+ struct dma_heap *heap = container_of(ref, struct dma_heap, refcount);
+ unsigned int minor = MINOR(heap->heap_devt);
+
+ mutex_lock(&heap_list_lock);
+ list_del(&heap->list);
+ mutex_unlock(&heap_list_lock);
+
+ device_destroy(dma_heap_class, heap->heap_devt);
+ cdev_del(&heap->heap_cdev);
+ xa_erase(&dma_heap_minors, minor);
+
+ kfree(heap);
+}
+
+/**
+ * dma_heap_put - drops a reference to a dmabuf heap, potentially freeing it
+ * @heap: DMA-Heap whose reference count to decrement
+ */
+void dma_heap_put(struct dma_heap *heap)
+{
+ kref_put(&heap->refcount, dma_heap_release);
+}
+
static char *dma_heap_devnode(const struct device *dev, umode_t *mode)
{
return kasprintf(GFP_KERNEL, "dma_heap/%s", dev_name(dev));
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index fbe86ec889a8..d57593f8a1bc 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -46,4 +46,6 @@ const char *dma_heap_get_name(struct dma_heap *heap);

struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);

+void dma_heap_put(struct dma_heap *heap);
+
#endif /* _DMA_HEAPS_H */
--
2.18.0

2023-12-06 08:16:49

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,03/21] v4l2: verify secure dmabufs are used in secure queue

From: Jeffrey Kardatzke <[email protected]>

Verfies in the dmabuf implementations that if the secure memory flag is
set for a queue that the dmabuf submitted to the queue is unmappable.

Signed-off-by: Jeffrey Kardatzke <[email protected]>
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/media/common/videobuf2/videobuf2-dma-contig.c | 6 ++++++
drivers/media/common/videobuf2/videobuf2-dma-sg.c | 6 ++++++
2 files changed, 12 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 3d4fd4ef5310..ad58ef8dc231 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -710,6 +710,12 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
return -EINVAL;
}

+ /* verify the dmabuf is secure if we are in secure mode */
+ if (buf->vb->vb2_queue->secure_mem && sg_page(sgt->sgl)) {
+ pr_err("secure queue requires secure dma_buf");
+ return -EINVAL;
+ }
+
/* checking if dmabuf is big enough to store contiguous chunk */
contig_size = vb2_dc_get_contiguous_size(sgt);
if (contig_size < buf->size) {
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index 28f3fdfe23a2..55428c73c380 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -564,6 +564,12 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv)
return -EINVAL;
}

+ /* verify the dmabuf is secure if we are in secure mode */
+ if (buf->vb->vb2_queue->secure_mem && !sg_dma_secure(sgt->sgl)) {
+ pr_err("secure queue requires secure dma_buf");
+ return -EINVAL;
+ }
+
buf->dma_sgt = sgt;
buf->vaddr = NULL;

--
2.18.0

2023-12-06 08:16:50

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,12/21] media: mediatek: vcodec: add interface to allocate/free secure memory

Need to call dma heap interface to allocate/free secure memory when playing
secure video.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../media/platform/mediatek/vcodec/Kconfig | 1 +
.../mediatek/vcodec/common/mtk_vcodec_util.c | 122 +++++++++++++++++-
.../mediatek/vcodec/common/mtk_vcodec_util.h | 3 +
3 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig b/drivers/media/platform/mediatek/vcodec/Kconfig
index bc8292232530..707865703e61 100644
--- a/drivers/media/platform/mediatek/vcodec/Kconfig
+++ b/drivers/media/platform/mediatek/vcodec/Kconfig
@@ -17,6 +17,7 @@ config VIDEO_MEDIATEK_VCODEC
depends on VIDEO_MEDIATEK_VPU || !VIDEO_MEDIATEK_VPU
depends on MTK_SCP || !MTK_SCP
depends on MTK_SMI || (COMPILE_TEST && MTK_SMI=n)
+ depends on DMABUF_HEAPS
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
index 9ce34a3b5ee6..2c44747f77a0 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.c
@@ -5,9 +5,11 @@
* Tiffany Lin <[email protected]>
*/

+#include <linux/dma-heap.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/regmap.h>
+#include <uapi/linux/dma-heap.h>

#include "../decoder/mtk_vcodec_dec_drv.h"
#include "../encoder/mtk_vcodec_enc_drv.h"
@@ -45,7 +47,7 @@ int mtk_vcodec_write_vdecsys(struct mtk_vcodec_dec_ctx *ctx, unsigned int reg,
}
EXPORT_SYMBOL(mtk_vcodec_write_vdecsys);

-int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem)
+static int mtk_vcodec_mem_alloc_nor(void *priv, struct mtk_vcodec_mem *mem)
{
enum mtk_instance_type inst_type = *((unsigned int *)priv);
struct platform_device *plat_dev;
@@ -76,9 +78,71 @@ int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem)

return 0;
}
-EXPORT_SYMBOL(mtk_vcodec_mem_alloc);

-void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem)
+static int mtk_vcodec_mem_alloc_sec(struct mtk_vcodec_dec_ctx *ctx, struct mtk_vcodec_mem *mem)
+{
+ struct device *dev = &ctx->dev->plat_dev->dev;
+ struct dma_buf *dma_buffer;
+ struct dma_heap *vdec_heap;
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
+ unsigned long size = mem->size;
+ int ret = 0;
+
+ if (!size)
+ return -EINVAL;
+
+ vdec_heap = dma_heap_find("mtk_svp");
+ if (!vdec_heap) {
+ mtk_v4l2_vdec_err(ctx, "dma heap find failed!");
+ return -EPERM;
+ }
+
+ dma_buffer = dma_heap_buffer_alloc(vdec_heap, size, DMA_HEAP_VALID_FD_FLAGS,
+ DMA_HEAP_VALID_HEAP_FLAGS);
+ if (IS_ERR_OR_NULL(dma_buffer)) {
+ mtk_v4l2_vdec_err(ctx, "dma heap alloc size=0x%lx failed!", size);
+ return PTR_ERR(dma_buffer);
+ }
+
+ attach = dma_buf_attach(dma_buffer, dev);
+ if (IS_ERR_OR_NULL(attach)) {
+ mtk_v4l2_vdec_err(ctx, "dma attach size=0x%lx failed!", size);
+ ret = PTR_ERR(attach);
+ goto err_attach;
+ }
+
+ sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
+ if (IS_ERR_OR_NULL(sgt)) {
+ mtk_v4l2_vdec_err(ctx, "dma map attach size=0x%lx failed!", size);
+ ret = PTR_ERR(sgt);
+ goto err_sgt;
+ }
+
+ mem->va = dma_buffer;
+ mem->dma_addr = (dma_addr_t)sg_dma_address((sgt)->sgl);
+
+ if (!mem->va || !mem->dma_addr) {
+ mtk_v4l2_vdec_err(ctx, "dma buffer size=0x%lx failed!", size);
+ ret = -EPERM;
+ goto err_addr;
+ }
+
+ mem->attach = attach;
+ mem->sgt = sgt;
+
+ return 0;
+err_addr:
+ dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
+err_sgt:
+ dma_buf_detach(dma_buffer, attach);
+err_attach:
+ dma_buf_put(dma_buffer);
+
+ return ret;
+}
+
+static void mtk_vcodec_mem_free_nor(void *priv, struct mtk_vcodec_mem *mem)
{
enum mtk_instance_type inst_type = *((unsigned int *)priv);
struct platform_device *plat_dev;
@@ -111,6 +175,57 @@ void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem)
mem->dma_addr = 0;
mem->size = 0;
}
+
+static void mtk_vcodec_mem_free_sec(struct mtk_vcodec_mem *mem)
+{
+ if (mem->sgt)
+ dma_buf_unmap_attachment(mem->attach, mem->sgt, DMA_BIDIRECTIONAL);
+ dma_buf_detach((struct dma_buf *)mem->va, mem->attach);
+ dma_buf_put((struct dma_buf *)mem->va);
+
+ mem->attach = NULL;
+ mem->sgt = NULL;
+ mem->va = NULL;
+ mem->dma_addr = 0;
+ mem->size = 0;
+}
+
+int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem)
+{
+ enum mtk_instance_type inst_type = *((unsigned int *)priv);
+ int ret;
+
+ if (inst_type == MTK_INST_DECODER) {
+ struct mtk_vcodec_dec_ctx *dec_ctx = priv;
+
+ if (dec_ctx->is_secure_playback) {
+ ret = mtk_vcodec_mem_alloc_sec(dec_ctx, mem);
+ goto alloc_end;
+ }
+ }
+
+ ret = mtk_vcodec_mem_alloc_nor(priv, mem);
+alloc_end:
+
+ return ret;
+}
+EXPORT_SYMBOL(mtk_vcodec_mem_alloc);
+
+void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem)
+{
+ enum mtk_instance_type inst_type = *((unsigned int *)priv);
+
+ if (inst_type == MTK_INST_DECODER) {
+ struct mtk_vcodec_dec_ctx *dec_ctx = priv;
+
+ if (dec_ctx->is_secure_playback) {
+ mtk_vcodec_mem_free_sec(mem);
+ return;
+ }
+ }
+
+ mtk_vcodec_mem_free_nor(priv, mem);
+}
EXPORT_SYMBOL(mtk_vcodec_mem_free);

void *mtk_vcodec_get_hw_dev(struct mtk_vcodec_dec_dev *dev, int hw_idx)
@@ -172,3 +287,4 @@ EXPORT_SYMBOL(mtk_vcodec_get_curr_ctx);

MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Mediatek video codec driver");
+MODULE_IMPORT_NS(DMA_BUF);
diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.h
index 85f615cdd4d3..22078e757ed0 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.h
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_util.h
@@ -18,6 +18,9 @@ struct mtk_vcodec_mem {
size_t size;
void *va;
dma_addr_t dma_addr;
+
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
};

struct mtk_vcodec_fb {
--
2.18.0

2023-12-06 08:16:52

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,11/21] media: mediatek: vcodec: initialize msg and vsi information

Need to initialize msg and vsi information before sending to optee-os, then
calling optee invoke command to send the information to optee-os.

For the optee communication interface is different with scp, using
flag to separate them.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/mtk_vcodec_dec_drv.h | 2 +
.../mediatek/vcodec/decoder/vdec_vpu_if.c | 49 ++++++++++++++++---
.../mediatek/vcodec/decoder/vdec_vpu_if.h | 4 ++
3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index b1a2107f2a1e..47eca245dc07 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -175,6 +175,7 @@ struct mtk_vcodec_dec_pdata {
* @vpu_inst: vpu instance pointer.
*
* @is_10bit_bitstream: set to true if it's 10bit bitstream
+ * @is_secure_playback: Secure Video Playback (SVP) mode
*/
struct mtk_vcodec_dec_ctx {
enum mtk_instance_type type;
@@ -220,6 +221,7 @@ struct mtk_vcodec_dec_ctx {
void *vpu_inst;

bool is_10bit_bitstream;
+ bool is_secure_playback;
};

/**
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index 82e57ae983d5..5336769a3fb5 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -148,7 +148,10 @@ static void vpu_dec_ipi_handler(void *data, unsigned int len, void *priv)

static int vcodec_vpu_send_msg(struct vdec_vpu_inst *vpu, void *msg, int len)
{
- int err, id, msgid;
+ struct mtk_vdec_optee_data_to_shm *optee_data;
+ int data_size, id, hw_id, msgid;
+ void *ack_msg, *data_msg;
+ int err;

msgid = *(uint32_t *)msg;
mtk_vdec_debug(vpu->ctx, "id=%X", msgid);
@@ -158,16 +161,46 @@ static int vcodec_vpu_send_msg(struct vdec_vpu_inst *vpu, void *msg, int len)

if (vpu->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_LAT_SINGLE_CORE) {
if (msgid == AP_IPIMSG_DEC_CORE ||
- msgid == AP_IPIMSG_DEC_CORE_END)
+ msgid == AP_IPIMSG_DEC_CORE_END) {
+ optee_data = &vpu->core_optee_info;
id = vpu->core_id;
- else
+ } else {
+ optee_data = &vpu->lat_optee_info;
id = vpu->id;
+ }
} else {
+ optee_data = &vpu->lat_optee_info;
id = vpu->id;
}

- err = mtk_vcodec_fw_ipi_send(vpu->ctx->dev->fw_handler, id, msg,
- len, 2000);
+ if (!vpu->ctx->is_secure_playback) {
+ err = mtk_vcodec_fw_ipi_send(vpu->ctx->dev->fw_handler, id, msg, len, 2000);
+ } else {
+ hw_id = (id == SCP_IPI_VDEC_LAT) ? MTK_VDEC_LAT0 : MTK_VDEC_CORE;
+
+ mtk_vcodec_dec_optee_set_data(optee_data, msg, len, OPTEE_MSG_INDEX);
+
+ /* There is no need to copy the data (VSI) message to shared memory,
+ * but we still need to set the buffer size to a non-zero value.
+ */
+ if (msgid == AP_IPIMSG_DEC_CORE || msgid == AP_IPIMSG_DEC_START) {
+ data_msg = mtk_vcodec_dec_get_shm_buffer_va(vpu->ctx->dev->optee_private,
+ hw_id, OPTEE_DATA_INDEX);
+ data_size = mtk_vcodec_dec_get_shm_buffer_size(vpu->ctx->dev->optee_private,
+ hw_id, OPTEE_DATA_INDEX);
+ mtk_vcodec_dec_optee_set_data(optee_data, data_msg, data_size,
+ OPTEE_DATA_INDEX);
+ }
+
+ err = mtk_vcodec_dec_optee_invokd_cmd(vpu->ctx->dev->optee_private,
+ hw_id, optee_data);
+ vpu->failure = err;
+
+ ack_msg = mtk_vcodec_dec_get_shm_buffer_va(vpu->ctx->dev->optee_private, hw_id,
+ OPTEE_MSG_INDEX);
+ vpu_dec_ipi_handler(ack_msg, 0, vpu->ctx->dev);
+ }
+
if (err) {
mtk_vdec_err(vpu->ctx, "send fail vpu_id=%d msg_id=%X status=%d",
id, msgid, err);
@@ -213,7 +246,11 @@ int vpu_dec_init(struct vdec_vpu_inst *vpu)
return err;
}

- if (vpu->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_LAT_SINGLE_CORE) {
+ /* Using tee interface to communicate with optee os directly for SVP mode,
+ * fw ipi interface is used for normal playback.
+ */
+ if (vpu->ctx->dev->vdec_pdata->hw_arch == MTK_VDEC_LAT_SINGLE_CORE &&
+ !vpu->ctx->is_secure_playback) {
err = mtk_vcodec_fw_ipi_register(vpu->ctx->dev->fw_handler,
vpu->core_id, vpu->handler,
"vdec", vpu->ctx->dev);
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.h b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.h
index fbb3f34a73f0..946e5abcc7d3 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.h
@@ -28,6 +28,8 @@ struct mtk_vcodec_dec_ctx;
* @codec_type : use codec type to separate different codecs
* @capture_type: used capture type to separate different capture format
* @fb_sz : frame buffer size of each plane
+ * @lat_optee_info : used to send msg to optee shm buffer
+ * @core_optee_info : used to send msg to optee shm buffer
*/
struct vdec_vpu_inst {
int id;
@@ -44,6 +46,8 @@ struct vdec_vpu_inst {
unsigned int codec_type;
unsigned int capture_type;
unsigned int fb_sz[2];
+ struct mtk_vdec_optee_data_to_shm lat_optee_info;
+ struct mtk_vdec_optee_data_to_shm core_optee_info;
};

/**
--
2.18.0

2023-12-06 08:16:52

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,04/21] v4l: add documentation for secure memory flag

From: Jeffrey Kardatzke <[email protected]>

Adds documentation for V4L2_MEMORY_FLAG_SECURE.

Signed-off-by: Jeffrey Kardatzke <[email protected]>
Signed-off-by: Yunfei Dong <[email protected]>
---
Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
index 52bbee81c080..a5a7d1c72d53 100644
--- a/Documentation/userspace-api/media/v4l/buffer.rst
+++ b/Documentation/userspace-api/media/v4l/buffer.rst
@@ -696,7 +696,7 @@ enum v4l2_memory

.. _memory-flags:

-Memory Consistency Flags
+Memory Flags
------------------------

.. raw:: latex
@@ -728,6 +728,12 @@ Memory Consistency Flags
only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
<V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
+ * .. _`V4L2-MEMORY-FLAG-SECURE`:
+
+ - ``V4L2_MEMORY_FLAG_SECURE``
+ - 0x00000002
+ - DMA bufs passed into the queue will be validated to ensure they were
+ allocated from a secure dma-heap.

.. raw:: latex

--
2.18.0

2023-12-06 08:16:59

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,13/21] media: mediatek: vcodec: using shared memory as vsi address

The vsi buffer is allocated by tee share memory for svp mode, need to
use the share memory as the vsi address to store vsi data.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/vdec/vdec_h264_req_multi_if.c | 9 +++++++--
.../media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 8 ++++++--
2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index 0e741e0dc8ba..4967e0f0984d 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -417,8 +417,13 @@ static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)

vsi_size = round_up(sizeof(struct vdec_h264_slice_vsi), VCODEC_DEC_ALIGNED_64);
inst->vsi = inst->vpu.vsi;
- inst->vsi_core =
- (struct vdec_h264_slice_vsi *)(((char *)inst->vpu.vsi) + vsi_size);
+ if (ctx->is_secure_playback)
+ inst->vsi_core =
+ mtk_vcodec_dec_get_shm_buffer_va(ctx->dev->optee_private, MTK_VDEC_CORE,
+ OPTEE_DATA_INDEX);
+ else
+ inst->vsi_core =
+ (struct vdec_h264_slice_vsi *)(((char *)inst->vpu.vsi) + vsi_size);
inst->resolution_changed = true;
inst->realloc_mv_buf = true;

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index 5336769a3fb5..5c31641e9abe 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -18,8 +18,12 @@ static void handle_init_ack_msg(const struct vdec_vpu_ipi_init_ack *msg)

/* mapping VPU address to kernel virtual address */
/* the content in vsi is initialized to 0 in VPU */
- vpu->vsi = mtk_vcodec_fw_map_dm_addr(vpu->ctx->dev->fw_handler,
- msg->vpu_inst_addr);
+ if (vpu->ctx->is_secure_playback)
+ vpu->vsi = mtk_vcodec_dec_get_shm_buffer_va(vpu->ctx->dev->optee_private,
+ MTK_VDEC_LAT0, OPTEE_DATA_INDEX);
+ else
+ vpu->vsi = mtk_vcodec_fw_map_dm_addr(vpu->ctx->dev->fw_handler,
+ msg->vpu_inst_addr);
vpu->inst_addr = msg->vpu_inst_addr;

mtk_vdec_debug(vpu->ctx, "- vpu_inst_addr = 0x%x", vpu->inst_addr);
--
2.18.0

2023-12-06 08:17:01

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,14/21] media: mediatek: vcodec: Add capture format to support one plane memory

Define one uncompressed capture format V4L2_PIX_FMT_MS21 in order to
support one plane memory. The buffer size is luma + chroma, luma is
stored at the start and chrome is stored at the end.

Signed-off-by: Yunfei Dong <[email protected]>
---
Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 8 ++++++++
drivers/media/v4l2-core/v4l2-common.c | 2 ++
drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
include/uapi/linux/videodev2.h | 1 +
4 files changed, 12 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
index 886ba7b08d6b..6ec899649d50 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
@@ -295,6 +295,14 @@ please make a proposal on the linux-media mailing list.
- Compressed format used by Nuvoton NPCM video driver. This format is
defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile
Encoding).
+ * .. _V4L2-PIX-FMT-MS21:
+
+ - ``V4L2_PIX_FMT_MS21``
+ - 'MS21'
+ - This format has one plane, luma and chroma are stored in a contiguous
+ memory. Luma pixel in 16x32 tiles at the start, chroma pixel in 16x16
+ tiles at the end. The image height must be aligned with 32 and the image
+ width must be aligned with 16.
.. raw:: latex

\normalsize
diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index e9e7e70fa24e..85aa3319cbc6 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -269,6 +269,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
.block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
{ .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
.block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
+ { .format = V4L2_PIX_FMT_MS21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2,
+ .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},

/* YUV planar formats */
{ .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 33076af4dfdb..26ffe4d1c28f 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1511,6 +1511,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_MT2110T: descr = "Mediatek 10bit Tile Mode"; break;
case V4L2_PIX_FMT_MT2110R: descr = "Mediatek 10bit Raster Mode"; break;
case V4L2_PIX_FMT_HEXTILE: descr = "Hextile Compressed Format"; break;
+ case V4L2_PIX_FMT_MS21: descr = "Mediatek One Plane Format"; break;
default:
if (fmt->description[0])
return;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 570b11552818..6be0d0e35077 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -798,6 +798,7 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */
#define V4L2_PIX_FMT_MT2110T v4l2_fourcc('M', 'T', '2', 'T') /* Mediatek 10-bit block tile mode */
#define V4L2_PIX_FMT_MT2110R v4l2_fourcc('M', 'T', '2', 'R') /* Mediatek 10-bit block raster mode */
+#define V4L2_PIX_FMT_MS21 v4l2_fourcc('M', 'S', '2', '1') /* Mediatek 8-bit block mode with one plane */
#define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */
#define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */
#define V4L2_PIX_FMT_HI240 v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */
--
2.18.0

2023-12-06 08:17:03

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,09/21] media: mediatek: vcodec: allocate tee share memory

Allocate two share memory for each lat and core hardware used to share
information with optee-os. Msg buffer used to send ipi command and get ack
command with optee-os, data buffer used to store vsi information which
used for hardware decode.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/mtk_vcodec_dec_optee.c | 80 ++++++++++++++++++-
.../vcodec/decoder/mtk_vcodec_dec_optee.h | 32 ++++++++
2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
index 38d9c1c1785a..611fb0e56480 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
@@ -47,13 +47,69 @@ int mtk_vcodec_dec_optee_private_init(struct mtk_vcodec_dec_dev *vcodec_dev)
}
EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_private_init);

+static void mtk_vcodec_dec_optee_deinit_memref(struct mtk_vdec_optee_ca_info *ca_info,
+ enum mtk_vdec_optee_data_index data_index)
+{
+ tee_shm_free(ca_info->shm_memref[data_index].msg_shm);
+}
+
+static int mtk_vcodec_dec_optee_init_memref(struct tee_context *tee_vdec_ctx,
+ struct mtk_vdec_optee_ca_info *ca_info,
+ enum mtk_vdec_optee_data_index data_index)
+{
+ struct mtk_vdec_optee_shm_memref *shm_memref;
+ int alloc_size = 0, err = 0;
+ u64 shm_param_type = 0;
+ bool copy_buffer;
+
+ switch (data_index) {
+ case OPTEE_MSG_INDEX:
+ shm_param_type = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ alloc_size = MTK_VDEC_OPTEE_MSG_SIZE;
+ copy_buffer = true;
+ break;
+ case OPTEE_DATA_INDEX:
+ shm_param_type = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ alloc_size = MTK_VDEC_OPTEE_HW_SIZE;
+ copy_buffer = false;
+ break;
+ default:
+ pr_err(MTK_DBG_VCODEC_STR "tee invalid data_index: %d.\n", data_index);
+ return -EINVAL;
+ }
+
+ shm_memref = &ca_info->shm_memref[data_index];
+
+ /* Allocate dynamic shared memory with decoder TA */
+ shm_memref->msg_shm_size = alloc_size;
+ shm_memref->param_type = shm_param_type;
+ shm_memref->copy_to_ta = copy_buffer;
+ shm_memref->msg_shm = tee_shm_alloc_kernel_buf(tee_vdec_ctx, shm_memref->msg_shm_size);
+ if (IS_ERR(shm_memref->msg_shm)) {
+ pr_err(MTK_DBG_VCODEC_STR "tee alloc buf fail: data_index:%d.\n", data_index);
+ return -ENOMEM;
+ }
+
+ shm_memref->msg_shm_ca_buf = tee_shm_get_va(shm_memref->msg_shm, 0);
+ if (IS_ERR(shm_memref->msg_shm_ca_buf)) {
+ pr_err(MTK_DBG_VCODEC_STR "tee get shm va fail: data_index:%d.\n", data_index);
+ err = PTR_ERR(shm_memref->msg_shm_ca_buf);
+ goto err_get_msg_va;
+ }
+
+ return err;
+err_get_msg_va:
+ tee_shm_free(shm_memref->msg_shm);
+ return err;
+}
+
static int mtk_vcodec_dec_optee_init_hw_info(struct mtk_vdec_optee_private *optee_private,
enum mtk_vdec_hw_id hardware_index)
{
struct device *dev = &optee_private->vcodec_dev->plat_dev->dev;
struct tee_ioctl_open_session_arg session_arg;
struct mtk_vdec_optee_ca_info *ca_info;
- int err = 0, session_func;
+ int err, i, j, session_func;

/* Open lat and core session with vdec TA. */
switch (hardware_index) {
@@ -87,6 +143,24 @@ static int mtk_vcodec_dec_optee_init_hw_info(struct mtk_vdec_optee_private *opte
dev_dbg(dev, MTK_DBG_VCODEC_STR "open vdec tee session hw_id:%d session_id=%x.\n",
hardware_index, ca_info->vdec_session_id);

+ /* Allocate dynamic shared memory with decoder TA */
+ for (i = 0; i < OPTEE_MAX_INDEX; i++) {
+ err = mtk_vcodec_dec_optee_init_memref(optee_private->tee_vdec_ctx, ca_info, i);
+ if (err) {
+ dev_err(dev, MTK_DBG_VCODEC_STR "init vdec memref failed: %d.\n", i);
+ goto err_init_memref;
+ }
+ }
+
+ return err;
+err_init_memref:
+ if (i != 0) {
+ for (j = 0; j < i; j++)
+ mtk_vcodec_dec_optee_deinit_memref(ca_info, j);
+ }
+
+ tee_client_close_session(optee_private->tee_vdec_ctx, ca_info->vdec_session_id);
+
return err;
}

@@ -94,12 +168,16 @@ static void mtk_vcodec_dec_optee_deinit_hw_info(struct mtk_vdec_optee_private *o
enum mtk_vdec_hw_id hw_id)
{
struct mtk_vdec_optee_ca_info *ca_info;
+ int i;

if (hw_id == MTK_VDEC_LAT0)
ca_info = &optee_private->lat_ca;
else
ca_info = &optee_private->core_ca;

+ for (i = 0; i < OPTEE_MAX_INDEX; i++)
+ mtk_vcodec_dec_optee_deinit_memref(ca_info, i);
+
tee_client_close_session(optee_private->tee_vdec_ctx, ca_info->vdec_session_id);
}

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
index 8b1dca49331e..24aa63af9887 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
@@ -18,16 +18,48 @@

#define MTK_OPTEE_MAX_TEE_PARAMS 4

+#define MTK_VDEC_OPTEE_MSG_SIZE 128
+#define MTK_VDEC_OPTEE_HW_SIZE (8 * SZ_1K)
+
+/**
+ * struct mtk_vdec_optee_shm_memref - share memory reference params
+ * @msg_shm: message shared with TA in TEE.
+ * @msg_shm_ca_buf: ca buffer.
+ *
+ * @msg_shm_size: share message size.
+ * @param_type: each tee param types.
+ * @copy_to_ta: need to copy data from ca to share memory.
+ */
+struct mtk_vdec_optee_shm_memref {
+ struct tee_shm *msg_shm;
+ u8 *msg_shm_ca_buf;
+
+ u32 msg_shm_size;
+ u64 param_type;
+ bool copy_to_ta;
+};
+
/**
* struct mtk_vdec_optee_ca_info - ca related param
* @vdec_session_id: optee TA session identifier.
* @hw_id: hardware index.
* @vdec_session_func: trusted application function id used specific to the TA.
+ * @shm_memref: share memory reference params.
*/
struct mtk_vdec_optee_ca_info {
u32 vdec_session_id;
enum mtk_vdec_hw_id hw_id;
u32 vdec_session_func;
+ struct mtk_vdec_optee_shm_memref shm_memref[MTK_OPTEE_MAX_TEE_PARAMS];
+};
+
+/*
+ * enum mtk_vdec_optee_data_index - used to indentify each share memory information
+ */
+enum mtk_vdec_optee_data_index {
+ OPTEE_MSG_INDEX = 0,
+ OPTEE_DATA_INDEX,
+ OPTEE_MAX_INDEX,
};

/**
--
2.18.0

2023-12-06 08:17:04

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,15/21] media: mediatek: vcodec: Add one plane format

Adding capture formats to support V4L2_PIX_FMT_MS21. This format has
one plane and only be used for secure video playback at current period.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c | 4 +++-
.../mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c | 9 ++++++++-
2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index ba742f0e391d..604fdc8ee3ce 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -49,7 +49,9 @@ static bool mtk_vdec_get_cap_fmt(struct mtk_vcodec_dec_ctx *ctx, int format_inde
num_frame_count++;
}

- if (num_frame_count == 1 || (!ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MM21))
+ if ((!ctx->is_10bit_bitstream && fmt->fourcc == V4L2_PIX_FMT_MM21) ||
+ (ctx->is_secure_playback && fmt->fourcc == V4L2_PIX_FMT_MS21) ||
+ num_frame_count == 1)
return true;

q_data = &ctx->q_data[MTK_Q_DATA_SRC];
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index d54b3833790d..cc42c942eb8a 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -229,7 +229,7 @@ static const struct mtk_stateless_control mtk_stateless_controls[] = {

#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)

-static struct mtk_video_fmt mtk_video_formats[9];
+static struct mtk_video_fmt mtk_video_formats[10];

static struct mtk_video_fmt default_out_format;
static struct mtk_video_fmt default_cap_format;
@@ -770,6 +770,11 @@ static void mtk_vcodec_add_formats(unsigned int fourcc,
mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
mtk_video_formats[count_formats].num_planes = 2;
break;
+ case V4L2_PIX_FMT_MS21:
+ mtk_video_formats[count_formats].fourcc = fourcc;
+ mtk_video_formats[count_formats].type = MTK_FMT_FRAME;
+ mtk_video_formats[count_formats].num_planes = 1;
+ break;
default:
mtk_v4l2_vdec_err(ctx, "Can not add unsupported format type");
return;
@@ -798,6 +803,8 @@ static void mtk_vcodec_get_supported_formats(struct mtk_vcodec_dec_ctx *ctx)
cap_format_count++;
}
if (ctx->dev->dec_capability & MTK_VDEC_FORMAT_MM21) {
+ mtk_vcodec_add_formats(V4L2_PIX_FMT_MS21, ctx);
+ cap_format_count++;
mtk_vcodec_add_formats(V4L2_PIX_FMT_MM21, ctx);
cap_format_count++;
}
--
2.18.0

2023-12-06 08:17:15

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,05/21] dma-buf: heaps: Deduplicate docs and adopt common format

From: "T.J. Mercier" <[email protected]>

The docs for dma_heap_get_name were incorrect, and since they were
duplicated in the header they were wrong there too.

The docs formatting was inconsistent so I tried to make it more
consistent across functions since I'm already in here doing cleanup.

Remove multiple unused includes and alphabetize.

Signed-off-by: T.J. Mercier <[email protected]>
Signed-off-by: Yong Wu <[email protected]>
[Yong: Just add a comment for "priv" to mute build warning]
Signed-off-by: Yunfei Dong <[email protected]>
---
drivers/dma-buf/dma-heap.c | 27 +++++++++++++++------------
include/linux/dma-heap.h | 21 +--------------------
2 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 84ae708fafe7..22f6c193db0d 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -7,17 +7,15 @@
*/

#include <linux/cdev.h>
-#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/dma-buf.h>
+#include <linux/dma-heap.h>
#include <linux/err.h>
-#include <linux/xarray.h>
#include <linux/list.h>
-#include <linux/slab.h>
#include <linux/nospec.h>
-#include <linux/uaccess.h>
#include <linux/syscalls.h>
-#include <linux/dma-heap.h>
+#include <linux/uaccess.h>
+#include <linux/xarray.h>
#include <uapi/linux/dma-heap.h>

#define DEVNAME "dma_heap"
@@ -28,9 +26,10 @@
* struct dma_heap - represents a dmabuf heap in the system
* @name: used for debugging/device-node name
* @ops: ops struct for this heap
- * @heap_devt heap device node
- * @list list head connecting to list of heaps
- * @heap_cdev heap char device
+ * @priv: private data for this heap
+ * @heap_devt: heap device node
+ * @list: list head connecting to list of heaps
+ * @heap_cdev: heap char device
*
* Represents a heap of memory from which buffers can be made.
*/
@@ -193,11 +192,11 @@ static const struct file_operations dma_heap_fops = {
};

/**
- * dma_heap_get_drvdata() - get per-subdriver data for the heap
+ * dma_heap_get_drvdata - get per-heap driver data
* @heap: DMA-Heap to retrieve private data for
*
* Returns:
- * The per-subdriver data for the heap.
+ * The per-heap data for the heap.
*/
void *dma_heap_get_drvdata(struct dma_heap *heap)
{
@@ -205,8 +204,8 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
}

/**
- * dma_heap_get_name() - get heap name
- * @heap: DMA-Heap to retrieve private data for
+ * dma_heap_get_name - get heap name
+ * @heap: DMA-Heap to retrieve the name of
*
* Returns:
* The char* for the heap name.
@@ -216,6 +215,10 @@ const char *dma_heap_get_name(struct dma_heap *heap)
return heap->name;
}

+/**
+ * dma_heap_add - adds a heap to dmabuf heaps
+ * @exp_info: information needed to register this heap
+ */
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
{
struct dma_heap *heap, *h, *err_ret;
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 0c05561cad6e..fbe86ec889a8 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -9,14 +9,13 @@
#ifndef _DMA_HEAPS_H
#define _DMA_HEAPS_H

-#include <linux/cdev.h>
#include <linux/types.h>

struct dma_heap;

/**
* struct dma_heap_ops - ops to operate on a given heap
- * @allocate: allocate dmabuf and return struct dma_buf ptr
+ * @allocate: allocate dmabuf and return struct dma_buf ptr
*
* allocate returns dmabuf on success, ERR_PTR(-errno) on error.
*/
@@ -41,28 +40,10 @@ struct dma_heap_export_info {
void *priv;
};

-/**
- * dma_heap_get_drvdata() - get per-heap driver data
- * @heap: DMA-Heap to retrieve private data for
- *
- * Returns:
- * The per-heap data for the heap.
- */
void *dma_heap_get_drvdata(struct dma_heap *heap);

-/**
- * dma_heap_get_name() - get heap name
- * @heap: DMA-Heap to retrieve private data for
- *
- * Returns:
- * The char* for the heap name.
- */
const char *dma_heap_get_name(struct dma_heap *heap);

-/**
- * dma_heap_add - adds a heap to dmabuf heaps
- * @exp_info: information needed to register this heap
- */
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);

#endif /* _DMA_HEAPS_H */
--
2.18.0

2023-12-06 08:17:16

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,16/21] media: medkatek: vcodec: support one plane capture buffer

The capture buffer has two planes for format MM21, but user space only
allocate secure memory for plane[0], and the size is Y data + uv data.
The driver need to support one plane decoder for svp mode.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 7 ++++-
.../vcodec/decoder/mtk_vcodec_dec_stateless.c | 26 ++++++++++---------
.../decoder/vdec/vdec_h264_req_common.c | 11 +++-----
.../mediatek/vcodec/decoder/vdec_drv_if.c | 4 +--
4 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index 604fdc8ee3ce..ab922e8d2d37 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -653,7 +653,12 @@ static int vidioc_vdec_g_fmt(struct file *file, void *priv,
* So we just return picinfo yet, and update picinfo in
* stop_streaming hook function
*/
- q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
+
+ if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1)
+ q_data->sizeimage[0] = ctx->picinfo.fb_sz[0] + ctx->picinfo.fb_sz[1];
+ else
+ q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
+
q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
q_data->bytesperline[0] = ctx->last_decoded_picinfo.buf_w;
q_data->bytesperline[1] = ctx->last_decoded_picinfo.buf_w;
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
index cc42c942eb8a..707ed57a412e 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
@@ -285,14 +285,14 @@ static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_dec_ctx *ctx)
framebuf = container_of(vb2_v4l2, struct mtk_video_dec_buf, m2m_buf.vb);

pfb = &framebuf->frame_buffer;
- pfb->base_y.va = vb2_plane_vaddr(dst_buf, 0);
+ if (!ctx->is_secure_playback)
+ pfb->base_y.va = vb2_plane_vaddr(dst_buf, 0);
pfb->base_y.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
pfb->base_y.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0];

- if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) {
+ if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2 && !ctx->is_secure_playback) {
pfb->base_c.va = vb2_plane_vaddr(dst_buf, 1);
- pfb->base_c.dma_addr =
- vb2_dma_contig_plane_dma_addr(dst_buf, 1);
+ pfb->base_c.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 1);
pfb->base_c.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[1];
}
mtk_v4l2_vdec_dbg(1, ctx,
@@ -339,16 +339,18 @@ static void mtk_vdec_worker(struct work_struct *work)
mtk_v4l2_vdec_dbg(3, ctx, "[%d] (%d) id=%d, vb=%p", ctx->id,
vb2_src->vb2_queue->type, vb2_src->index, vb2_src);

- bs_src->va = vb2_plane_vaddr(vb2_src, 0);
- bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0);
- bs_src->size = (size_t)vb2_src->planes[0].bytesused;
- if (!bs_src->va) {
- v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
- mtk_v4l2_vdec_err(ctx, "[%d] id=%d source buffer is NULL", ctx->id,
- vb2_src->index);
- return;
+ if (!ctx->is_secure_playback) {
+ bs_src->va = vb2_plane_vaddr(vb2_src, 0);
+ if (!bs_src->va) {
+ v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
+ mtk_v4l2_vdec_err(ctx, "[%d] id=%d source buffer is NULL", ctx->id,
+ vb2_src->index);
+ return;
+ }
}

+ bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0);
+ bs_src->size = (size_t)vb2_src->planes[0].bytesused;
mtk_v4l2_vdec_dbg(3, ctx, "[%d] Bitstream VA=%p DMA=%pad Size=%zx vb=%p",
ctx->id, bs_src->va, &bs_src->dma_addr, bs_src->size, vb2_src);
/* Apply request controls. */
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
index 5ca20d75dc8e..2a57e689ec07 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
@@ -79,15 +79,12 @@ void mtk_vdec_h264_fill_dpb_info(struct mtk_vcodec_dec_ctx *ctx,
vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
h264_dpb_info[index].field = vb2_v4l2->field;

- h264_dpb_info[index].y_dma_addr =
- vb2_dma_contig_plane_dma_addr(vb, 0);
+ h264_dpb_info[index].y_dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2)
+ h264_dpb_info[index].c_dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
+ else if (!ctx->is_secure_playback)
h264_dpb_info[index].c_dma_addr =
- vb2_dma_contig_plane_dma_addr(vb, 1);
- else
- h264_dpb_info[index].c_dma_addr =
- h264_dpb_info[index].y_dma_addr +
- ctx->picinfo.fb_sz[0];
+ h264_dpb_info[index].y_dma_addr + ctx->picinfo.fb_sz[0];
}
}

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
index d0b459b1603f..fb3e4f75ed93 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
@@ -73,14 +73,14 @@ int vdec_if_decode(struct mtk_vcodec_dec_ctx *ctx, struct mtk_vcodec_mem *bs,
{
int ret = 0;

- if (bs) {
+ if (bs && !ctx->is_secure_playback) {
if ((bs->dma_addr & 63) != 0) {
mtk_v4l2_vdec_err(ctx, "bs dma_addr should 64 byte align");
return -EINVAL;
}
}

- if (fb) {
+ if (fb && !ctx->is_secure_playback) {
if (((fb->base_y.dma_addr & 511) != 0) ||
((fb->base_c.dma_addr & 511) != 0)) {
mtk_v4l2_vdec_err(ctx, "frame buffer dma_addr should 512 byte align");
--
2.18.0

2023-12-06 08:17:25

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,17/21] media: medkatek: vcodec: re-construct h264 driver to support svp mode

Need secure buffer size to convert secure handle to secure
pa in optee-os, re-construct the vsi struct to store each
secure buffer size.

Separate svp and normal wait interrupt condition for svp mode
waiting hardware interrupt in optee-os.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../decoder/vdec/vdec_h264_req_multi_if.c | 261 +++++++++++-------
.../mediatek/vcodec/decoder/vdec_msg_queue.c | 9 +-
2 files changed, 168 insertions(+), 102 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index 4967e0f0984d..a1a68487131c 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -60,14 +60,36 @@ struct vdec_h264_slice_lat_dec_param {
* @crc: Used to check whether hardware's status is right
*/
struct vdec_h264_slice_info {
+ u64 wdma_end_addr_offset;
u16 nal_info;
u16 timeout;
- u32 bs_buf_size;
- u64 bs_buf_addr;
- u64 y_fb_dma;
- u64 c_fb_dma;
u64 vdec_fb_va;
u32 crc[8];
+ u32 reserved;
+};
+
+/*
+ * struct vdec_h264_slice_mem - memory address and size
+ */
+struct vdec_h264_slice_mem {
+ union {
+ u64 buf;
+ u64 dma_addr;
+ };
+ union {
+ size_t size;
+ u64 dma_addr_end;
+ };
+};
+
+/**
+ * struct vdec_h264_slice_fb - frame buffer for decoding
+ * @y: current y buffer address info
+ * @c: current c buffer address info
+ */
+struct vdec_h264_slice_fb {
+ struct vdec_h264_slice_mem y;
+ struct vdec_h264_slice_mem c;
};

/**
@@ -92,18 +114,16 @@ struct vdec_h264_slice_info {
*/
struct vdec_h264_slice_vsi {
/* LAT dec addr */
- u64 wdma_err_addr;
- u64 wdma_start_addr;
- u64 wdma_end_addr;
- u64 slice_bc_start_addr;
- u64 slice_bc_end_addr;
- u64 row_info_start_addr;
- u64 row_info_end_addr;
- u64 trans_start;
- u64 trans_end;
- u64 wdma_end_addr_offset;
+ struct vdec_h264_slice_mem bs;
+ struct vdec_h264_slice_fb fb;

- u64 mv_buf_dma[H264_MAX_MV_NUM];
+ struct vdec_h264_slice_mem ube;
+ struct vdec_h264_slice_mem trans;
+ struct vdec_h264_slice_mem row_info;
+ struct vdec_h264_slice_mem err_map;
+ struct vdec_h264_slice_mem slice_bc;
+
+ struct vdec_h264_slice_mem mv_buf_dma[H264_MAX_MV_NUM];
struct vdec_h264_slice_info dec;
struct vdec_h264_slice_lat_dec_param h264_slice_params;
};
@@ -392,6 +412,100 @@ static void vdec_h264_slice_get_crop_info(struct vdec_h264_slice_inst *inst,
cr->left, cr->top, cr->width, cr->height);
}

+static void vdec_h264_slice_setup_lat_buffer(struct vdec_h264_slice_inst *inst,
+ struct mtk_vcodec_mem *bs,
+ struct vdec_lat_buf *lat_buf)
+{
+ struct mtk_vcodec_mem *mem;
+ int i;
+
+ inst->vsi->bs.dma_addr = (u64)bs->dma_addr;
+ inst->vsi->bs.size = bs->size;
+
+ for (i = 0; i < H264_MAX_MV_NUM; i++) {
+ mem = &inst->mv_buf[i];
+ inst->vsi->mv_buf_dma[i].dma_addr = mem->dma_addr;
+ inst->vsi->mv_buf_dma[i].size = mem->size;
+ }
+ inst->vsi->ube.dma_addr = lat_buf->ctx->msg_queue.wdma_addr.dma_addr;
+ inst->vsi->ube.size = lat_buf->ctx->msg_queue.wdma_addr.size;
+
+ inst->vsi->row_info.dma_addr = 0;
+ inst->vsi->row_info.size = 0;
+
+ inst->vsi->err_map.dma_addr = lat_buf->wdma_err_addr.dma_addr;
+ inst->vsi->err_map.size = lat_buf->wdma_err_addr.size;
+
+ inst->vsi->slice_bc.dma_addr = lat_buf->slice_bc_addr.dma_addr;
+ inst->vsi->slice_bc.size = lat_buf->slice_bc_addr.size;
+
+ inst->vsi->trans.dma_addr_end = inst->ctx->msg_queue.wdma_rptr_addr;
+ inst->vsi->trans.dma_addr = inst->ctx->msg_queue.wdma_wptr_addr;
+}
+
+static int vdec_h264_slice_setup_core_buffer(struct vdec_h264_slice_inst *inst,
+ struct vdec_h264_slice_share_info *share_info,
+ struct vdec_lat_buf *lat_buf)
+{
+ struct mtk_vcodec_mem *mem;
+ struct mtk_vcodec_dec_ctx *ctx = inst->ctx;
+ struct vb2_v4l2_buffer *vb2_v4l2;
+ struct vdec_fb *fb;
+ u64 y_fb_dma, c_fb_dma = 0;
+ int i;
+
+ fb = ctx->dev->vdec_pdata->get_cap_buffer(ctx);
+ if (!fb) {
+ mtk_vdec_err(ctx, "fb buffer is NULL");
+ return -EBUSY;
+ }
+
+ y_fb_dma = (u64)fb->base_y.dma_addr;
+ if (!ctx->is_secure_playback) {
+ if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1)
+ c_fb_dma =
+ y_fb_dma + inst->ctx->picinfo.buf_w * inst->ctx->picinfo.buf_h;
+ else
+ c_fb_dma = (u64)fb->base_c.dma_addr;
+ }
+
+ mtk_vdec_debug(ctx, "[h264-core] y/c addr = 0x%llx 0x%llx", y_fb_dma, c_fb_dma);
+
+ inst->vsi_core->fb.y.dma_addr = y_fb_dma;
+ inst->vsi_core->fb.y.size = ctx->picinfo.fb_sz[0];
+ inst->vsi_core->fb.c.dma_addr = c_fb_dma;
+ inst->vsi_core->fb.c.size = ctx->picinfo.fb_sz[1];
+
+ inst->vsi_core->dec.vdec_fb_va = (unsigned long)fb;
+ inst->vsi_core->dec.nal_info = share_info->nal_info;
+
+ inst->vsi_core->ube.dma_addr = lat_buf->ctx->msg_queue.wdma_addr.dma_addr;
+ inst->vsi_core->ube.size = lat_buf->ctx->msg_queue.wdma_addr.size;
+
+ inst->vsi_core->err_map.dma_addr = lat_buf->wdma_err_addr.dma_addr;
+ inst->vsi_core->err_map.size = lat_buf->wdma_err_addr.size;
+
+ inst->vsi_core->slice_bc.dma_addr = lat_buf->slice_bc_addr.dma_addr;
+ inst->vsi_core->slice_bc.size = lat_buf->slice_bc_addr.size;
+
+ inst->vsi_core->row_info.dma_addr = 0;
+ inst->vsi_core->row_info.size = 0;
+
+ inst->vsi_core->trans.dma_addr = share_info->trans_start;
+ inst->vsi_core->trans.dma_addr_end = share_info->trans_end;
+
+ for (i = 0; i < H264_MAX_MV_NUM; i++) {
+ mem = &inst->mv_buf[i];
+ inst->vsi_core->mv_buf_dma[i].dma_addr = mem->dma_addr;
+ inst->vsi_core->mv_buf_dma[i].size = mem->size;
+ }
+
+ vb2_v4l2 = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
+ v4l2_m2m_buf_copy_metadata(&lat_buf->ts_info, vb2_v4l2, true);
+
+ return 0;
+}
+
static int vdec_h264_slice_init(struct mtk_vcodec_dec_ctx *ctx)
{
struct vdec_h264_slice_inst *inst;
@@ -457,64 +571,22 @@ static void vdec_h264_slice_deinit(void *h_vdec)

static int vdec_h264_slice_core_decode(struct vdec_lat_buf *lat_buf)
{
- struct vdec_fb *fb;
- u64 vdec_fb_va;
- u64 y_fb_dma, c_fb_dma;
- int err, timeout, i;
+ int err, timeout;
struct mtk_vcodec_dec_ctx *ctx = lat_buf->ctx;
struct vdec_h264_slice_inst *inst = ctx->drv_handle;
- struct vb2_v4l2_buffer *vb2_v4l2;
struct vdec_h264_slice_share_info *share_info = lat_buf->private_data;
- struct mtk_vcodec_mem *mem;
struct vdec_vpu_inst *vpu = &inst->vpu;

mtk_vdec_debug(ctx, "[h264-core] vdec_h264 core decode");
memcpy(&inst->vsi_core->h264_slice_params, &share_info->h264_slice_params,
sizeof(share_info->h264_slice_params));

- fb = ctx->dev->vdec_pdata->get_cap_buffer(ctx);
- if (!fb) {
- err = -EBUSY;
- mtk_vdec_err(ctx, "fb buffer is NULL");
+ err = vdec_h264_slice_setup_core_buffer(inst, share_info, lat_buf);
+ if (err)
goto vdec_dec_end;
- }
-
- vdec_fb_va = (unsigned long)fb;
- y_fb_dma = (u64)fb->base_y.dma_addr;
- if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1)
- c_fb_dma =
- y_fb_dma + inst->ctx->picinfo.buf_w * inst->ctx->picinfo.buf_h;
- else
- c_fb_dma = (u64)fb->base_c.dma_addr;
-
- mtk_vdec_debug(ctx, "[h264-core] y/c addr = 0x%llx 0x%llx", y_fb_dma, c_fb_dma);
-
- inst->vsi_core->dec.y_fb_dma = y_fb_dma;
- inst->vsi_core->dec.c_fb_dma = c_fb_dma;
- inst->vsi_core->dec.vdec_fb_va = vdec_fb_va;
- inst->vsi_core->dec.nal_info = share_info->nal_info;
- inst->vsi_core->wdma_start_addr =
- lat_buf->ctx->msg_queue.wdma_addr.dma_addr;
- inst->vsi_core->wdma_end_addr =
- lat_buf->ctx->msg_queue.wdma_addr.dma_addr +
- lat_buf->ctx->msg_queue.wdma_addr.size;
- inst->vsi_core->wdma_err_addr = lat_buf->wdma_err_addr.dma_addr;
- inst->vsi_core->slice_bc_start_addr = lat_buf->slice_bc_addr.dma_addr;
- inst->vsi_core->slice_bc_end_addr = lat_buf->slice_bc_addr.dma_addr +
- lat_buf->slice_bc_addr.size;
- inst->vsi_core->trans_start = share_info->trans_start;
- inst->vsi_core->trans_end = share_info->trans_end;
- for (i = 0; i < H264_MAX_MV_NUM; i++) {
- mem = &inst->mv_buf[i];
- inst->vsi_core->mv_buf_dma[i] = mem->dma_addr;
- }
-
- vb2_v4l2 = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
- v4l2_m2m_buf_copy_metadata(&lat_buf->ts_info, vb2_v4l2, true);

vdec_h264_slice_fill_decode_reflist(inst, &inst->vsi_core->h264_slice_params,
share_info);
-
err = vpu_dec_core(vpu);
if (err) {
mtk_vdec_err(ctx, "core decode err=%d", err);
@@ -573,12 +645,11 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
struct vdec_h264_slice_inst *inst = h_vdec;
struct vdec_vpu_inst *vpu = &inst->vpu;
struct mtk_video_dec_buf *src_buf_info;
- int nal_start_idx, err, timeout = 0, i;
+ int nal_start_idx, err, timeout = 0;
unsigned int data[2];
struct vdec_lat_buf *lat_buf;
struct vdec_h264_slice_share_info *share_info;
unsigned char *buf;
- struct mtk_vcodec_mem *mem;

if (vdec_msg_queue_init(&inst->ctx->msg_queue, inst->ctx,
vdec_h264_slice_core_decode,
@@ -617,11 +688,9 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
if (err)
goto err_free_fb_out;

- vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
- &share_info->h264_slice_params.pps);
-
- inst->vsi->dec.bs_buf_addr = (uint64_t)bs->dma_addr;
- inst->vsi->dec.bs_buf_size = bs->size;
+ if (!inst->ctx->is_secure_playback)
+ vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
+ &share_info->h264_slice_params.pps);

*res_chg = inst->resolution_changed;
if (inst->resolution_changed) {
@@ -634,38 +703,27 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
}
inst->resolution_changed = false;
}
- for (i = 0; i < H264_MAX_MV_NUM; i++) {
- mem = &inst->mv_buf[i];
- inst->vsi->mv_buf_dma[i] = mem->dma_addr;
- }
- inst->vsi->wdma_start_addr = lat_buf->ctx->msg_queue.wdma_addr.dma_addr;
- inst->vsi->wdma_end_addr = lat_buf->ctx->msg_queue.wdma_addr.dma_addr +
- lat_buf->ctx->msg_queue.wdma_addr.size;
- inst->vsi->wdma_err_addr = lat_buf->wdma_err_addr.dma_addr;
- inst->vsi->slice_bc_start_addr = lat_buf->slice_bc_addr.dma_addr;
- inst->vsi->slice_bc_end_addr = lat_buf->slice_bc_addr.dma_addr +
- lat_buf->slice_bc_addr.size;
-
- inst->vsi->trans_end = inst->ctx->msg_queue.wdma_rptr_addr;
- inst->vsi->trans_start = inst->ctx->msg_queue.wdma_wptr_addr;
- mtk_vdec_debug(inst->ctx, "lat:trans(0x%llx 0x%llx) err:0x%llx",
- inst->vsi->wdma_start_addr,
- inst->vsi->wdma_end_addr,
- inst->vsi->wdma_err_addr);
-
- mtk_vdec_debug(inst->ctx, "slice(0x%llx 0x%llx) rprt((0x%llx 0x%llx))",
- inst->vsi->slice_bc_start_addr,
- inst->vsi->slice_bc_end_addr,
- inst->vsi->trans_start,
- inst->vsi->trans_end);
+
+ vdec_h264_slice_setup_lat_buffer(inst, bs, lat_buf);
+ mtk_vdec_debug(inst->ctx, "lat:trans(0x%llx 0x%lx) err:0x%llx",
+ inst->vsi->ube.dma_addr, (unsigned long)inst->vsi->ube.size,
+ inst->vsi->err_map.dma_addr);
+
+ mtk_vdec_debug(inst->ctx, "slice(0x%llx 0x%lx) rprt((0x%llx 0x%llx))",
+ inst->vsi->slice_bc.dma_addr, (unsigned long)inst->vsi->slice_bc.size,
+ inst->vsi->trans.dma_addr, inst->vsi->trans.dma_addr_end);
err = vpu_dec_start(vpu, data, 2);
if (err) {
mtk_vdec_debug(inst->ctx, "lat decode err: %d", err);
goto err_free_fb_out;
}

- share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr +
- inst->vsi->wdma_end_addr_offset;
+ if (inst->ctx->is_secure_playback)
+ share_info->trans_end = inst->vsi->dec.wdma_end_addr_offset;
+ else
+ share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr +
+ inst->vsi->dec.wdma_end_addr_offset;
+
share_info->trans_start = inst->ctx->msg_queue.wdma_wptr_addr;
share_info->nal_info = inst->vsi->dec.nal_info;

@@ -691,8 +749,11 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
return -EINVAL;
}

- share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr +
- inst->vsi->wdma_end_addr_offset;
+ if (inst->ctx->is_secure_playback)
+ share_info->trans_end = inst->vsi->dec.wdma_end_addr_offset;
+ else
+ share_info->trans_end = inst->ctx->msg_queue.wdma_addr.dma_addr +
+ inst->vsi->dec.wdma_end_addr_offset;
vdec_msg_queue_update_ube_wptr(&lat_buf->ctx->msg_queue, share_info->trans_end);

if (!IS_VDEC_INNER_RACING(inst->ctx->dev->dec_capability)) {
@@ -737,10 +798,10 @@ static int vdec_h264_slice_single_decode(void *h_vdec, struct mtk_vcodec_mem *bs
mtk_vdec_debug(inst->ctx, "[h264-dec] [%d] y_dma=%llx c_dma=%llx",
inst->ctx->decoded_frame_cnt, y_fb_dma, c_fb_dma);

- inst->vsi_ctx.dec.bs_buf_addr = (u64)bs->dma_addr;
- inst->vsi_ctx.dec.bs_buf_size = bs->size;
- inst->vsi_ctx.dec.y_fb_dma = y_fb_dma;
- inst->vsi_ctx.dec.c_fb_dma = c_fb_dma;
+ inst->vsi_ctx.bs.dma_addr = (u64)bs->dma_addr;
+ inst->vsi_ctx.bs.size = bs->size;
+ inst->vsi_ctx.fb.y.dma_addr = y_fb_dma;
+ inst->vsi_ctx.fb.c.dma_addr = c_fb_dma;
inst->vsi_ctx.dec.vdec_fb_va = (u64)(uintptr_t)fb;

v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb,
@@ -770,7 +831,7 @@ static int vdec_h264_slice_single_decode(void *h_vdec, struct mtk_vcodec_mem *bs

for (i = 0; i < H264_MAX_MV_NUM; i++) {
mem = &inst->mv_buf[i];
- inst->vsi_ctx.mv_buf_dma[i] = mem->dma_addr;
+ inst->vsi_ctx.mv_buf_dma[i].dma_addr = mem->dma_addr;
}
}

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_msg_queue.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_msg_queue.c
index f283c4703dc6..c1310176ae05 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_msg_queue.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_msg_queue.c
@@ -308,8 +308,13 @@ int vdec_msg_queue_init(struct vdec_msg_queue *msg_queue,
msg_queue->wdma_addr.size = 0;
return -ENOMEM;
}
- msg_queue->wdma_rptr_addr = msg_queue->wdma_addr.dma_addr;
- msg_queue->wdma_wptr_addr = msg_queue->wdma_addr.dma_addr;
+ if (ctx->is_secure_playback) {
+ msg_queue->wdma_rptr_addr = 0;
+ msg_queue->wdma_wptr_addr = 0;
+ } else {
+ msg_queue->wdma_rptr_addr = msg_queue->wdma_addr.dma_addr;
+ msg_queue->wdma_wptr_addr = msg_queue->wdma_addr.dma_addr;
+ }

msg_queue->empty_lat_buf.ctx = ctx;
msg_queue->empty_lat_buf.core_decode = NULL;
--
2.18.0

2023-12-06 08:18:11

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,10/21] media: mediatek: vcodec: send share memory data to optee

Setting msg and vsi information to shared buffer, then call tee invoke
function to send it to optee-os.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/mtk_vcodec_dec_optee.c | 140 ++++++++++++++++++
.../vcodec/decoder/mtk_vcodec_dec_optee.h | 51 +++++++
2 files changed, 191 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
index 611fb0e56480..f29a8d143fee 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
@@ -241,3 +241,143 @@ void mtk_vcodec_dec_optee_release(struct mtk_vdec_optee_private *optee_private)
mutex_unlock(&optee_private->tee_mutex);
}
EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_release);
+
+static int mtk_vcodec_dec_optee_fill_shm(struct tee_param *command_params,
+ struct mtk_vdec_optee_shm_memref *shm_memref,
+ struct mtk_vdec_optee_data_to_shm *data,
+ int index, struct device *dev)
+{
+ if (!data->msg_buf_size[index] || !data->msg_buf[index]) {
+ pr_err(MTK_DBG_VCODEC_STR "tee invalid buf param: %d.\n", index);
+ return -EINVAL;
+ }
+
+ *command_params = (struct tee_param) {
+ .attr = shm_memref->param_type,
+ .u.memref = {
+ .shm = shm_memref->msg_shm,
+ .size = data->msg_buf_size[index],
+ .shm_offs = 0,
+ },
+ };
+
+ if (!shm_memref->copy_to_ta) {
+ dev_dbg(dev, MTK_DBG_VCODEC_STR "share memref data: 0x%x param_type:%llu.\n",
+ *((unsigned int *)shm_memref->msg_shm_ca_buf), shm_memref->param_type);
+ return 0;
+ }
+
+ memset(shm_memref->msg_shm_ca_buf, 0, shm_memref->msg_shm_size);
+ memcpy(shm_memref->msg_shm_ca_buf, data->msg_buf[index], data->msg_buf_size[index]);
+
+ dev_dbg(dev, MTK_DBG_VCODEC_STR "share memref data => msg id:0x%x 0x%x param_type:%llu.\n",
+ *((unsigned int *)data->msg_buf[index]),
+ *((unsigned int *)shm_memref->msg_shm_ca_buf),
+ shm_memref->param_type);
+
+ return 0;
+}
+
+void mtk_vcodec_dec_optee_set_data(struct mtk_vdec_optee_data_to_shm *data,
+ void *buf, int buf_size,
+ enum mtk_vdec_optee_data_index index)
+{
+ data->msg_buf[index] = buf;
+ data->msg_buf_size[index] = buf_size;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_set_data);
+
+int mtk_vcodec_dec_optee_invokd_cmd(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ struct mtk_vdec_optee_data_to_shm *data)
+{
+ struct device *dev = &optee_private->vcodec_dev->plat_dev->dev;
+ struct tee_ioctl_invoke_arg trans_args;
+ struct tee_param command_params[MTK_OPTEE_MAX_TEE_PARAMS];
+ struct mtk_vdec_optee_ca_info *ca_info;
+ struct mtk_vdec_optee_shm_memref *shm_memref;
+ int ret, index;
+
+ if (hw_id == MTK_VDEC_LAT0)
+ ca_info = &optee_private->lat_ca;
+ else
+ ca_info = &optee_private->core_ca;
+
+ memset(&trans_args, 0, sizeof(trans_args));
+ memset(command_params, 0, sizeof(command_params));
+
+ trans_args = (struct tee_ioctl_invoke_arg) {
+ .func = ca_info->vdec_session_func,
+ .session = ca_info->vdec_session_id,
+ .num_params = MTK_OPTEE_MAX_TEE_PARAMS,
+ };
+
+ /* Fill msg command parameters */
+ for (index = 0; index < MTK_OPTEE_MAX_TEE_PARAMS; index++) {
+ shm_memref = &ca_info->shm_memref[index];
+
+ if (shm_memref->param_type == TEE_IOCTL_PARAM_ATTR_TYPE_NONE ||
+ data->msg_buf_size[index] == 0)
+ continue;
+
+ dev_dbg(dev, MTK_DBG_VCODEC_STR "tee share memory data size: %d -> %d.\n",
+ data->msg_buf_size[index], shm_memref->msg_shm_size);
+
+ if (data->msg_buf_size[index] > shm_memref->msg_shm_size) {
+ dev_err(dev, MTK_DBG_VCODEC_STR "tee buf size big than shm (%d -> %d).\n",
+ data->msg_buf_size[index], shm_memref->msg_shm_size);
+ return -EINVAL;
+ }
+
+ ret = mtk_vcodec_dec_optee_fill_shm(&command_params[index], shm_memref,
+ data, index, dev);
+ if (ret)
+ return ret;
+ }
+
+ ret = tee_client_invoke_func(optee_private->tee_vdec_ctx, &trans_args, command_params);
+ if (ret < 0 || trans_args.ret != 0) {
+ dev_err(dev, MTK_DBG_VCODEC_STR "tee submit command fail: 0x%x 0x%x.\n",
+ trans_args.ret, ret);
+ return (ret < 0) ? ret : trans_args.ret;
+ }
+
+ /* clear all attrs, set all command param to unused */
+ for (index = 0; index < MTK_OPTEE_MAX_TEE_PARAMS; index++) {
+ data->msg_buf[index] = NULL;
+ data->msg_buf_size[index] = 0;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_invokd_cmd);
+
+void *mtk_vcodec_dec_get_shm_buffer_va(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ enum mtk_vdec_optee_data_index data_index)
+{
+ struct mtk_vdec_optee_ca_info *ca_info;
+
+ if (hw_id == MTK_VDEC_LAT0)
+ ca_info = &optee_private->lat_ca;
+ else
+ ca_info = &optee_private->core_ca;
+
+ return ca_info->shm_memref[data_index].msg_shm_ca_buf;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_get_shm_buffer_va);
+
+int mtk_vcodec_dec_get_shm_buffer_size(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ enum mtk_vdec_optee_data_index data_index)
+{
+ struct mtk_vdec_optee_ca_info *ca_info;
+
+ if (hw_id == MTK_VDEC_LAT0)
+ ca_info = &optee_private->lat_ca;
+ else
+ ca_info = &optee_private->core_ca;
+
+ return ca_info->shm_memref[data_index].msg_shm_size;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_get_shm_buffer_size);
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
index 24aa63af9887..c24a567ec877 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
@@ -62,6 +62,16 @@ enum mtk_vdec_optee_data_index {
OPTEE_MAX_INDEX,
};

+/**
+ * struct mtk_vdec_optee_data_to_shm - shm data used for TA
+ * @msg_buf: msg information to TA.
+ * @msg_buf_len: length of msg information.
+ */
+struct mtk_vdec_optee_data_to_shm {
+ void *msg_buf[MTK_OPTEE_MAX_TEE_PARAMS];
+ int msg_buf_size[MTK_OPTEE_MAX_TEE_PARAMS];
+};
+
/**
* struct mtk_vdec_optee_private - optee private data
* @vcodec_dev: pointer to the mtk_vcodec_dev of the device
@@ -102,4 +112,45 @@ int mtk_vcodec_dec_optee_private_init(struct mtk_vcodec_dec_dev *vcodec_dev);
*/
void mtk_vcodec_dec_optee_release(struct mtk_vdec_optee_private *optee_private);

+/**
+ * mtk_vcodec_dec_optee_set_data - set buffer to share memref.
+ * @vcodec_dev: normal world data used to init optee share memory
+ * @buf: normal world buffer address
+ * @buf_size: buf size
+ * @data_index: indentify each share memory informaiton
+ */
+void mtk_vcodec_dec_optee_set_data(struct mtk_vdec_optee_data_to_shm *data,
+ void *buf, int buf_size,
+ enum mtk_vdec_optee_data_index data_index);
+
+/**
+ * mtk_vcodec_dec_optee_invokd_cmd - send share memory data to optee .
+ * @optee_private: optee private context
+ * @hw_id: hardware index
+ * @data: normal world data used to init optee share memory
+ */
+int mtk_vcodec_dec_optee_invokd_cmd(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ struct mtk_vdec_optee_data_to_shm *data);
+
+/**
+ * mtk_vcodec_dec_get_shm_buffer_va - close the communication channels with TA.
+ * @optee_private: optee private context
+ * @hw_id: hardware index
+ * @@data_index: indentify each share memory informaiton
+ */
+void *mtk_vcodec_dec_get_shm_buffer_va(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ enum mtk_vdec_optee_data_index data_index);
+
+/**
+ * mtk_vcodec_dec_get_shm_buffer_size - close the communication channels with TA.
+ * @optee_private: optee private context
+ * @hw_id: hardware index
+ * @@data_index: indentify each share memory informaiton
+ */
+int mtk_vcodec_dec_get_shm_buffer_size(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id,
+ enum mtk_vdec_optee_data_index data_index);
+
#endif /* _MTK_VCODEC_FW_OPTEE_H_ */
--
2.18.0

2023-12-06 08:18:13

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,18/21] media: medkatek: vcodec: remove parse nal_info in kernel

The hardware can parse syntax to get nal_info, needn't to use cpu.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/vdec/vdec_h264_req_multi_if.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index a1a68487131c..2dfb3043493e 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -645,11 +645,10 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
struct vdec_h264_slice_inst *inst = h_vdec;
struct vdec_vpu_inst *vpu = &inst->vpu;
struct mtk_video_dec_buf *src_buf_info;
- int nal_start_idx, err, timeout = 0;
+ int err, timeout = 0;
unsigned int data[2];
struct vdec_lat_buf *lat_buf;
struct vdec_h264_slice_share_info *share_info;
- unsigned char *buf;

if (vdec_msg_queue_init(&inst->ctx->msg_queue, inst->ctx,
vdec_h264_slice_core_decode,
@@ -673,14 +672,6 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
share_info = lat_buf->private_data;
src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);

- buf = (unsigned char *)bs->va;
- nal_start_idx = mtk_vdec_h264_find_start_code(buf, bs->size);
- if (nal_start_idx < 0) {
- err = -EINVAL;
- goto err_free_fb_out;
- }
-
- inst->vsi->dec.nal_info = buf[nal_start_idx];
lat_buf->src_buf_req = src_buf_info->m2m_buf.vb.vb2_buf.req_obj.req;
v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb, &lat_buf->ts_info, true);

@@ -689,7 +680,7 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
goto err_free_fb_out;

if (!inst->ctx->is_secure_playback)
- vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
+ vdec_h264_insert_startcode(inst->ctx->dev, bs->va, &bs->size,
&share_info->h264_slice_params.pps);

*res_chg = inst->resolution_changed;
--
2.18.0

2023-12-06 08:18:14

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,08/21] media: mediatek: vcodec: add tee client interface to communiate with optee-os

Open tee context to initialize the environment in order to communication
with optee-os, then open tee session as the communication pipeline for
lat and core to send data for hardware decode.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../platform/mediatek/vcodec/decoder/Makefile | 1 +
.../vcodec/decoder/mtk_vcodec_dec_drv.h | 5 +
.../vcodec/decoder/mtk_vcodec_dec_optee.c | 165 ++++++++++++++++++
.../vcodec/decoder/mtk_vcodec_dec_optee.h | 73 ++++++++
4 files changed, 244 insertions(+)
create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
create mode 100644 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/Makefile b/drivers/media/platform/mediatek/vcodec/decoder/Makefile
index 904cd22def84..1624933dfd5e 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/Makefile
+++ b/drivers/media/platform/mediatek/vcodec/decoder/Makefile
@@ -21,5 +21,6 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
mtk_vcodec_dec_stateful.o \
mtk_vcodec_dec_stateless.o \
mtk_vcodec_dec_pm.o \
+ mtk_vcodec_dec_optee.o \

mtk-vcodec-dec-hw-y := mtk_vcodec_dec_hw.o
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index 849b89dd205c..b1a2107f2a1e 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -11,6 +11,7 @@
#include "../common/mtk_vcodec_dbgfs.h"
#include "../common/mtk_vcodec_fw_priv.h"
#include "../common/mtk_vcodec_util.h"
+#include "mtk_vcodec_dec_optee.h"
#include "vdec_msg_queue.h"

#define MTK_VCODEC_DEC_NAME "mtk-vcodec-dec"
@@ -261,6 +262,8 @@ struct mtk_vcodec_dec_ctx {
* @dbgfs: debug log related information
*
* @chip_name: used to distinguish platforms and select the correct codec configuration values
+ *
+ * @optee_private: optee private data
*/
struct mtk_vcodec_dec_dev {
struct v4l2_device v4l2_dev;
@@ -303,6 +306,8 @@ struct mtk_vcodec_dec_dev {
struct mtk_vcodec_dbgfs dbgfs;

enum mtk_vcodec_dec_chip_name chip_name;
+
+ struct mtk_vdec_optee_private *optee_private;
};

static inline struct mtk_vcodec_dec_ctx *fh_to_dec_ctx(struct v4l2_fh *fh)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
new file mode 100644
index 000000000000..38d9c1c1785a
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Yunfei Dong <[email protected]>
+ */
+
+#include "mtk_vcodec_dec_drv.h"
+#include "mtk_vcodec_dec_optee.h"
+
+/*
+ * Randomly generated, and must correspond to the GUID on the TA side.
+ */
+static const uuid_t mtk_vdec_lat_uuid =
+ UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
+ 0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x90);
+
+static const uuid_t mtk_vdec_core_uuid =
+ UUID_INIT(0xBC50D971, 0xD4C9, 0x42C4,
+ 0x82, 0xCB, 0x34, 0x3F, 0xB7, 0xF3, 0x78, 0x91);
+
+/*
+ * Check whether this driver supports decoder TA in the TEE instance,
+ * represented by the params (ver/data) of this function.
+ */
+static int mtk_vcodec_dec_optee_match(struct tee_ioctl_version_data *ver_data, const void *not_used)
+{
+ if (ver_data->impl_id == TEE_IMPL_ID_OPTEE)
+ return 1;
+ else
+ return 0;
+}
+
+int mtk_vcodec_dec_optee_private_init(struct mtk_vcodec_dec_dev *vcodec_dev)
+{
+ vcodec_dev->optee_private = devm_kzalloc(&vcodec_dev->plat_dev->dev,
+ sizeof(*vcodec_dev->optee_private),
+ GFP_KERNEL);
+ if (!vcodec_dev->optee_private)
+ return -ENOMEM;
+
+ vcodec_dev->optee_private->vcodec_dev = vcodec_dev;
+
+ atomic_set(&vcodec_dev->optee_private->tee_active_cnt, 0);
+ mutex_init(&vcodec_dev->optee_private->tee_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_private_init);
+
+static int mtk_vcodec_dec_optee_init_hw_info(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hardware_index)
+{
+ struct device *dev = &optee_private->vcodec_dev->plat_dev->dev;
+ struct tee_ioctl_open_session_arg session_arg;
+ struct mtk_vdec_optee_ca_info *ca_info;
+ int err = 0, session_func;
+
+ /* Open lat and core session with vdec TA. */
+ switch (hardware_index) {
+ case MTK_VDEC_LAT0:
+ export_uuid(session_arg.uuid, &mtk_vdec_lat_uuid);
+ session_func = MTK_VDEC_OPTEE_TA_LAT_SUBMIT_COMMAND;
+ ca_info = &optee_private->lat_ca;
+ break;
+ case MTK_VDEC_CORE:
+ export_uuid(session_arg.uuid, &mtk_vdec_core_uuid);
+ session_func = MTK_VDEC_OPTEE_TA_CORE_SUBMIT_COMMAND;
+ ca_info = &optee_private->core_ca;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ session_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ session_arg.num_params = 0;
+
+ err = tee_client_open_session(optee_private->tee_vdec_ctx, &session_arg, NULL);
+ if (err < 0 || session_arg.ret != 0) {
+ dev_err(dev, MTK_DBG_VCODEC_STR "open vdec tee session fail hw_id:%d err:%x.\n",
+ hardware_index, session_arg.ret);
+ return -EINVAL;
+ }
+ ca_info->vdec_session_id = session_arg.session;
+ ca_info->hw_id = hardware_index;
+ ca_info->vdec_session_func = session_func;
+
+ dev_dbg(dev, MTK_DBG_VCODEC_STR "open vdec tee session hw_id:%d session_id=%x.\n",
+ hardware_index, ca_info->vdec_session_id);
+
+ return err;
+}
+
+static void mtk_vcodec_dec_optee_deinit_hw_info(struct mtk_vdec_optee_private *optee_private,
+ enum mtk_vdec_hw_id hw_id)
+{
+ struct mtk_vdec_optee_ca_info *ca_info;
+
+ if (hw_id == MTK_VDEC_LAT0)
+ ca_info = &optee_private->lat_ca;
+ else
+ ca_info = &optee_private->core_ca;
+
+ tee_client_close_session(optee_private->tee_vdec_ctx, ca_info->vdec_session_id);
+}
+
+int mtk_vcodec_dec_optee_open(struct mtk_vdec_optee_private *optee_private)
+{
+ struct device *dev = &optee_private->vcodec_dev->plat_dev->dev;
+ int ret;
+
+ mutex_lock(&optee_private->tee_mutex);
+ if (atomic_inc_return(&optee_private->tee_active_cnt) > 1) {
+ mutex_unlock(&optee_private->tee_mutex);
+ dev_dbg(dev, MTK_DBG_VCODEC_STR "already init vdec optee private data!\n");
+ return 0;
+ }
+
+ /* Open context with TEE driver */
+ optee_private->tee_vdec_ctx = tee_client_open_context(NULL, mtk_vcodec_dec_optee_match,
+ NULL, NULL);
+ if (IS_ERR(optee_private->tee_vdec_ctx)) {
+ dev_err(dev, MTK_DBG_VCODEC_STR "optee vdec tee context failed.\n");
+ ret = PTR_ERR(optee_private->tee_vdec_ctx);
+ goto err_ctx_open;
+ }
+
+ ret = mtk_vcodec_dec_optee_init_hw_info(optee_private, MTK_VDEC_LAT0);
+ if (ret < 0)
+ goto err_lat_init;
+
+ if (IS_VDEC_LAT_ARCH(optee_private->vcodec_dev->vdec_pdata->hw_arch)) {
+ ret = mtk_vcodec_dec_optee_init_hw_info(optee_private, MTK_VDEC_CORE);
+ if (ret < 0)
+ goto err_core_init;
+ }
+
+ mutex_unlock(&optee_private->tee_mutex);
+ return 0;
+err_core_init:
+ mtk_vcodec_dec_optee_deinit_hw_info(optee_private, MTK_VDEC_LAT0);
+err_lat_init:
+ tee_client_close_context(optee_private->tee_vdec_ctx);
+err_ctx_open:
+
+ mutex_unlock(&optee_private->tee_mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_open);
+
+void mtk_vcodec_dec_optee_release(struct mtk_vdec_optee_private *optee_private)
+{
+ mutex_lock(&optee_private->tee_mutex);
+ if (!atomic_dec_and_test(&optee_private->tee_active_cnt)) {
+ mutex_unlock(&optee_private->tee_mutex);
+ return;
+ }
+
+ mtk_vcodec_dec_optee_deinit_hw_info(optee_private, MTK_VDEC_LAT0);
+ if (IS_VDEC_LAT_ARCH(optee_private->vcodec_dev->vdec_pdata->hw_arch))
+ mtk_vcodec_dec_optee_deinit_hw_info(optee_private, MTK_VDEC_CORE);
+
+ tee_client_close_context(optee_private->tee_vdec_ctx);
+ mutex_unlock(&optee_private->tee_mutex);
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dec_optee_release);
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
new file mode 100644
index 000000000000..8b1dca49331e
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_optee.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Yunfei Dong <[email protected]>
+ */
+
+#ifndef _MTK_VCODEC_DEC_OPTEE_H_
+#define _MTK_VCODEC_DEC_OPTEE_H_
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+
+#include "mtk_vcodec_dec_drv.h"
+
+/* The TA ID implemented in this TA */
+#define MTK_VDEC_OPTEE_TA_LAT_SUBMIT_COMMAND (0x10)
+#define MTK_VDEC_OPTEE_TA_CORE_SUBMIT_COMMAND (0x20)
+
+#define MTK_OPTEE_MAX_TEE_PARAMS 4
+
+/**
+ * struct mtk_vdec_optee_ca_info - ca related param
+ * @vdec_session_id: optee TA session identifier.
+ * @hw_id: hardware index.
+ * @vdec_session_func: trusted application function id used specific to the TA.
+ */
+struct mtk_vdec_optee_ca_info {
+ u32 vdec_session_id;
+ enum mtk_vdec_hw_id hw_id;
+ u32 vdec_session_func;
+};
+
+/**
+ * struct mtk_vdec_optee_private - optee private data
+ * @vcodec_dev: pointer to the mtk_vcodec_dev of the device
+ * @tee_vdec_ctx: decoder TEE context handler.
+ * @lat_ca: lat hardware information used to communicate with TA.
+ * @core_ca: core hardware information used to communicate with TA.
+ *
+ * @tee_active_cnt: used to mark whether need to init optee
+ * @tee_mutex: mutex lock used for optee
+ */
+struct mtk_vdec_optee_private {
+ struct mtk_vcodec_dec_dev *vcodec_dev;
+ struct tee_context *tee_vdec_ctx;
+
+ struct mtk_vdec_optee_ca_info lat_ca;
+ struct mtk_vdec_optee_ca_info core_ca;
+
+ atomic_t tee_active_cnt;
+ /* mutext used to lock optee open and release information. */
+ struct mutex tee_mutex;
+};
+
+/**
+ * mtk_vcodec_dec_optee_open - setup the communication channels with TA.
+ * @optee_private: optee private context
+ */
+int mtk_vcodec_dec_optee_open(struct mtk_vdec_optee_private *optee_private);
+
+/**
+ * mtk_vcodec_dec_optee_private_init - init optee parameters.
+ * @vcodec_dev: pointer to the mtk_vcodec_dev of the device
+ */
+int mtk_vcodec_dec_optee_private_init(struct mtk_vcodec_dec_dev *vcodec_dev);
+
+/**
+ * mtk_vcodec_dec_optee_release - close the communication channels with TA.
+ * @optee_private: optee private context
+ */
+void mtk_vcodec_dec_optee_release(struct mtk_vdec_optee_private *optee_private);
+
+#endif /* _MTK_VCODEC_FW_OPTEE_H_ */
--
2.18.0

2023-12-06 08:18:24

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,19/21] media: medkatek: vcodec: disable wait interrupt for svp mode

Waiting interrupt in optee-os for svp mode, need to disable it in kernel
in case of interrupt is cleaned.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../vcodec/decoder/mtk_vcodec_dec_hw.c | 34 +++++------
.../vcodec/decoder/mtk_vcodec_dec_pm.c | 6 +-
.../decoder/vdec/vdec_h264_req_multi_if.c | 57 +++++++++++--------
3 files changed, 54 insertions(+), 43 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
index 881d5de41e05..1982c088c6da 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.c
@@ -72,26 +72,28 @@ static irqreturn_t mtk_vdec_hw_irq_handler(int irq, void *priv)

ctx = mtk_vcodec_get_curr_ctx(dev->main_dev, dev->hw_idx);

- /* check if HW active or not */
- cg_status = readl(dev->reg_base[VDEC_HW_SYS] + VDEC_HW_ACTIVE_ADDR);
- if (cg_status & VDEC_HW_ACTIVE_MASK) {
- mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0 (0x%08x)", cg_status);
- return IRQ_HANDLED;
- }
+ if (!ctx->is_secure_playback) {
+ /* check if HW active or not */
+ cg_status = readl(dev->reg_base[VDEC_HW_SYS] + VDEC_HW_ACTIVE_ADDR);
+ if (cg_status & VDEC_HW_ACTIVE_MASK) {
+ mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0 (0x%08x)", cg_status);
+ return IRQ_HANDLED;
+ }

- dec_done_status = readl(vdec_misc_addr);
- if ((dec_done_status & MTK_VDEC_IRQ_STATUS_DEC_SUCCESS) !=
- MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)
- return IRQ_HANDLED;
+ dec_done_status = readl(vdec_misc_addr);
+ if ((dec_done_status & MTK_VDEC_IRQ_STATUS_DEC_SUCCESS) !=
+ MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)
+ return IRQ_HANDLED;

- /* clear interrupt */
- writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
- writel(dec_done_status & ~VDEC_IRQ_CLR, vdec_misc_addr);
+ /* clear interrupt */
+ writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
+ writel(dec_done_status & ~VDEC_IRQ_CLR, vdec_misc_addr);

- wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev->hw_idx);
+ wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev->hw_idx);

- mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d, dec_done_status=%x",
- ctx->id, dec_done_status);
+ mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d, dec_done_status=%x",
+ ctx->id, dec_done_status);
+ }

return IRQ_HANDLED;
}
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.c
index aefd3e9e3061..a94eda936f16 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.c
@@ -238,7 +238,8 @@ void mtk_vcodec_dec_enable_hardware(struct mtk_vcodec_dec_ctx *ctx, int hw_idx)
mtk_vcodec_dec_child_dev_on(ctx->dev, MTK_VDEC_LAT0);
mtk_vcodec_dec_child_dev_on(ctx->dev, hw_idx);

- mtk_vcodec_dec_enable_irq(ctx->dev, hw_idx);
+ if (!ctx->is_secure_playback)
+ mtk_vcodec_dec_enable_irq(ctx->dev, hw_idx);

if (IS_VDEC_INNER_RACING(ctx->dev->dec_capability))
mtk_vcodec_load_racing_info(ctx);
@@ -250,7 +251,8 @@ void mtk_vcodec_dec_disable_hardware(struct mtk_vcodec_dec_ctx *ctx, int hw_idx)
if (IS_VDEC_INNER_RACING(ctx->dev->dec_capability))
mtk_vcodec_record_racing_info(ctx);

- mtk_vcodec_dec_disable_irq(ctx->dev, hw_idx);
+ if (!ctx->is_secure_playback)
+ mtk_vcodec_dec_disable_irq(ctx->dev, hw_idx);

mtk_vcodec_dec_child_dev_off(ctx->dev, hw_idx);
if (IS_VDEC_LAT_ARCH(ctx->dev->vdec_pdata->hw_arch) &&
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
index 2dfb3043493e..3e2270399b6c 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_multi_if.c
@@ -593,14 +593,16 @@ static int vdec_h264_slice_core_decode(struct vdec_lat_buf *lat_buf)
goto vdec_dec_end;
}

- /* wait decoder done interrupt */
- timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
- WAIT_INTR_TIMEOUT_MS, MTK_VDEC_CORE);
- if (timeout)
- mtk_vdec_err(ctx, "core decode timeout: pic_%d", ctx->decoded_frame_cnt);
- inst->vsi_core->dec.timeout = !!timeout;
-
- vpu_dec_core_end(vpu);
+ if (!ctx->is_secure_playback) {
+ /* wait decoder done interrupt */
+ timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
+ WAIT_INTR_TIMEOUT_MS, MTK_VDEC_CORE);
+ if (timeout)
+ mtk_vdec_err(ctx, "core decode timeout: pic_%d", ctx->decoded_frame_cnt);
+ inst->vsi_core->dec.timeout = !!timeout;
+
+ vpu_dec_core_end(vpu);
+ }
mtk_vdec_debug(ctx, "pic[%d] crc: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x",
ctx->decoded_frame_cnt,
inst->vsi_core->dec.crc[0], inst->vsi_core->dec.crc[1],
@@ -724,14 +726,16 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
vdec_msg_queue_qbuf(&inst->ctx->msg_queue.core_ctx, lat_buf);
}

- /* wait decoder done interrupt */
- timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
- WAIT_INTR_TIMEOUT_MS, MTK_VDEC_LAT0);
- if (timeout)
- mtk_vdec_err(inst->ctx, "lat decode timeout: pic_%d", inst->slice_dec_num);
- inst->vsi->dec.timeout = !!timeout;
+ if (!inst->ctx->is_secure_playback) {
+ /* wait decoder done interrupt */
+ timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
+ WAIT_INTR_TIMEOUT_MS, MTK_VDEC_LAT0);
+ if (timeout)
+ mtk_vdec_err(inst->ctx, "lat decode timeout: pic_%d", inst->slice_dec_num);
+ inst->vsi->dec.timeout = !!timeout;

- err = vpu_dec_end(vpu);
+ err = vpu_dec_end(vpu);
+ }
if (err == SLICE_HEADER_FULL || err == TRANS_BUFFER_FULL) {
if (!IS_VDEC_INNER_RACING(inst->ctx->dev->dec_capability))
vdec_msg_queue_qbuf(&inst->ctx->msg_queue.lat_ctx, lat_buf);
@@ -831,16 +835,19 @@ static int vdec_h264_slice_single_decode(void *h_vdec, struct mtk_vcodec_mem *bs
if (err)
goto err_free_fb_out;

- /* wait decoder done interrupt */
- err = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
- WAIT_INTR_TIMEOUT_MS, MTK_VDEC_CORE);
- if (err)
- mtk_vdec_err(inst->ctx, "decode timeout: pic_%d", inst->ctx->decoded_frame_cnt);
-
- inst->vsi->dec.timeout = !!err;
- err = vpu_dec_end(vpu);
- if (err)
- goto err_free_fb_out;
+ if (!inst->ctx->is_secure_playback) {
+ /* wait decoder done interrupt */
+ err = mtk_vcodec_wait_for_done_ctx(inst->ctx, MTK_INST_IRQ_RECEIVED,
+ WAIT_INTR_TIMEOUT_MS, MTK_VDEC_CORE);
+ if (err)
+ mtk_vdec_err(inst->ctx, "decode timeout: pic_%d",
+ inst->ctx->decoded_frame_cnt);
+
+ inst->vsi->dec.timeout = !!err;
+ err = vpu_dec_end(vpu);
+ if (err)
+ goto err_free_fb_out;
+ }

memcpy(&inst->vsi_ctx, inst->vpu.vsi, sizeof(inst->vsi_ctx));
mtk_vdec_debug(inst->ctx, "pic[%d] crc: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x",
--
2.18.0

2023-12-06 08:18:28

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,20/21] media: medkatek: vcodec: support tee decoder

Initialize tee private data to support secure decoder.
Release tee related information for each instance when decoder
done.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index f47c98faf068..08e7d250487b 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -310,6 +310,9 @@ static int fops_vcodec_release(struct file *file)
v4l2_fh_exit(&ctx->fh);
v4l2_ctrl_handler_free(&ctx->ctrl_hdl);

+ if (ctx->is_secure_playback)
+ mtk_vcodec_dec_optee_release(dev->optee_private);
+
mtk_vcodec_dbgfs_remove(dev, ctx->id);
list_del_init(&ctx->list);
kfree(ctx);
@@ -466,6 +469,11 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
atomic_set(&dev->dec_active_cnt, 0);
memset(dev->vdec_racing_info, 0, sizeof(dev->vdec_racing_info));
mutex_init(&dev->dec_racing_info_mutex);
+ ret = mtk_vcodec_dec_optee_private_init(dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to init svp private.");
+ goto err_reg_cont;
+ }

ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, -1);
if (ret) {
--
2.18.0

2023-12-06 08:18:30

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH v3,21/21] media: mediatek: vcodec: move vdec init interface to setup callback

Getting secure video playback (svp) flag when request output buffer, then
calling init interface to init svp parameters in optee-os.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 144 ++++++++++++------
1 file changed, 94 insertions(+), 50 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index ab922e8d2d37..3639beac20cb 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -184,6 +184,74 @@ void mtk_vcodec_dec_set_default_params(struct mtk_vcodec_dec_ctx *ctx)
q_data->bytesperline[1] = q_data->coded_width;
}

+static int mtk_vcodec_dec_init_pic_info(struct mtk_vcodec_dec_ctx *ctx, enum v4l2_buf_type type)
+{
+ const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev->vdec_pdata;
+ struct mtk_q_data *q_data;
+ int ret;
+
+ if (!ctx->current_codec)
+ return 0;
+
+ if (V4L2_TYPE_IS_OUTPUT(type) && ctx->state == MTK_STATE_FREE) {
+ q_data = mtk_vdec_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+ if (!q_data)
+ return -EINVAL;
+
+ ret = vdec_if_init(ctx, q_data->fmt->fourcc);
+ if (ret) {
+ mtk_v4l2_vdec_err(ctx, "[%d]: vdec_if_init() fail ret=%d",
+ ctx->id, ret);
+ return -EINVAL;
+ }
+ ctx->state = MTK_STATE_INIT;
+ }
+
+ if (!dec_pdata->uses_stateless_api)
+ return 0;
+
+ /*
+ * If get pic info fail, need to use the default pic info params, or
+ * v4l2-compliance will fail
+ */
+ ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
+ if (ret) {
+ mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail",
+ ctx->id);
+ }
+
+ ctx->last_decoded_picinfo = ctx->picinfo;
+
+ if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) {
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
+ ctx->picinfo.fb_sz[0] +
+ ctx->picinfo.fb_sz[1];
+ ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] =
+ ctx->picinfo.buf_w;
+ } else {
+ if (ctx->is_secure_playback)
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
+ ctx->picinfo.fb_sz[0] + ctx->picinfo.fb_sz[1];
+ else
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = ctx->picinfo.fb_sz[0];
+
+ ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = ctx->picinfo.buf_w;
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] = ctx->picinfo.fb_sz[1];
+ ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] = ctx->picinfo.buf_w;
+ }
+
+ ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w;
+ ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h;
+ mtk_v4l2_vdec_dbg(2, ctx,
+ "[%d] init() plane:%d wxh=%dx%d pic wxh=%dx%d sz=0x%x_0x%x",
+ ctx->id, q_data->fmt->num_planes,
+ ctx->picinfo.buf_w, ctx->picinfo.buf_h,
+ ctx->picinfo.pic_w, ctx->picinfo.pic_h,
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0],
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]);
+ return 0;
+}
+
static int vidioc_vdec_qbuf(struct file *file, void *priv,
struct v4l2_buffer *buf)
{
@@ -479,17 +547,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
ctx->ycbcr_enc = pix_mp->ycbcr_enc;
ctx->quantization = pix_mp->quantization;
ctx->xfer_func = pix_mp->xfer_func;
-
ctx->current_codec = fmt->fourcc;
- if (ctx->state == MTK_STATE_FREE) {
- ret = vdec_if_init(ctx, q_data->fmt->fourcc);
- if (ret) {
- mtk_v4l2_vdec_err(ctx, "[%d]: vdec_if_init() fail ret=%d",
- ctx->id, ret);
- return -EINVAL;
- }
- ctx->state = MTK_STATE_INIT;
- }
} else {
ctx->capture_fourcc = fmt->fourcc;
}
@@ -502,46 +560,11 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
ctx->picinfo.pic_w = pix_mp->width;
ctx->picinfo.pic_h = pix_mp->height;

- /*
- * If get pic info fail, need to use the default pic info params, or
- * v4l2-compliance will fail
- */
- ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
- if (ret) {
- mtk_v4l2_vdec_err(ctx, "[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail",
- ctx->id);
- }
-
- ctx->last_decoded_picinfo = ctx->picinfo;
-
- if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) {
- ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
- ctx->picinfo.fb_sz[0] +
- ctx->picinfo.fb_sz[1];
- ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] =
- ctx->picinfo.buf_w;
- } else {
- ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
- ctx->picinfo.fb_sz[0];
- ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] =
- ctx->picinfo.buf_w;
- ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] =
- ctx->picinfo.fb_sz[1];
- ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] =
- ctx->picinfo.buf_w;
- }
-
- ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w;
- ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h;
- mtk_v4l2_vdec_dbg(2, ctx,
- "[%d] init() plane:%d wxh=%dx%d pic wxh=%dx%d sz=0x%x_0x%x",
- ctx->id, pix_mp->num_planes,
- ctx->picinfo.buf_w, ctx->picinfo.buf_h,
- ctx->picinfo.pic_w, ctx->picinfo.pic_h,
- ctx->q_data[MTK_Q_DATA_DST].sizeimage[0],
- ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]);
+ if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+ ret = mtk_vcodec_dec_init_pic_info(ctx, f->type);
}
- return 0;
+
+ return ret;
}

static int vidioc_enum_framesizes(struct file *file, void *priv,
@@ -722,7 +745,7 @@ int vb2ops_vdec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
{
struct mtk_vcodec_dec_ctx *ctx = vb2_get_drv_priv(vq);
struct mtk_q_data *q_data;
- unsigned int i;
+ unsigned int i, ret;

q_data = mtk_vdec_get_q_data(ctx, vq->type);

@@ -731,6 +754,25 @@ int vb2ops_vdec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
return -EINVAL;
}

+ if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+ ret = mtk_vcodec_dec_init_pic_info(ctx, vq->type);
+ if (ret) {
+ mtk_v4l2_vdec_err(ctx, "Failed to init picture information");
+ return ret;
+ }
+
+ if (vq->secure_mem && !ctx->is_secure_playback) {
+ ret = mtk_vcodec_dec_optee_open(ctx->dev->optee_private);
+ if (ret) {
+ mtk_v4l2_vdec_err(ctx, "Failed to open decoder optee os");
+ return ret;
+ }
+ ctx->is_secure_playback = vq->secure_mem;
+ mtk_v4l2_vdec_dbg(1, ctx, "Getting secure decoder mode:%d",
+ ctx->is_secure_playback);
+ }
+ }
+
if (*nplanes) {
if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
if (*nplanes != q_data->fmt->num_planes)
@@ -980,6 +1022,7 @@ int mtk_vcodec_dec_queue_init(void *priv, struct vb2_queue *src_vq,
src_vq->lock = &ctx->dev->dev_mutex;
src_vq->dev = &ctx->dev->plat_dev->dev;
src_vq->allow_cache_hints = 1;
+ src_vq->allow_secure_mem = 1;

ret = vb2_queue_init(src_vq);
if (ret) {
@@ -996,6 +1039,7 @@ int mtk_vcodec_dec_queue_init(void *priv, struct vb2_queue *src_vq,
dst_vq->lock = &ctx->dev->dev_mutex;
dst_vq->dev = &ctx->dev->plat_dev->dev;
dst_vq->allow_cache_hints = 1;
+ dst_vq->allow_secure_mem = 1;

ret = vb2_queue_init(dst_vq);
if (ret)
--
2.18.0

2023-12-06 08:47:00

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3,16/21] media: medkatek: vcodec: support one plane capture buffer

On 06/12/2023 09:15, Yunfei Dong wrote:
> The capture buffer has two planes for format MM21, but user space only
> allocate secure memory for plane[0], and the size is Y data + uv data.
> The driver need to support one plane decoder for svp mode.

For a future v4: note the typo in the Subject line: medkatek -> mediatek.
It's present in patches 16-20.

Regards,

Hans

>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 7 ++++-
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 26 ++++++++++---------
> .../decoder/vdec/vdec_h264_req_common.c | 11 +++-----
> .../mediatek/vcodec/decoder/vdec_drv_if.c | 4 +--
> 4 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> index 604fdc8ee3ce..ab922e8d2d37 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> @@ -653,7 +653,12 @@ static int vidioc_vdec_g_fmt(struct file *file, void *priv,
> * So we just return picinfo yet, and update picinfo in
> * stop_streaming hook function
> */
> - q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> +
> + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1)
> + q_data->sizeimage[0] = ctx->picinfo.fb_sz[0] + ctx->picinfo.fb_sz[1];
> + else
> + q_data->sizeimage[0] = ctx->picinfo.fb_sz[0];
> +
> q_data->sizeimage[1] = ctx->picinfo.fb_sz[1];
> q_data->bytesperline[0] = ctx->last_decoded_picinfo.buf_w;
> q_data->bytesperline[1] = ctx->last_decoded_picinfo.buf_w;
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> index cc42c942eb8a..707ed57a412e 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> @@ -285,14 +285,14 @@ static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_dec_ctx *ctx)
> framebuf = container_of(vb2_v4l2, struct mtk_video_dec_buf, m2m_buf.vb);
>
> pfb = &framebuf->frame_buffer;
> - pfb->base_y.va = vb2_plane_vaddr(dst_buf, 0);
> + if (!ctx->is_secure_playback)
> + pfb->base_y.va = vb2_plane_vaddr(dst_buf, 0);
> pfb->base_y.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> pfb->base_y.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0];
>
> - if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) {
> + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2 && !ctx->is_secure_playback) {
> pfb->base_c.va = vb2_plane_vaddr(dst_buf, 1);
> - pfb->base_c.dma_addr =
> - vb2_dma_contig_plane_dma_addr(dst_buf, 1);
> + pfb->base_c.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 1);
> pfb->base_c.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[1];
> }
> mtk_v4l2_vdec_dbg(1, ctx,
> @@ -339,16 +339,18 @@ static void mtk_vdec_worker(struct work_struct *work)
> mtk_v4l2_vdec_dbg(3, ctx, "[%d] (%d) id=%d, vb=%p", ctx->id,
> vb2_src->vb2_queue->type, vb2_src->index, vb2_src);
>
> - bs_src->va = vb2_plane_vaddr(vb2_src, 0);
> - bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0);
> - bs_src->size = (size_t)vb2_src->planes[0].bytesused;
> - if (!bs_src->va) {
> - v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
> - mtk_v4l2_vdec_err(ctx, "[%d] id=%d source buffer is NULL", ctx->id,
> - vb2_src->index);
> - return;
> + if (!ctx->is_secure_playback) {
> + bs_src->va = vb2_plane_vaddr(vb2_src, 0);
> + if (!bs_src->va) {
> + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
> + mtk_v4l2_vdec_err(ctx, "[%d] id=%d source buffer is NULL", ctx->id,
> + vb2_src->index);
> + return;
> + }
> }
>
> + bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0);
> + bs_src->size = (size_t)vb2_src->planes[0].bytesused;
> mtk_v4l2_vdec_dbg(3, ctx, "[%d] Bitstream VA=%p DMA=%pad Size=%zx vb=%p",
> ctx->id, bs_src->va, &bs_src->dma_addr, bs_src->size, vb2_src);
> /* Apply request controls. */
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
> index 5ca20d75dc8e..2a57e689ec07 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req_common.c
> @@ -79,15 +79,12 @@ void mtk_vdec_h264_fill_dpb_info(struct mtk_vcodec_dec_ctx *ctx,
> vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
> h264_dpb_info[index].field = vb2_v4l2->field;
>
> - h264_dpb_info[index].y_dma_addr =
> - vb2_dma_contig_plane_dma_addr(vb, 0);
> + h264_dpb_info[index].y_dma_addr = vb2_dma_contig_plane_dma_addr(vb, 0);
> if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2)
> + h264_dpb_info[index].c_dma_addr = vb2_dma_contig_plane_dma_addr(vb, 1);
> + else if (!ctx->is_secure_playback)
> h264_dpb_info[index].c_dma_addr =
> - vb2_dma_contig_plane_dma_addr(vb, 1);
> - else
> - h264_dpb_info[index].c_dma_addr =
> - h264_dpb_info[index].y_dma_addr +
> - ctx->picinfo.fb_sz[0];
> + h264_dpb_info[index].y_dma_addr + ctx->picinfo.fb_sz[0];
> }
> }
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
> index d0b459b1603f..fb3e4f75ed93 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_drv_if.c
> @@ -73,14 +73,14 @@ int vdec_if_decode(struct mtk_vcodec_dec_ctx *ctx, struct mtk_vcodec_mem *bs,
> {
> int ret = 0;
>
> - if (bs) {
> + if (bs && !ctx->is_secure_playback) {
> if ((bs->dma_addr & 63) != 0) {
> mtk_v4l2_vdec_err(ctx, "bs dma_addr should 64 byte align");
> return -EINVAL;
> }
> }
>
> - if (fb) {
> + if (fb && !ctx->is_secure_playback) {
> if (((fb->base_y.dma_addr & 511) != 0) ||
> ((fb->base_c.dma_addr & 511) != 0)) {
> mtk_v4l2_vdec_err(ctx, "frame buffer dma_addr should 512 byte align");

2023-12-06 23:44:25

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] media: medkatek: vcodec: support tee decoder

Hi Yunfei,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Yunfei-Dong/media-medkatek-vcodec-support-tee-decoder/20231206-201843
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20231206081538.17056-21-yunfei.dong%40mediatek.com
patch subject: [PATCH] media: medkatek: vcodec: support tee decoder
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231207/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c: In function 'fops_vcodec_release':
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:313:16: error: 'struct mtk_vcodec_dec_ctx' has no member named 'is_secure_playback'
313 | if (ctx->is_secure_playback)
| ^~
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:314:17: error: implicit declaration of function 'mtk_vcodec_dec_optee_release'; did you mean 'mtk_vcodec_dec_release'? [-Werror=implicit-function-declaration]
314 | mtk_vcodec_dec_optee_release(dev->optee_private);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| mtk_vcodec_dec_release
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:314:49: error: 'struct mtk_vcodec_dec_dev' has no member named 'optee_private'
314 | mtk_vcodec_dec_optee_release(dev->optee_private);
| ^~
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c: In function 'mtk_vcodec_probe':
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:472:15: error: implicit declaration of function 'mtk_vcodec_dec_optee_private_init'; did you mean 'mtk_vcodec_dec_queue_init'? [-Werror=implicit-function-declaration]
472 | ret = mtk_vcodec_dec_optee_private_init(dev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| mtk_vcodec_dec_queue_init
cc1: some warnings being treated as errors


vim +313 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c

291
292 static int fops_vcodec_release(struct file *file)
293 {
294 struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
295 struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(file->private_data);
296
297 mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id);
298 mutex_lock(&dev->dev_mutex);
299
300 /*
301 * Call v4l2_m2m_ctx_release before mtk_vcodec_dec_release. First, it
302 * makes sure the worker thread is not running after vdec_if_deinit.
303 * Second, the decoder will be flushed and all the buffers will be
304 * returned in stop_streaming.
305 */
306 v4l2_m2m_ctx_release(ctx->m2m_ctx);
307 mtk_vcodec_dec_release(ctx);
308
309 v4l2_fh_del(&ctx->fh);
310 v4l2_fh_exit(&ctx->fh);
311 v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
312
> 313 if (ctx->is_secure_playback)
> 314 mtk_vcodec_dec_optee_release(dev->optee_private);
315
316 mtk_vcodec_dbgfs_remove(dev, ctx->id);
317 list_del_init(&ctx->list);
318 kfree(ctx);
319 mutex_unlock(&dev->dev_mutex);
320 return 0;
321 }
322
323 static const struct v4l2_file_operations mtk_vcodec_fops = {
324 .owner = THIS_MODULE,
325 .open = fops_vcodec_open,
326 .release = fops_vcodec_release,
327 .poll = v4l2_m2m_fop_poll,
328 .unlocked_ioctl = video_ioctl2,
329 .mmap = v4l2_m2m_fop_mmap,
330 };
331
332 static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
333 {
334 struct device *dev = &vdec_dev->plat_dev->dev;
335
336 if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
337 vdec_dev->chip_name = MTK_VDEC_MT8173;
338 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
339 vdec_dev->chip_name = MTK_VDEC_MT8183;
340 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
341 vdec_dev->chip_name = MTK_VDEC_MT8192;
342 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
343 vdec_dev->chip_name = MTK_VDEC_MT8195;
344 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
345 vdec_dev->chip_name = MTK_VDEC_MT8186;
346 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
347 vdec_dev->chip_name = MTK_VDEC_MT8188;
348 else
349 vdec_dev->chip_name = MTK_VDEC_INVAL;
350 }
351
352 static int mtk_vcodec_probe(struct platform_device *pdev)
353 {
354 struct mtk_vcodec_dec_dev *dev;
355 struct video_device *vfd_dec;
356 phandle rproc_phandle;
357 enum mtk_vcodec_fw_type fw_type;
358 int i, ret;
359
360 dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
361 if (!dev)
362 return -ENOMEM;
363
364 INIT_LIST_HEAD(&dev->ctx_list);
365 dev->plat_dev = pdev;
366
367 mtk_vcodec_dec_get_chip_name(dev);
368 if (dev->chip_name == MTK_VDEC_INVAL) {
369 dev_err(&pdev->dev, "Failed to get decoder chip name");
370 return -EINVAL;
371 }
372
373 dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
374 if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
375 &rproc_phandle)) {
376 fw_type = VPU;
377 } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
378 &rproc_phandle)) {
379 fw_type = SCP;
380 } else {
381 dev_dbg(&pdev->dev, "Could not get vdec IPI device");
382 return -ENODEV;
383 }
384 dma_set_max_seg_size(&pdev->dev, UINT_MAX);
385
386 dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, DECODER);
387 if (IS_ERR(dev->fw_handler))
388 return PTR_ERR(dev->fw_handler);
389
390 ret = mtk_vcodec_init_dec_resources(dev);
391 if (ret) {
392 dev_err(&pdev->dev, "Failed to init dec resources");
393 goto err_dec_pm;
394 }
395
396 if (IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch)) {
397 dev->core_workqueue =
398 alloc_ordered_workqueue("core-decoder",
399 WQ_MEM_RECLAIM | WQ_FREEZABLE);
400 if (!dev->core_workqueue) {
401 dev_dbg(&pdev->dev, "Failed to create core workqueue");
402 ret = -EINVAL;
403 goto err_res;
404 }
405 }
406
407 for (i = 0; i < MTK_VDEC_HW_MAX; i++)
408 mutex_init(&dev->dec_mutex[i]);
409 mutex_init(&dev->dev_mutex);
410 spin_lock_init(&dev->irqlock);
411
412 snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
413 "[/MTK_V4L2_VDEC]");
414
415 ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
416 if (ret) {
417 dev_err(&pdev->dev, "v4l2_device_register err=%d", ret);
418 goto err_core_workq;
419 }
420
421 vfd_dec = video_device_alloc();
422 if (!vfd_dec) {
423 dev_err(&pdev->dev, "Failed to allocate video device");
424 ret = -ENOMEM;
425 goto err_dec_alloc;
426 }
427 vfd_dec->fops = &mtk_vcodec_fops;
428 vfd_dec->ioctl_ops = &mtk_vdec_ioctl_ops;
429 vfd_dec->release = video_device_release;
430 vfd_dec->lock = &dev->dev_mutex;
431 vfd_dec->v4l2_dev = &dev->v4l2_dev;
432 vfd_dec->vfl_dir = VFL_DIR_M2M;
433 vfd_dec->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE |
434 V4L2_CAP_STREAMING;
435
436 snprintf(vfd_dec->name, sizeof(vfd_dec->name), "%s",
437 MTK_VCODEC_DEC_NAME);
438 video_set_drvdata(vfd_dec, dev);
439 dev->vfd_dec = vfd_dec;
440 platform_set_drvdata(pdev, dev);
441
442 dev->m2m_dev_dec = v4l2_m2m_init(&mtk_vdec_m2m_ops);
443 if (IS_ERR((__force void *)dev->m2m_dev_dec)) {
444 dev_err(&pdev->dev, "Failed to init mem2mem dec device");
445 ret = PTR_ERR((__force void *)dev->m2m_dev_dec);
446 goto err_dec_alloc;
447 }
448
449 dev->decode_workqueue =
450 alloc_ordered_workqueue(MTK_VCODEC_DEC_NAME,
451 WQ_MEM_RECLAIM | WQ_FREEZABLE);
452 if (!dev->decode_workqueue) {
453 dev_err(&pdev->dev, "Failed to create decode workqueue");
454 ret = -EINVAL;
455 goto err_event_workq;
456 }
457
458 if (dev->vdec_pdata->is_subdev_supported) {
459 ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
460 &pdev->dev);
461 if (ret) {
462 dev_err(&pdev->dev, "Main device of_platform_populate failed.");
463 goto err_reg_cont;
464 }
465 } else {
466 set_bit(MTK_VDEC_CORE, dev->subdev_bitmap);
467 }
468
469 atomic_set(&dev->dec_active_cnt, 0);
470 memset(dev->vdec_racing_info, 0, sizeof(dev->vdec_racing_info));
471 mutex_init(&dev->dec_racing_info_mutex);
> 472 ret = mtk_vcodec_dec_optee_private_init(dev);
473 if (ret) {
474 dev_err(&pdev->dev, "Failed to init svp private.");
475 goto err_reg_cont;
476 }
477
478 ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, -1);
479 if (ret) {
480 dev_err(&pdev->dev, "Failed to register video device");
481 goto err_reg_cont;
482 }
483
484 if (dev->vdec_pdata->uses_stateless_api) {
485 v4l2_disable_ioctl(vfd_dec, VIDIOC_DECODER_CMD);
486 v4l2_disable_ioctl(vfd_dec, VIDIOC_TRY_DECODER_CMD);
487
488 dev->mdev_dec.dev = &pdev->dev;
489 strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME,
490 sizeof(dev->mdev_dec.model));
491
492 media_device_init(&dev->mdev_dec);
493 dev->mdev_dec.ops = &mtk_vcodec_media_ops;
494 dev->v4l2_dev.mdev = &dev->mdev_dec;
495
496 ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, dev->vfd_dec,
497 MEDIA_ENT_F_PROC_VIDEO_DECODER);
498 if (ret) {
499 dev_err(&pdev->dev, "Failed to register media controller");
500 goto err_dec_mem_init;
501 }
502
503 ret = media_device_register(&dev->mdev_dec);
504 if (ret) {
505 dev_err(&pdev->dev, "Failed to register media device");
506 goto err_media_reg;
507 }
508
509 dev_dbg(&pdev->dev, "media registered as /dev/media%d", vfd_dec->minor);
510 }
511
512 mtk_vcodec_dbgfs_init(dev, false);
513 dev_dbg(&pdev->dev, "decoder registered as /dev/video%d", vfd_dec->minor);
514
515 return 0;
516
517 err_media_reg:
518 v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec);
519 err_dec_mem_init:
520 video_unregister_device(vfd_dec);
521 err_reg_cont:
522 if (dev->vdec_pdata->uses_stateless_api)
523 media_device_cleanup(&dev->mdev_dec);
524 destroy_workqueue(dev->decode_workqueue);
525 err_event_workq:
526 v4l2_m2m_release(dev->m2m_dev_dec);
527 err_dec_alloc:
528 v4l2_device_unregister(&dev->v4l2_dev);
529 err_core_workq:
530 if (IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch))
531 destroy_workqueue(dev->core_workqueue);
532 err_res:
533 if (!dev->vdec_pdata->is_subdev_supported)
534 pm_runtime_disable(dev->pm.dev);
535 err_dec_pm:
536 mtk_vcodec_fw_release(dev->fw_handler);
537 return ret;
538 }
539

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-06 23:44:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] media: medkatek: vcodec: support tee decoder

Hi Yunfei,

kernel test robot noticed the following build errors:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Yunfei-Dong/media-medkatek-vcodec-support-tee-decoder/20231206-201843
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20231206081538.17056-21-yunfei.dong%40mediatek.com
patch subject: [PATCH] media: medkatek: vcodec: support tee decoder
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20231207/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:313:11: error: no member named 'is_secure_playback' in 'struct mtk_vcodec_dec_ctx'
313 | if (ctx->is_secure_playback)
| ~~~ ^
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:314:3: error: call to undeclared function 'mtk_vcodec_dec_optee_release'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
314 | mtk_vcodec_dec_optee_release(dev->optee_private);
| ^
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:314:3: note: did you mean 'mtk_vcodec_dec_release'?
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.h:88:6: note: 'mtk_vcodec_dec_release' declared here
88 | void mtk_vcodec_dec_release(struct mtk_vcodec_dec_ctx *ctx);
| ^
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:314:37: error: no member named 'optee_private' in 'struct mtk_vcodec_dec_dev'
314 | mtk_vcodec_dec_optee_release(dev->optee_private);
| ~~~ ^
>> drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:472:8: error: call to undeclared function 'mtk_vcodec_dec_optee_private_init'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
472 | ret = mtk_vcodec_dec_optee_private_init(dev);
| ^
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c:472:8: note: did you mean 'mtk_vcodec_dec_queue_init'?
drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.h:85:5: note: 'mtk_vcodec_dec_queue_init' declared here
85 | int mtk_vcodec_dec_queue_init(void *priv, struct vb2_queue *src_vq,
| ^
4 errors generated.


vim +313 drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c

291
292 static int fops_vcodec_release(struct file *file)
293 {
294 struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
295 struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(file->private_data);
296
297 mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id);
298 mutex_lock(&dev->dev_mutex);
299
300 /*
301 * Call v4l2_m2m_ctx_release before mtk_vcodec_dec_release. First, it
302 * makes sure the worker thread is not running after vdec_if_deinit.
303 * Second, the decoder will be flushed and all the buffers will be
304 * returned in stop_streaming.
305 */
306 v4l2_m2m_ctx_release(ctx->m2m_ctx);
307 mtk_vcodec_dec_release(ctx);
308
309 v4l2_fh_del(&ctx->fh);
310 v4l2_fh_exit(&ctx->fh);
311 v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
312
> 313 if (ctx->is_secure_playback)
> 314 mtk_vcodec_dec_optee_release(dev->optee_private);
315
316 mtk_vcodec_dbgfs_remove(dev, ctx->id);
317 list_del_init(&ctx->list);
318 kfree(ctx);
319 mutex_unlock(&dev->dev_mutex);
320 return 0;
321 }
322
323 static const struct v4l2_file_operations mtk_vcodec_fops = {
324 .owner = THIS_MODULE,
325 .open = fops_vcodec_open,
326 .release = fops_vcodec_release,
327 .poll = v4l2_m2m_fop_poll,
328 .unlocked_ioctl = video_ioctl2,
329 .mmap = v4l2_m2m_fop_mmap,
330 };
331
332 static void mtk_vcodec_dec_get_chip_name(struct mtk_vcodec_dec_dev *vdec_dev)
333 {
334 struct device *dev = &vdec_dev->plat_dev->dev;
335
336 if (of_device_is_compatible(dev->of_node, "mediatek,mt8173-vcodec-dec"))
337 vdec_dev->chip_name = MTK_VDEC_MT8173;
338 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8183-vcodec-dec"))
339 vdec_dev->chip_name = MTK_VDEC_MT8183;
340 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8192-vcodec-dec"))
341 vdec_dev->chip_name = MTK_VDEC_MT8192;
342 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8195-vcodec-dec"))
343 vdec_dev->chip_name = MTK_VDEC_MT8195;
344 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8186-vcodec-dec"))
345 vdec_dev->chip_name = MTK_VDEC_MT8186;
346 else if (of_device_is_compatible(dev->of_node, "mediatek,mt8188-vcodec-dec"))
347 vdec_dev->chip_name = MTK_VDEC_MT8188;
348 else
349 vdec_dev->chip_name = MTK_VDEC_INVAL;
350 }
351
352 static int mtk_vcodec_probe(struct platform_device *pdev)
353 {
354 struct mtk_vcodec_dec_dev *dev;
355 struct video_device *vfd_dec;
356 phandle rproc_phandle;
357 enum mtk_vcodec_fw_type fw_type;
358 int i, ret;
359
360 dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
361 if (!dev)
362 return -ENOMEM;
363
364 INIT_LIST_HEAD(&dev->ctx_list);
365 dev->plat_dev = pdev;
366
367 mtk_vcodec_dec_get_chip_name(dev);
368 if (dev->chip_name == MTK_VDEC_INVAL) {
369 dev_err(&pdev->dev, "Failed to get decoder chip name");
370 return -EINVAL;
371 }
372
373 dev->vdec_pdata = of_device_get_match_data(&pdev->dev);
374 if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
375 &rproc_phandle)) {
376 fw_type = VPU;
377 } else if (!of_property_read_u32(pdev->dev.of_node, "mediatek,scp",
378 &rproc_phandle)) {
379 fw_type = SCP;
380 } else {
381 dev_dbg(&pdev->dev, "Could not get vdec IPI device");
382 return -ENODEV;
383 }
384 dma_set_max_seg_size(&pdev->dev, UINT_MAX);
385
386 dev->fw_handler = mtk_vcodec_fw_select(dev, fw_type, DECODER);
387 if (IS_ERR(dev->fw_handler))
388 return PTR_ERR(dev->fw_handler);
389
390 ret = mtk_vcodec_init_dec_resources(dev);
391 if (ret) {
392 dev_err(&pdev->dev, "Failed to init dec resources");
393 goto err_dec_pm;
394 }
395
396 if (IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch)) {
397 dev->core_workqueue =
398 alloc_ordered_workqueue("core-decoder",
399 WQ_MEM_RECLAIM | WQ_FREEZABLE);
400 if (!dev->core_workqueue) {
401 dev_dbg(&pdev->dev, "Failed to create core workqueue");
402 ret = -EINVAL;
403 goto err_res;
404 }
405 }
406
407 for (i = 0; i < MTK_VDEC_HW_MAX; i++)
408 mutex_init(&dev->dec_mutex[i]);
409 mutex_init(&dev->dev_mutex);
410 spin_lock_init(&dev->irqlock);
411
412 snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
413 "[/MTK_V4L2_VDEC]");
414
415 ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
416 if (ret) {
417 dev_err(&pdev->dev, "v4l2_device_register err=%d", ret);
418 goto err_core_workq;
419 }
420
421 vfd_dec = video_device_alloc();
422 if (!vfd_dec) {
423 dev_err(&pdev->dev, "Failed to allocate video device");
424 ret = -ENOMEM;
425 goto err_dec_alloc;
426 }
427 vfd_dec->fops = &mtk_vcodec_fops;
428 vfd_dec->ioctl_ops = &mtk_vdec_ioctl_ops;
429 vfd_dec->release = video_device_release;
430 vfd_dec->lock = &dev->dev_mutex;
431 vfd_dec->v4l2_dev = &dev->v4l2_dev;
432 vfd_dec->vfl_dir = VFL_DIR_M2M;
433 vfd_dec->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE |
434 V4L2_CAP_STREAMING;
435
436 snprintf(vfd_dec->name, sizeof(vfd_dec->name), "%s",
437 MTK_VCODEC_DEC_NAME);
438 video_set_drvdata(vfd_dec, dev);
439 dev->vfd_dec = vfd_dec;
440 platform_set_drvdata(pdev, dev);
441
442 dev->m2m_dev_dec = v4l2_m2m_init(&mtk_vdec_m2m_ops);
443 if (IS_ERR((__force void *)dev->m2m_dev_dec)) {
444 dev_err(&pdev->dev, "Failed to init mem2mem dec device");
445 ret = PTR_ERR((__force void *)dev->m2m_dev_dec);
446 goto err_dec_alloc;
447 }
448
449 dev->decode_workqueue =
450 alloc_ordered_workqueue(MTK_VCODEC_DEC_NAME,
451 WQ_MEM_RECLAIM | WQ_FREEZABLE);
452 if (!dev->decode_workqueue) {
453 dev_err(&pdev->dev, "Failed to create decode workqueue");
454 ret = -EINVAL;
455 goto err_event_workq;
456 }
457
458 if (dev->vdec_pdata->is_subdev_supported) {
459 ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
460 &pdev->dev);
461 if (ret) {
462 dev_err(&pdev->dev, "Main device of_platform_populate failed.");
463 goto err_reg_cont;
464 }
465 } else {
466 set_bit(MTK_VDEC_CORE, dev->subdev_bitmap);
467 }
468
469 atomic_set(&dev->dec_active_cnt, 0);
470 memset(dev->vdec_racing_info, 0, sizeof(dev->vdec_racing_info));
471 mutex_init(&dev->dec_racing_info_mutex);
> 472 ret = mtk_vcodec_dec_optee_private_init(dev);
473 if (ret) {
474 dev_err(&pdev->dev, "Failed to init svp private.");
475 goto err_reg_cont;
476 }
477
478 ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, -1);
479 if (ret) {
480 dev_err(&pdev->dev, "Failed to register video device");
481 goto err_reg_cont;
482 }
483
484 if (dev->vdec_pdata->uses_stateless_api) {
485 v4l2_disable_ioctl(vfd_dec, VIDIOC_DECODER_CMD);
486 v4l2_disable_ioctl(vfd_dec, VIDIOC_TRY_DECODER_CMD);
487
488 dev->mdev_dec.dev = &pdev->dev;
489 strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME,
490 sizeof(dev->mdev_dec.model));
491
492 media_device_init(&dev->mdev_dec);
493 dev->mdev_dec.ops = &mtk_vcodec_media_ops;
494 dev->v4l2_dev.mdev = &dev->mdev_dec;
495
496 ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, dev->vfd_dec,
497 MEDIA_ENT_F_PROC_VIDEO_DECODER);
498 if (ret) {
499 dev_err(&pdev->dev, "Failed to register media controller");
500 goto err_dec_mem_init;
501 }
502
503 ret = media_device_register(&dev->mdev_dec);
504 if (ret) {
505 dev_err(&pdev->dev, "Failed to register media device");
506 goto err_media_reg;
507 }
508
509 dev_dbg(&pdev->dev, "media registered as /dev/media%d", vfd_dec->minor);
510 }
511
512 mtk_vcodec_dbgfs_init(dev, false);
513 dev_dbg(&pdev->dev, "decoder registered as /dev/video%d", vfd_dec->minor);
514
515 return 0;
516
517 err_media_reg:
518 v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec);
519 err_dec_mem_init:
520 video_unregister_device(vfd_dec);
521 err_reg_cont:
522 if (dev->vdec_pdata->uses_stateless_api)
523 media_device_cleanup(&dev->mdev_dec);
524 destroy_workqueue(dev->decode_workqueue);
525 err_event_workq:
526 v4l2_m2m_release(dev->m2m_dev_dec);
527 err_dec_alloc:
528 v4l2_device_unregister(&dev->v4l2_dev);
529 err_core_workq:
530 if (IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch))
531 destroy_workqueue(dev->core_workqueue);
532 err_res:
533 if (!dev->vdec_pdata->is_subdev_supported)
534 pm_runtime_disable(dev->pm.dev);
535 err_dec_pm:
536 mtk_vcodec_fw_release(dev->fw_handler);
537 return ret;
538 }
539

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-12-11 10:47:18

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3,02/21] v4l2: handle secure memory flags in queue setup

Hi Yunfei, Jeffrey,

Some comments below:

On 06/12/2023 09:15, Yunfei Dong wrote:
> From: Jeffrey Kardatzke <[email protected]>
>
> Validates the secure memory flags when setting up a queue and ensures
> the queue has the proper capability.
>
> Signed-off-by: Jeffrey Kardatzke <[email protected]>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> .../media/common/videobuf2/videobuf2-core.c | 23 +++++++++++++
> .../media/common/videobuf2/videobuf2-v4l2.c | 34 +++++++++++++------
> 2 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 8c1df829745b..09dc030484be 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -813,6 +813,15 @@ static bool verify_coherency_flags(struct vb2_queue *q, bool non_coherent_mem)
> return true;
> }
>
> +static bool verify_secure_mem_flags(struct vb2_queue *q, bool secure_mem)
> +{
> + if (secure_mem != q->secure_mem) {
> + dprintk(q, 1, "secure memory model mismatch\n");
> + return false;
> + }
> + return true;
> +}
> +
> int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> unsigned int flags, unsigned int *count)
> {
> @@ -820,6 +829,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> unsigned int q_num_bufs = vb2_get_num_buffers(q);
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> + bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
> unsigned int i;
> int ret = 0;
>
> @@ -836,6 +846,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> if (*count == 0 || q_num_bufs != 0 ||
> (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) ||
> !verify_coherency_flags(q, non_coherent_mem)) {
> + bool no_previous_buffers = !q->num_buffers;
> +
> /*
> * We already have buffers allocated, so first check if they
> * are not in use and can be freed.
> @@ -854,6 +866,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> __vb2_queue_free(q, q_num_bufs);
> mutex_unlock(&q->mmap_lock);
>
> + /*
> + * Do not allow switching secure buffer mode.
> + */
> + if (!no_previous_buffers && !verify_secure_mem_flags(q, secure_mem))
> + return -EINVAL;
> +

Why is this needed? Here VIDIOC_REQBUFS is called either to just delete
all existing buffers (count == 0), or to delete all existing buffers and
allocate new buffers (count > 0).

Since in both cases all existing buffers are deleted, you are free to choose
whatever new secure mode you want.

> /*
> * In case of REQBUFS(0) return immediately without calling
> * driver's queue_setup() callback and allocating resources.
> @@ -882,6 +900,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> if (ret)
> return ret;
> set_queue_coherency(q, non_coherent_mem);
> + q->secure_mem = secure_mem;
>
> /*
> * Ask the driver how many buffers and planes per buffer it requires.
> @@ -986,6 +1005,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> unsigned int q_num_bufs = vb2_get_num_buffers(q);
> + bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
> bool no_previous_buffers = !q_num_bufs;
> int ret = 0;
>
> @@ -1015,6 +1035,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> return ret;
> q->waiting_for_buffers = !q->is_output;
> set_queue_coherency(q, non_coherent_mem);
> + q->secure_mem = secure_mem;
> } else {
> if (q->memory != memory) {
> dprintk(q, 1, "memory model mismatch\n");
> @@ -1022,6 +1043,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> }
> if (!verify_coherency_flags(q, non_coherent_mem))
> return -EINVAL;
> + if (!verify_secure_mem_flags(q, secure_mem))
> + return -EINVAL;
> }
>
> num_buffers = min(*count, q->max_num_buffers - q_num_bufs);
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 54d572c3b515..0a530830276c 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -686,22 +686,30 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
> *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
> if (q->supports_requests)
> *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
> + if (q->allow_secure_mem && q->io_modes & VB2_DMABUF)
> + *caps |= V4L2_BUF_CAP_SUPPORTS_SECURE_MEM;
> }
>
> -static void validate_memory_flags(struct vb2_queue *q,
> +static bool validate_memory_flags(struct vb2_queue *q,
> int memory,
> u32 *flags)
> {
> + if (*flags & V4L2_MEMORY_FLAG_SECURE &&
> + (!q->allow_secure_mem || memory != V4L2_MEMORY_DMABUF)) {
> + return false;
> + }
> +

This check belongs to videobuf2-core.c and the check should be done
in vb2_core_reqbufs and vb2_core_create_bufs.

So just leave this function as a void.

> if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) {
> /*
> - * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only,
> - * but in order to avoid bugs we zero out all bits.
> + * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only.

Just drop this as well since it adds no useful information anymore.

> */
> - *flags = 0;
> - } else {
> - /* Clear all unknown flags. */
> - *flags &= V4L2_MEMORY_FLAG_NON_COHERENT;
> + *flags &= ~V4L2_MEMORY_FLAG_NON_COHERENT;
> }
> +
> + /* Clear all unknown flags. */
> + *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | V4L2_MEMORY_FLAG_SECURE;

This is still needed here.

> +
> + return true;
> }
>

So the following changes from here...

> int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> @@ -710,7 +718,8 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> u32 flags = req->flags;
>
> fill_buf_caps(q, &req->capabilities);
> - validate_memory_flags(q, req->memory, &flags);
> + if (!validate_memory_flags(q, req->memory, &flags))
> + return -EINVAL;
> req->flags = flags;
> return ret ? ret : vb2_core_reqbufs(q, req->memory,
> req->flags, &req->count);
> @@ -752,7 +761,8 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> unsigned i;
>
> fill_buf_caps(q, &create->capabilities);
> - validate_memory_flags(q, create->memory, &create->flags);
> + if (!validate_memory_flags(q, create->memory, &create->flags))
> + return -EINVAL;
> create->index = vb2_get_num_buffers(q);
> create->max_num_buffers = q->max_num_buffers;
> create->capabilities |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS;
> @@ -1007,7 +1017,8 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
> u32 flags = p->flags;
>
> fill_buf_caps(vdev->queue, &p->capabilities);
> - validate_memory_flags(vdev->queue, p->memory, &flags);
> + if (!validate_memory_flags(vdev->queue, p->memory, &flags))
> + return -EINVAL;
> p->flags = flags;
> if (res)
> return res;
> @@ -1031,7 +1042,8 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
>
> p->index = vdev->queue->num_buffers;
> fill_buf_caps(vdev->queue, &p->capabilities);
> - validate_memory_flags(vdev->queue, p->memory, &p->flags);
> + if (!validate_memory_flags(vdev->queue, p->memory, &p->flags))
> + return -EINVAL;
> /*
> * If count == 0, then just check if memory and type are valid.
> * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.

...to the end should all be dropped since the vb2 core will do the checks.

Regards,

Hans

2023-12-11 10:58:56

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3,03/21] v4l2: verify secure dmabufs are used in secure queue

On 06/12/2023 09:15, Yunfei Dong wrote:
> From: Jeffrey Kardatzke <[email protected]>
>
> Verfies in the dmabuf implementations that if the secure memory flag is

Verfies -> Verifies

> set for a queue that the dmabuf submitted to the queue is unmappable.
>
> Signed-off-by: Jeffrey Kardatzke <[email protected]>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> drivers/media/common/videobuf2/videobuf2-dma-contig.c | 6 ++++++
> drivers/media/common/videobuf2/videobuf2-dma-sg.c | 6 ++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 3d4fd4ef5310..ad58ef8dc231 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -710,6 +710,12 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
> return -EINVAL;
> }
>
> + /* verify the dmabuf is secure if we are in secure mode */
> + if (buf->vb->vb2_queue->secure_mem && sg_page(sgt->sgl)) {

This needs a bit more explanation. I guess that for secure memory
sg_page returns NULL?

> + pr_err("secure queue requires secure dma_buf");
> + return -EINVAL;
> + }
> +
> /* checking if dmabuf is big enough to store contiguous chunk */
> contig_size = vb2_dc_get_contiguous_size(sgt);
> if (contig_size < buf->size) {
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 28f3fdfe23a2..55428c73c380 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -564,6 +564,12 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv)
> return -EINVAL;
> }
>
> + /* verify the dmabuf is secure if we are in secure mode */
> + if (buf->vb->vb2_queue->secure_mem && !sg_dma_secure(sgt->sgl)) {

I can't find the sg_dma_secure function. I suspect this patch series
depends on another series?

> + pr_err("secure queue requires secure dma_buf");
> + return -EINVAL;
> + }
> +
> buf->dma_sgt = sgt;
> buf->vaddr = NULL;
>

Regards,

Hans

2023-12-11 11:06:25

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3,04/21] v4l: add documentation for secure memory flag

On 06/12/2023 09:15, Yunfei Dong wrote:
> From: Jeffrey Kardatzke <[email protected]>
>
> Adds documentation for V4L2_MEMORY_FLAG_SECURE.
>
> Signed-off-by: Jeffrey Kardatzke <[email protected]>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> index 52bbee81c080..a5a7d1c72d53 100644
> --- a/Documentation/userspace-api/media/v4l/buffer.rst
> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> @@ -696,7 +696,7 @@ enum v4l2_memory
>
> .. _memory-flags:
>
> -Memory Consistency Flags
> +Memory Flags
> ------------------------
>
> .. raw:: latex
> @@ -728,6 +728,12 @@ Memory Consistency Flags
> only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
> queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
> <V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
> + * .. _`V4L2-MEMORY-FLAG-SECURE`:
> +
> + - ``V4L2_MEMORY_FLAG_SECURE``
> + - 0x00000002
> + - DMA bufs passed into the queue will be validated to ensure they were
> + allocated from a secure dma-heap.

Hmm, that needs a bit more work. How about:

- The queued buffers are expected to be in secure memory. If not, an error will be
returned. This flag can only be used with ``V4L2_MEMORY_DMABUF``. Typically
secure buffers are allocated using a secure dma-heap. This flag can only be
specified if the ``V4L2_BUF_CAP_SUPPORTS_SECURE_MEM`` is set.

In addition, the title of this table is currently "Memory Consistency Flags": that
should be renamed to "Memory Flags".

Regards,

Hans

>
> .. raw:: latex
>

2024-01-04 20:05:32

by Jeffrey Kardatzke

[permalink] [raw]
Subject: Re: [PATCH v3,02/21] v4l2: handle secure memory flags in queue setup

Yunfei,

Can you please integrate these changes into the patch?

Thanks,
Jeff

On Mon, Dec 11, 2023 at 2:45 AM Hans Verkuil <[email protected]> wrote:
>
> Hi Yunfei, Jeffrey,
>
> Some comments below:
>
> On 06/12/2023 09:15, Yunfei Dong wrote:
> > From: Jeffrey Kardatzke <[email protected]>
> >
> > Validates the secure memory flags when setting up a queue and ensures
> > the queue has the proper capability.
> >
> > Signed-off-by: Jeffrey Kardatzke <[email protected]>
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > .../media/common/videobuf2/videobuf2-core.c | 23 +++++++++++++
> > .../media/common/videobuf2/videobuf2-v4l2.c | 34 +++++++++++++------
> > 2 files changed, 46 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index 8c1df829745b..09dc030484be 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -813,6 +813,15 @@ static bool verify_coherency_flags(struct vb2_queue *q, bool non_coherent_mem)
> > return true;
> > }
> >
> > +static bool verify_secure_mem_flags(struct vb2_queue *q, bool secure_mem)
> > +{
> > + if (secure_mem != q->secure_mem) {
> > + dprintk(q, 1, "secure memory model mismatch\n");
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> > unsigned int flags, unsigned int *count)
> > {
> > @@ -820,6 +829,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> > unsigned int q_num_bufs = vb2_get_num_buffers(q);
> > unsigned plane_sizes[VB2_MAX_PLANES] = { };
> > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> > + bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
> > unsigned int i;
> > int ret = 0;
> >
> > @@ -836,6 +846,8 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> > if (*count == 0 || q_num_bufs != 0 ||
> > (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) ||
> > !verify_coherency_flags(q, non_coherent_mem)) {
> > + bool no_previous_buffers = !q->num_buffers;
> > +
> > /*
> > * We already have buffers allocated, so first check if they
> > * are not in use and can be freed.
> > @@ -854,6 +866,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> > __vb2_queue_free(q, q_num_bufs);
> > mutex_unlock(&q->mmap_lock);
> >
> > + /*
> > + * Do not allow switching secure buffer mode.
> > + */
> > + if (!no_previous_buffers && !verify_secure_mem_flags(q, secure_mem))
> > + return -EINVAL;
> > +
>
> Why is this needed? Here VIDIOC_REQBUFS is called either to just delete
> all existing buffers (count == 0), or to delete all existing buffers and
> allocate new buffers (count > 0).
>
> Since in both cases all existing buffers are deleted, you are free to choose
> whatever new secure mode you want.
>
> > /*
> > * In case of REQBUFS(0) return immediately without calling
> > * driver's queue_setup() callback and allocating resources.
> > @@ -882,6 +900,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> > if (ret)
> > return ret;
> > set_queue_coherency(q, non_coherent_mem);
> > + q->secure_mem = secure_mem;
> >
> > /*
> > * Ask the driver how many buffers and planes per buffer it requires.
> > @@ -986,6 +1005,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> > unsigned plane_sizes[VB2_MAX_PLANES] = { };
> > bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT;
> > unsigned int q_num_bufs = vb2_get_num_buffers(q);
> > + bool secure_mem = flags & V4L2_MEMORY_FLAG_SECURE;
> > bool no_previous_buffers = !q_num_bufs;
> > int ret = 0;
> >
> > @@ -1015,6 +1035,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> > return ret;
> > q->waiting_for_buffers = !q->is_output;
> > set_queue_coherency(q, non_coherent_mem);
> > + q->secure_mem = secure_mem;
> > } else {
> > if (q->memory != memory) {
> > dprintk(q, 1, "memory model mismatch\n");
> > @@ -1022,6 +1043,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
> > }
> > if (!verify_coherency_flags(q, non_coherent_mem))
> > return -EINVAL;
> > + if (!verify_secure_mem_flags(q, secure_mem))
> > + return -EINVAL;
> > }
> >
> > num_buffers = min(*count, q->max_num_buffers - q_num_bufs);
> > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > index 54d572c3b515..0a530830276c 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -686,22 +686,30 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps)
> > *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS;
> > if (q->supports_requests)
> > *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS;
> > + if (q->allow_secure_mem && q->io_modes & VB2_DMABUF)
> > + *caps |= V4L2_BUF_CAP_SUPPORTS_SECURE_MEM;
> > }
> >
> > -static void validate_memory_flags(struct vb2_queue *q,
> > +static bool validate_memory_flags(struct vb2_queue *q,
> > int memory,
> > u32 *flags)
> > {
> > + if (*flags & V4L2_MEMORY_FLAG_SECURE &&
> > + (!q->allow_secure_mem || memory != V4L2_MEMORY_DMABUF)) {
> > + return false;
> > + }
> > +
>
> This check belongs to videobuf2-core.c and the check should be done
> in vb2_core_reqbufs and vb2_core_create_bufs.
>
> So just leave this function as a void.
>
> > if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) {
> > /*
> > - * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only,
> > - * but in order to avoid bugs we zero out all bits.
> > + * This needs to clear V4L2_MEMORY_FLAG_NON_COHERENT only.
>
> Just drop this as well since it adds no useful information anymore.
>
> > */
> > - *flags = 0;
> > - } else {
> > - /* Clear all unknown flags. */
> > - *flags &= V4L2_MEMORY_FLAG_NON_COHERENT;
> > + *flags &= ~V4L2_MEMORY_FLAG_NON_COHERENT;
> > }
> > +
> > + /* Clear all unknown flags. */
> > + *flags &= V4L2_MEMORY_FLAG_NON_COHERENT | V4L2_MEMORY_FLAG_SECURE;
>
> This is still needed here.
>
> > +
> > + return true;
> > }
> >
>
> So the following changes from here...
>
> > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> > @@ -710,7 +718,8 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
> > u32 flags = req->flags;
> >
> > fill_buf_caps(q, &req->capabilities);
> > - validate_memory_flags(q, req->memory, &flags);
> > + if (!validate_memory_flags(q, req->memory, &flags))
> > + return -EINVAL;
> > req->flags = flags;
> > return ret ? ret : vb2_core_reqbufs(q, req->memory,
> > req->flags, &req->count);
> > @@ -752,7 +761,8 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> > unsigned i;
> >
> > fill_buf_caps(q, &create->capabilities);
> > - validate_memory_flags(q, create->memory, &create->flags);
> > + if (!validate_memory_flags(q, create->memory, &create->flags))
> > + return -EINVAL;
> > create->index = vb2_get_num_buffers(q);
> > create->max_num_buffers = q->max_num_buffers;
> > create->capabilities |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS;
> > @@ -1007,7 +1017,8 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv,
> > u32 flags = p->flags;
> >
> > fill_buf_caps(vdev->queue, &p->capabilities);
> > - validate_memory_flags(vdev->queue, p->memory, &flags);
> > + if (!validate_memory_flags(vdev->queue, p->memory, &flags))
> > + return -EINVAL;
> > p->flags = flags;
> > if (res)
> > return res;
> > @@ -1031,7 +1042,8 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv,
> >
> > p->index = vdev->queue->num_buffers;
> > fill_buf_caps(vdev->queue, &p->capabilities);
> > - validate_memory_flags(vdev->queue, p->memory, &p->flags);
> > + if (!validate_memory_flags(vdev->queue, p->memory, &p->flags))
> > + return -EINVAL;
> > /*
> > * If count == 0, then just check if memory and type are valid.
> > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0.
>
> ...to the end should all be dropped since the vb2 core will do the checks.
>
> Regards,
>
> Hans

2024-01-04 20:06:01

by Jeffrey Kardatzke

[permalink] [raw]
Subject: Re: [PATCH v3,04/21] v4l: add documentation for secure memory flag

On Mon, Dec 11, 2023 at 3:05 AM Hans Verkuil <[email protected]> wrote:
>
> On 06/12/2023 09:15, Yunfei Dong wrote:
> > From: Jeffrey Kardatzke <[email protected]>
> >
> > Adds documentation for V4L2_MEMORY_FLAG_SECURE.
> >
> > Signed-off-by: Jeffrey Kardatzke <[email protected]>
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> > index 52bbee81c080..a5a7d1c72d53 100644
> > --- a/Documentation/userspace-api/media/v4l/buffer.rst
> > +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> > @@ -696,7 +696,7 @@ enum v4l2_memory
> >
> > .. _memory-flags:
> >
> > -Memory Consistency Flags
> > +Memory Flags
> > ------------------------
> >
> > .. raw:: latex
> > @@ -728,6 +728,12 @@ Memory Consistency Flags
> > only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
> > queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
> > <V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
> > + * .. _`V4L2-MEMORY-FLAG-SECURE`:
> > +
> > + - ``V4L2_MEMORY_FLAG_SECURE``
> > + - 0x00000002
> > + - DMA bufs passed into the queue will be validated to ensure they were
> > + allocated from a secure dma-heap.
>
> Hmm, that needs a bit more work. How about:
>
> - The queued buffers are expected to be in secure memory. If not, an error will be
> returned. This flag can only be used with ``V4L2_MEMORY_DMABUF``. Typically
> secure buffers are allocated using a secure dma-heap. This flag can only be
> specified if the ``V4L2_BUF_CAP_SUPPORTS_SECURE_MEM`` is set.
>

Thanks Hans. Yunfei, can you integrate this change into the patch please?

> In addition, the title of this table is currently "Memory Consistency Flags": that
> should be renamed to "Memory Flags".

Hans, the patch is already renaming the table as you suggested. :)
(unless there's some other spot I'm missing)
>
> Regards,
>
> Hans
>
> >
> > .. raw:: latex
> >
>

2024-01-04 20:07:57

by Jeffrey Kardatzke

[permalink] [raw]
Subject: Re: [PATCH v3,03/21] v4l2: verify secure dmabufs are used in secure queue

On Mon, Dec 11, 2023 at 2:58 AM Hans Verkuil <[email protected]> wrote:
>
> On 06/12/2023 09:15, Yunfei Dong wrote:
> > From: Jeffrey Kardatzke <[email protected]>
> >
> > Verfies in the dmabuf implementations that if the secure memory flag is
>
> Verfies -> Verifies

Thanks. Yunfei, change that please.
>
> > set for a queue that the dmabuf submitted to the queue is unmappable.
> >
> > Signed-off-by: Jeffrey Kardatzke <[email protected]>
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > drivers/media/common/videobuf2/videobuf2-dma-contig.c | 6 ++++++
> > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 6 ++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > index 3d4fd4ef5310..ad58ef8dc231 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > @@ -710,6 +710,12 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
> > return -EINVAL;
> > }
> >
> > + /* verify the dmabuf is secure if we are in secure mode */
> > + if (buf->vb->vb2_queue->secure_mem && sg_page(sgt->sgl)) {
>
> This needs a bit more explanation. I guess that for secure memory
> sg_page returns NULL?

How about if we change it to:

/* verify the dmabuf is secure if we are in secure mode, this is done
by validating there is no page entry for the dmabuf */

>
> > + pr_err("secure queue requires secure dma_buf");
> > + return -EINVAL;
> > + }
> > +
> > /* checking if dmabuf is big enough to store contiguous chunk */
> > contig_size = vb2_dc_get_contiguous_size(sgt);
> > if (contig_size < buf->size) {
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index 28f3fdfe23a2..55428c73c380 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -564,6 +564,12 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv)
> > return -EINVAL;
> > }
> >
> > + /* verify the dmabuf is secure if we are in secure mode */
> > + if (buf->vb->vb2_queue->secure_mem && !sg_dma_secure(sgt->sgl)) {
>
> I can't find the sg_dma_secure function. I suspect this patch series
> depends on another series?

That was an oversight, it should be the same as in
videobuf2-dma-contig.c. Yunfei, can you change this to match what's in
videobuf2-dma-contig.c after the comment is reworded?
>
> > + pr_err("secure queue requires secure dma_buf");
> > + return -EINVAL;
> > + }
> > +
> > buf->dma_sgt = sgt;
> > buf->vaddr = NULL;
> >
>
> Regards,
>
> Hans

2024-01-08 08:30:17

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v3,04/21] v4l: add documentation for secure memory flag

On 04/01/2024 21:05, Jeffrey Kardatzke wrote:
> On Mon, Dec 11, 2023 at 3:05 AM Hans Verkuil <[email protected]> wrote:
>>
>> On 06/12/2023 09:15, Yunfei Dong wrote:
>>> From: Jeffrey Kardatzke <[email protected]>
>>>
>>> Adds documentation for V4L2_MEMORY_FLAG_SECURE.
>>>
>>> Signed-off-by: Jeffrey Kardatzke <[email protected]>
>>> Signed-off-by: Yunfei Dong <[email protected]>
>>> ---
>>> Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
>>> index 52bbee81c080..a5a7d1c72d53 100644
>>> --- a/Documentation/userspace-api/media/v4l/buffer.rst
>>> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
>>> @@ -696,7 +696,7 @@ enum v4l2_memory
>>>
>>> .. _memory-flags:
>>>
>>> -Memory Consistency Flags
>>> +Memory Flags
>>> ------------------------
>>>
>>> .. raw:: latex
>>> @@ -728,6 +728,12 @@ Memory Consistency Flags
>>> only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
>>> queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
>>> <V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
>>> + * .. _`V4L2-MEMORY-FLAG-SECURE`:
>>> +
>>> + - ``V4L2_MEMORY_FLAG_SECURE``
>>> + - 0x00000002
>>> + - DMA bufs passed into the queue will be validated to ensure they were
>>> + allocated from a secure dma-heap.
>>
>> Hmm, that needs a bit more work. How about:
>>
>> - The queued buffers are expected to be in secure memory. If not, an error will be
>> returned. This flag can only be used with ``V4L2_MEMORY_DMABUF``. Typically
>> secure buffers are allocated using a secure dma-heap. This flag can only be
>> specified if the ``V4L2_BUF_CAP_SUPPORTS_SECURE_MEM`` is set.
>>
>
> Thanks Hans. Yunfei, can you integrate this change into the patch please?
>
>> In addition, the title of this table is currently "Memory Consistency Flags": that
>> should be renamed to "Memory Flags".
>
> Hans, the patch is already renaming the table as you suggested. :)
> (unless there's some other spot I'm missing)

Sorry for the noise, I missed that change.

Regards,

Hans

>>
>> Regards,
>>
>> Hans
>>
>>>
>>> .. raw:: latex
>>>
>>


2024-01-17 20:04:15

by Nicolas Dufresne

[permalink] [raw]
Subject: Re: [PATCH v3,04/21] v4l: add documentation for secure memory flag

Hi,

Le mercredi 06 décembre 2023 à 16:15 +0800, Yunfei Dong a écrit :
> From: Jeffrey Kardatzke <[email protected]>
>
> Adds documentation for V4L2_MEMORY_FLAG_SECURE.

As I noticed from DMA Heap discussions, shall this also be renamed SECURE ->
RESTRICTED ?

regards,
Nicolas

>
> Signed-off-by: Jeffrey Kardatzke <[email protected]>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> index 52bbee81c080..a5a7d1c72d53 100644
> --- a/Documentation/userspace-api/media/v4l/buffer.rst
> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> @@ -696,7 +696,7 @@ enum v4l2_memory
>
> .. _memory-flags:
>
> -Memory Consistency Flags
> +Memory Flags
> ------------------------
>
> .. raw:: latex
> @@ -728,6 +728,12 @@ Memory Consistency Flags
> only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
> queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
> <V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
> + * .. _`V4L2-MEMORY-FLAG-SECURE`:
> +
> + - ``V4L2_MEMORY_FLAG_SECURE``
> + - 0x00000002
> + - DMA bufs passed into the queue will be validated to ensure they were
> + allocated from a secure dma-heap.
>
> .. raw:: latex
>