diff mbox series

[FFmpeg-devel] hwcontext_drm: issue DMA_BUF_IOCTL_SYNC when mapping FDs

Message ID MMagQ2w--3-2@lynne.ee
State Accepted
Commit 993d8d94296e72cf5a1185fac573ba3c6ce3113d
Headers show
Series [FFmpeg-devel] hwcontext_drm: issue DMA_BUF_IOCTL_SYNC when mapping FDs
Related show

Checks

Context Check Description
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Lynne Nov. 20, 2020, 5:26 p.m. UTC
This improves performance and helps a little when given FDs without 
any synchronization fences.

Patch attached.
Subject: [PATCH] hwcontext_drm: issue DMA_BUF_IOCTL_SYNC when mapping FDs

This improves performance and helps a little when given FDs without
any synchronization fences.
---
 libavutil/hwcontext_drm.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

Comments

Mark Thompson Nov. 21, 2020, 6:42 p.m. UTC | #1
On 20/11/2020 17:26, Lynne wrote:
> This improves performance and helps a little when given FDs without
> any synchronization fences.
> 
> Patch attached.
>  > From b4b0b8038bef08cf3dae9daa78eca3a675b67f89 Mon Sep 17 00:00:00 2001
 > From: Lynne <dev@lynne.ee>
 > Date: Fri, 20 Nov 2020 18:23:42 +0100
 > Subject: [PATCH] hwcontext_drm: issue DMA_BUF_IOCTL_SYNC when mapping FDs
 >
 > This improves performance and helps a little when given FDs without
 > any synchronization fences.
 > ---
 >  libavutil/hwcontext_drm.c | 36 +++++++++++++++++++++++++++++++-----
 >  1 file changed, 31 insertions(+), 5 deletions(-)
 >
 > diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
 > index 32cbde82eb..ceacf683a0 100644
 > --- a/libavutil/hwcontext_drm.c
 > +++ b/libavutil/hwcontext_drm.c
 > @@ -19,6 +19,8 @@
 >  #include <fcntl.h>
 >  #include <sys/mman.h>
 >  #include <unistd.h>
 > +#include <linux/dma-buf.h>

