Hi Philipp,
2017-03-08 Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>:
> Loosely based on commit f0a42bb5423a ("drm/msm: submit support for
> in-fences"). Unfortunately, struct drm_etnaviv_gem_submit doesn't have
> a flags field yet, so we have to extend the structure and trust that
> drm_ioctl will clear the flags for us if an older userspace only submits
> part of the struct.
>
> Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/etnaviv/Kconfig | 1 +
> drivers/gpu/drm/etnaviv/etnaviv_gem.h | 1 +
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 34 +++++++++++++++++++++++++++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 +++-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +-
> include/uapi/drm/etnaviv_drm.h | 6 +++++
> 6 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index cc1731c5289c2..71cee4e9fefbb 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -5,6 +5,7 @@ config DRM_ETNAVIV
> depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
> depends on MMU
> select SHMEM
> + select SYNC_FILE
> select TMPFS
> select IOMMU_API
> select IOMMU_SUPPORT
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index e63ff116a3b3d..120410d67eb5b 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -107,6 +107,7 @@ struct etnaviv_gem_submit {
> u32 fence;
> unsigned int nr_bos;
> struct etnaviv_gem_submit_bo bos[0];
> + u32 flags;
> };
>
> int etnaviv_gem_wait_bo(struct etnaviv_gpu *gpu, struct drm_gem_object *obj,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 726090d7a6ace..e67d83eac22dc 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -15,6 +15,7 @@
> */
>
> #include <linux/reservation.h>
> +#include <linux/sync_file.h>
> #include "etnaviv_cmdbuf.h"
> #include "etnaviv_drv.h"
> #include "etnaviv_gpu.h"
> @@ -169,8 +170,10 @@ static int submit_fence_sync(const struct etnaviv_gem_submit *submit)
> for (i = 0; i < submit->nr_bos; i++) {
> struct etnaviv_gem_object *etnaviv_obj = submit->bos[i].obj;
> bool write = submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE;
> + bool explicit = !(submit->flags & ETNA_SUBMIT_NO_IMPLICIT);
>
> - ret = etnaviv_gpu_fence_sync_obj(etnaviv_obj, context, write);
> + ret = etnaviv_gpu_fence_sync_obj(etnaviv_obj, context, write,
> + explicit);
> if (ret)
> break;
> }
> @@ -303,6 +306,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> struct etnaviv_gem_submit *submit;
> struct etnaviv_cmdbuf *cmdbuf;
> struct etnaviv_gpu *gpu;
> + struct dma_fence *in_fence = NULL;
> void *stream;
> int ret;
>
> @@ -326,6 +330,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> return -EINVAL;
> }
>
> + if (args->flags & ~ETNA_SUBMIT_FLAGS) {
> + DRM_ERROR("invalid flags: 0x%x\n", args->flags);
> + return -EINVAL;
> + }
> +
> /*
> * Copy the command submission and bo array to kernel space in
> * one go, and do this outside of any locks.
> @@ -371,6 +380,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> goto err_submit_cmds;
> }
>
> + submit->flags = args->flags;
> +
> ret = submit_lookup_objects(submit, file, bos, args->nr_bos);
> if (ret)
> goto err_submit_objects;
> @@ -385,6 +396,25 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> goto err_submit_objects;
> }
>
> + if (args->flags & ETNA_SUBMIT_FENCE_FD_IN) {
> + in_fence = sync_file_get_fence(args->fence_fd);
> + if (!in_fence) {
> + ret = -EINVAL;
> + goto err_submit_objects;
> + }
> +
> + /* TODO if we get an array-fence due to userspace merging
> + * multiple fences, we need a way to determine if all the
> + * backing fences are from our own context..
> + */
All GPU drivers seem to have the same need on fence_array, so I think we
can create a fence array helper to inspect if it has a specific context
or not.
> +
> + if (in_fence->context != gpu->fence_context) {
> + ret = dma_fence_wait(in_fence, true);
> + if (ret)
> + goto err_submit_objects;
sync_file_get_fence() hold a fence ref, we need to release it here
always and not only in case of error.
> + }
> + }
> +
> ret = submit_fence_sync(submit);
> if (ret)
> goto err_submit_objects;
> @@ -419,6 +449,8 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> flush_workqueue(priv->wq);
>
> err_submit_objects:
> + if (in_fence)
> + dma_fence_put(in_fence);
> submit_cleanup(submit);
>
> err_submit_cmds:
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 130d7d517a19a..51d52c72aef17 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1064,7 +1064,7 @@ static struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu)
> }
>
> int etnaviv_gpu_fence_sync_obj(struct etnaviv_gem_object *etnaviv_obj,
> - unsigned int context, bool exclusive)
> + unsigned int context, bool exclusive, bool explicit)
> {
> struct reservation_object *robj = etnaviv_obj->resv;
> struct reservation_object_list *fobj;
> @@ -1077,6 +1077,9 @@ int etnaviv_gpu_fence_sync_obj(struct etnaviv_gem_object *etnaviv_obj,
> return ret;
> }
>
> + if (explicit)
> + return 0;
> +
> /*
> * If we have any shared fences, then the exclusive fence
> * should be ignored as it will already have been signalled.
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 1c0606ea7d5e8..dc27c7a039060 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -181,7 +181,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m);
> #endif
>
> int etnaviv_gpu_fence_sync_obj(struct etnaviv_gem_object *etnaviv_obj,
> - unsigned int context, bool exclusive);
> + unsigned int context, bool exclusive, bool implicit);
>
> void etnaviv_gpu_retire(struct etnaviv_gpu *gpu);
> int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu,
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 2584c1cca42f6..e9c388a1d8ebe 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -154,6 +154,10 @@ struct drm_etnaviv_gem_submit_bo {
> * one or more cmdstream buffers. This allows for conditional execution
> * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> */
> +#define ETNA_SUBMIT_NO_IMPLICIT 0x0001
> +#define ETNA_SUBMIT_FENCE_FD_IN 0x0002
ETNA_SUBMIT_NO_IMPLICIT and ETNA_SUBMIT_FENCE_FD_IN are the same, when
you send and fence_fd to the kernel you are requesting on explicit sync
thus I think the ETNA_SUBMIT_NO_IMPLICIT can be dropped and
ETNA_SUBMIT_FENCE_FD_IN would be the one to look at.
> +#define ETNA_SUBMIT_FLAGS (ETNA_SUBMIT_NO_IMPLICIT | \
> + ETNA_SUBMIT_FENCE_FD_IN)
> #define ETNA_PIPE_3D 0x00
> #define ETNA_PIPE_2D 0x01
> #define ETNA_PIPE_VG 0x02
> @@ -167,6 +171,8 @@ struct drm_etnaviv_gem_submit {
> __u64 bos; /* in, ptr to array of submit_bo's */
> __u64 relocs; /* in, ptr to array of submit_reloc's */
> __u64 stream; /* in, ptr to cmdstream */
> + __u32 flags; /* in, mask of ETNA_SUBMIT_x */
> + __s32 fence_fd; /* in, fence fd (see ETNA_SUBMIT_FENCE_FD_IN) */
> };
>
> /* The normal way to synchronize with the GPU is just to CPU_PREP on
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
