| Message ID | 20170308125328.28306-1-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Philipp, 2017-03-08 Philipp Zabel <p.zabel@pengutronix.de>: > 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@pengutronix.de> > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Gustavo, thank you for the review. On Wed, 2017-03-08 at 11:37 -0300, Gustavo Padovan wrote: [...] > > @@ -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. Do you have a pointer where I could read up on how this should be done? > > + > > + 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. We do that already. err_submit_objects is an unfortunate name for patch review, but the out label at the end of the function falls right through to it. > > + } > > + } > > + > > 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); We pass through here in the non-error case, too. [...] > > @@ -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. I just followed Rob's lead. I'm not sure if there may be a reason to submit an in fence still keep implicit fencing enabled at the same time, or to allow a submit without any fencing whatsoever. Maybe for testing purposes? I'm happy to drop the NO_IMPLICIT flag if there is no need for it. regards Philipp
Hi Philipp, 2017-03-13 Philipp Zabel <p.zabel@pengutronix.de>: > Hi Gustavo, > > thank you for the review. > > On Wed, 2017-03-08 at 11:37 -0300, Gustavo Padovan wrote: > [...] > > > @@ -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. > > Do you have a pointer where I could read up on how this should be done? I was thinking on some function that would iterate over all fences in the fence_array and check their context. The if we find our own gpu context in there we fail the submit. > > > > + > > > + 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. > > We do that already. err_submit_objects is an unfortunate name for patch > review, but the out label at the end of the function falls right through > to it. > > > > + } > > > + } > > > + > > > 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); > > We pass through here in the non-error case, too. Cool. > > [...] > > > @@ -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. > > I just followed Rob's lead. I'm not sure if there may be a reason to > submit an in fence still keep implicit fencing enabled at the same time, > or to allow a submit without any fencing whatsoever. Maybe for testing > purposes? > I'm happy to drop the NO_IMPLICIT flag if there is no need for it. Right. My understanding is that the flags would mean the same thing, but I'm no expert on the GPU side of things. Maybe Rob can provide us some insight on why he added NO_IMPLICIT. Gustavo
On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan <gustavo@padovan.org> wrote: >> 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. jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly because I couldn't think of a good backwards-compatible way to add it later if needed. Currently userspace sets both flags together, and possibly always will. But keeping separate flags seemed like a good idea for future-proofing.. BR, -R >> +#define ETNA_SUBMIT_FLAGS (ETNA_SUBMIT_NO_IMPLICIT | \ >> + ETNA_SUBMIT_FENCE_FD_IN)
2017-03-16 Rob Clark <robdclark@gmail.com>: > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan <gustavo@padovan.org> wrote: > >> 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. > > jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly > because I couldn't think of a good backwards-compatible way to add it > later if needed. Currently userspace sets both flags together, and > possibly always will. But keeping separate flags seemed like a good > idea for future-proofing.. Fair enough. Let's do the same for etnaviv then. Gustavo
On Fri, 2017-03-17 at 10:55 -0300, Gustavo Padovan wrote: > 2017-03-16 Rob Clark <robdclark@gmail.com>: > > > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan <gustavo@padovan.org> wrote: > > >> 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. > > > > jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly > > because I couldn't think of a good backwards-compatible way to add it > > later if needed. Currently userspace sets both flags together, and > > possibly always will. But keeping separate flags seemed like a good > > idea for future-proofing.. > > Fair enough. Let's do the same for etnaviv then. Alright. Unless Lucas disagrees, I'll keep it as is for consistency. regards Philipp
Am Freitag, den 17.03.2017, 15:09 +0100 schrieb Philipp Zabel: > On Fri, 2017-03-17 at 10:55 -0300, Gustavo Padovan wrote: > > 2017-03-16 Rob Clark <robdclark@gmail.com>: > > > > > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan <gustavo@padovan.org> wrote: > > > >> 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. > > > > > > jfwiw, I kept separate no-implicit and fence-fd-in flags in msm mostly > > > because I couldn't think of a good backwards-compatible way to add it > > > later if needed. Currently userspace sets both flags together, and > > > possibly always will. But keeping separate flags seemed like a good > > > idea for future-proofing.. > > > > Fair enough. Let's do the same for etnaviv then. > > Alright. Unless Lucas disagrees, I'll keep it as is for consistency. This flag might make things a bit more "fun" later, as we might need to merge fences (or even fence arrays of different types) if we are going to use both implicit and explicit fences to infer scheduling decisions from. But to avoid introducing any unnecessary differences between freedreno and etnaviv, I would suggest to keep the flag around. Regards, Lucas
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.. + */ + + if (in_fence->context != gpu->fence_context) { + ret = dma_fence_wait(in_fence, true); + if (ret) + goto err_submit_objects; + } + } + 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 +#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
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@pengutronix.de> --- 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(-)