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

[Mark Thompson]

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 at 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