Hey Marek and others,
just wanted to point out that I've also played around with Seung-Woo' patch for
a while. However this patch alone is very much incomplete.
In particular this is missing:
- At least v5 MFC hw needs source buffers to be also writable, so dma mapping
needs to be setup bidirectional.
- Like with mmap, all buffers need to be setup before decoding can begin. This
is due to how MFC hw gets initialized. dmabufs that are added later are not
going to be used before MFC hw isn't reinitialized.
- Removing dmabufs, or replacing them seems to be impossible with the current
code architecture.
I had extended samsung-utils with some C++ app to test stuff when I was looking
into this. You can find the code here:
https://github.com/tobiasjakobi/samsung-utils/tree/devel/v4l2-mfc-drm-direct
Now here is what happens. I allocate N buffer objects in DRM land to be used as
destination for the MFC decoder. The BOs are exported, so that I can then use
them in V4L2 space. I have to queue n (with n < N) buffers before I can start
the MFC engine.
If I do start the engine at that point (n buffers queued), I soon get an IOMMU
pagefault. I need to queue all N buffers before anything works at all. Queueing
a buffer the first time also registers it, and this has to happen before the MFC
hw is initialized.
In particular I can't just allocate more buffers from DRM and use them here
_after_ decoding has started.
To me it looks like the MFC code was never written with dmabuf in mind. It's
centered around a static memory setup that is fixed before decoding begins.
Anyway, just my 2 cents :)
- Tobias
Marek Szyprowski wrote:
> From: Seung-Woo Kim
>
> There is memory constraint for the buffers in V5 of the MFC hardware, but
> when IOMMU is used, then this constraint is meaningless. Other version of
> the MFC hardware don't have such limitations. So in such cases the driver
> is able to use buffers placed anywhere in the system memory, thus USERPTR
> and DMABUF operation modes can be also enabled.
>
> This patch also removes USERPTR operation mode from encoder node, as it
> doesn't work with v5 MFC hardware without IOMMU being enabled.
>
> Signed-off-by: Seung-Woo Kim
> [mszyprow: adapted to v4.14 code base and updated commit message]
> Signed-off-by: Marek Szyprowski
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc.c | 14 --
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 73
>
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 24 ++---
> 3 files changed, 89 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index cf68aed59e0d..f975523dc040 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -754,6 +754,7 @@ static int s5p_mfc_open(struct file *file)
> struct s5p_mfc_dev *dev = video_drvdata(file);
> struct s5p_mfc_ctx *ctx = NULL;
> struct vb2_queue *q;
> + unsigned int io_modes;
> int ret = 0;
>
> mfc_debug_enter();
> @@ -839,16 +840,21 @@ static int s5p_mfc_open(struct file *file)
> if (ret)
> goto err_init_hw;
> }
> +
> + io_modes = VB2_MMAP;
> + if (exynos_is_iommu_available(&dev->plat_dev->dev) || !IS_TWOPORT(dev))
> + io_modes |= VB2_USERPTR | VB2_DMABUF;
> +
> /* Init videobuf2 queue for CAPTURE */
> q = &ctx->vq_dst;
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> q->drv_priv = &ctx->fh;
> q->lock = &dev->mfc_mutex;
> if (vdev == dev->vfd_dec) {
> - q->io_modes = VB2_MMAP;
> + q->io_modes = io_modes;
> q->ops = get_dec_queue_ops();
> } else if (vdev == dev->vfd_enc) {
> - q->io_modes = VB2_MMAP | VB2_USERPTR;
> + q->io_modes = io_modes;
> q->ops = get_enc_queue_ops();
> } else {
> ret = -ENOENT;
> @@ -872,10 +878,10 @@ static int s5p_mfc_open(struct file *file)
> q->drv_priv = &ctx->fh;
> q->lock = &dev->mfc_mutex;
> if (vdev == dev->vfd_dec) {
> - q->io_modes = VB2_MMAP;
> + q->io_modes = io_modes;
> q->ops = get_dec_queue_ops();
> } else if (vdev == dev->vfd_enc) {
> - q->io_modes = VB2_MMAP | VB2_USERPTR;
> + q->io_modes = io_modes;
> q->ops = get_enc_queue_ops();
> } else {
> ret = -ENOENT;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index 8937b0af7cb3..efe65fce4880 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -546,14 +546,27 @@ static int reqbufs_capture(struct s5p_mfc_dev *dev,
> struct s5p_mfc_ctx *ctx,
> goto out;
> }
>
> - WARN_ON(ctx->dst_bufs_cnt != ctx->total_dpb_cou