Is it possible that adding raw ioctl() calls will cause problems on non-Linux platforms with libdrm, like FreeBSD?  (Maybe they implement the same ioctl()s, I don't know.)

 > +#include <sys/ioctl.h>
 >
 >  #include <drm.h>
 >  #include <xf86drm.h>
 > @@ -85,6 +87,8 @@ static int drm_get_buffer(AVHWFramesContext *hwfc, AVFrame *frame)
 >  typedef struct DRMMapping {
 >      // Address and length of each mmap()ed region.
 >      int nb_regions;
 > +    int sync_flags;
 > +    int object[AV_DRM_MAX_PLANES];
 >      void *address[AV_DRM_MAX_PLANES];
 >      size_t length[AV_DRM_MAX_PLANES];
 >  } DRMMapping;
 > @@ -93,10 +97,16 @@ static void drm_unmap_frame(AVHWFramesContext *hwfc,
 >                              HWMapDescriptor *hwmap)
 >  {
 >      DRMMapping *map = hwmap->priv;
 > -    int i;
 > -
 > -    for (i = 0; i < map->nb_regions; i++)
 > +    struct dma_buf_sync sync = { .flags = DMA_BUF_SYNC_END | map->sync_flags };
 > +    int i, ret;
 > +
 > +    for (i = 0; i < map->nb_regions; i++) {
 > +        ret = ioctl(map->object[i], DMA_BUF_IOCTL_SYNC, &sync);
 > +        if (ret)
 > +            av_log(hwfc, AV_LOG_ERROR, "Failed to issue ioctl sync to DRM object "
 > +                   "%d: %d.\n", map->object[i], errno);
 >          munmap(map->address[i], map->length[i]);
 > +    }
 >
 >      av_free(map);
 >  }
 > @@ -105,6 +115,7 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
 >                           AVFrame *dst, const AVFrame *src, int flags)
 >  {
 >      const AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor*)src->data[0];
 > +    struct dma_buf_sync sync_start = { 0 };
 >      DRMMapping *map;
 >      int err, i, p, plane;
 >      int mmap_prot;
 > @@ -115,10 +126,16 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
 >          return AVERROR(ENOMEM);
 >
 >      mmap_prot = 0;
 > -    if (flags & AV_HWFRAME_MAP_READ)
 > +    if (flags & AV_HWFRAME_MAP_READ) {
 >          mmap_prot |= PROT_READ;
 > -    if (flags & AV_HWFRAME_MAP_WRITE)
 > +        map->sync_flags |= DMA_BUF_SYNC_READ;
 > +    }
 > +    if (flags & AV_HWFRAME_MAP_WRITE) {
 >          mmap_prot |= PROT_WRITE;
 > +        map->sync_flags |= DMA_BUF_SYNC_WRITE;
 > +    }
 > +
 > +    sync_start.flags = DMA_BUF_SYNC_START | map->sync_flags;
 >
 >      av_assert0(desc->nb_objects <= AV_DRM_MAX_PLANES);
 >      for (i = 0; i < desc->nb_objects; i++) {
 > @@ -133,6 +150,15 @@ static int drm_map_frame(AVHWFramesContext *hwfc,
 >
 >          map->address[i] = addr;
 >          map->length[i]  = desc->objects[i].size;
 > +        map->object[i] = desc->objects[i].fd;
 > +
 > +        err = ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
 > +        if (err) {
 > +            err = AVERROR(errno);
 > +            av_log(hwfc, AV_LOG_ERROR, "Failed to issue ioctl sync to DRM object "
 > +                   "%d: %d.\n", desc->objects[i].fd, errno);
 > +            goto fail;
 > +        }
 >      }
 >      map->nb_regions = i;
 >
 > --
 > 2.29.2
 >

Patch LGTM if we can check the compatibility.

Thanks,

- Mark
Lynne Nov. 25, 2020, 10:20 p.m. UTC | #2
Nov 20, 2020, 18:26 by dev@lynne.ee:

> This improves performance and helps a little when given FDs without 
> any synchronization fences.
>
> Patch attached.
>

Pushed.
diff mbox series

Patch

diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
index 32cbde82eb..ceacf683a0 100644
--- a/libavutil/hwcontext_drm.c
+++ b/libavutil/hwcontext_drm.c
@@ -19,6 +19,8 @@ 
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <unistd.h>
+#include <linux/dma-buf.h>
+#include <sys/ioctl.h>
 
 #include <drm.h>
 #include <xf86drm.h>
@@ -85,6 +87,8 @@  static int drm_get_buffer(AVHWFramesContext *hwfc, AVFrame *frame)
 typedef struct DRMMapping {
     // Address and length of each mmap()ed region.
     int nb_regions;
+    int sync_flags;
+    int object[AV_DRM_MAX_PLANES];
     void *address[AV_DRM_MAX_PLANES];
     size_t length[AV_DRM_MAX_PLANES];
 } DRMMapping;
@@ -93,10 +97,16 @@  static void drm_unmap_frame(AVHWFramesContext *hwfc,
                             HWMapDescriptor *hwmap)
 {
     DRMMapping *map = hwmap->priv;
-    int i;
-
-    for (i = 0; i < map->nb_regions; i++)
+    struct dma_buf_sync sync = { .flags = DMA_BUF_SYNC_END | map->sync_flags };
+    int i, ret;
+
+    for (i = 0; i < map->nb_regions; i++) {
+        ret = ioctl(map->object[i], DMA_BUF_IOCTL_SYNC, &sync);
+        if (ret)
+            av_log(hwfc, AV_LOG_ERROR, "Failed to issue ioctl sync to DRM object "
+                   "%d: %d.\n", map->object[i], errno);
         munmap(map->address[i], map->length[i]);
+    }
 
     av_free(map);
 }
@@ -105,6 +115,7 @@  static int drm_map_frame(AVHWFramesContext *hwfc,
                          AVFrame *dst, const AVFrame *src, int flags)
 {
     const AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor*)src->data[0];
+    struct dma_buf_sync sync_start = { 0 };
     DRMMapping *map;
     int err, i, p, plane;
     int mmap_prot;
@@ -115,10 +126,16 @@  static int drm_map_frame(AVHWFramesContext *hwfc,
         return AVERROR(ENOMEM);
 
     mmap_prot = 0;
-    if (flags & AV_HWFRAME_MAP_READ)
+    if (flags & AV_HWFRAME_MAP_READ) {
         mmap_prot |= PROT_READ;
-    if (flags & AV_HWFRAME_MAP_WRITE)
+        map->sync_flags |= DMA_BUF_SYNC_READ;
+    }
+    if (flags & AV_HWFRAME_MAP_WRITE) {
         mmap_prot |= PROT_WRITE;
+        map->sync_flags |= DMA_BUF_SYNC_WRITE;
+    }
+
+    sync_start.flags = DMA_BUF_SYNC_START | map->sync_flags;
 
     av_assert0(desc->nb_objects <= AV_DRM_MAX_PLANES);
     for (i = 0; i < desc->nb_objects; i++) {
@@ -133,6 +150,15 @@  static int drm_map_frame(AVHWFramesContext *hwfc,
 
         map->address[i] = addr;
         map->length[i]  = desc->objects[i].size;
+        map->object[i] = desc->objects[i].fd;
+
+        err = ioctl(desc->objects[i].fd, DMA_BUF_IOCTL_SYNC, &sync_start);
+        if (err) {
+            err = AVERROR(errno);
+            av_log(hwfc, AV_LOG_ERROR, "Failed to issue ioctl sync to DRM object "
+                   "%d: %d.\n", desc->objects[i].fd, errno);
+            goto fail;
+        }
     }
     map->nb_regions = i;