Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Daniel Vetter]

On Fri, Mar 17, 2017 at 11:00:44AM -0300, Gustavo Padovan wrote:
> 2017-03-16 Philipp Zabel :
> 
> > Hi Gustavo,
> > 
> > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > [...]
> > > 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.
> > 
> > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > it be enough to just wait if there are foreign fences in the array?
> 
> You don't need to fail necessarily. In my mind I had the use case that
> maybe some driver could deadlock waiting for his own fence. What you
> do with the information that the array has it a fence with the driver's
> context is entirely up to the driver to decide.

With the current infrastructure you can't have future fences (i.e. a fence
for something that might happen somewhen in the future, but for which no
driver is yet committed to execute - i.e. it's not yet queued). And
without future fences you can't ever have deadlocks.

The "are these all my own fences" check is more useful just to import the
fences into your own internal objects and use your driver-internal depency
handling (which usually means, more hw, less cpu waiting). But that's
entirely orthogonal to "can we deadlock", which atm is "no"[1].
-Daniel

1: You can deadlock when e.g. one driver holds a lock while waiting for a
fence, and another driver needs that lock before it is willing to signal
said fence. But that's not any different from waiting for anything else in
the kernel, and will be checked by the cross-release lockdep stuff rsn.

> 
> > 
> > How about something like this:
> > 
> > --8<--
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index 364fe50d020de..0b0bdaf4406d4 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit 
> > *submit)
> > kfree(submit);
> >  }
> >  
> > +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
> > +{
> > +   if (dma_fence_is_array(fence)) {
> > +   struct dma_fence_array *array = to_dma_fence_array(fence);
> > +   int i;
> > +
> > +   for (i = 0; i < array->num_fences; i++) {
> > +   if (array->fences[i]->context != context)
> > +   return false;
> > +   }
> > +   return true;
> > +   }
> > +
> > +   return fence->context == context;
> > +}
> 
> If we don't mind having fences with our own context, why should we check
> them?
> 
> Gustavo
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Lucas Stach]

Am Samstag, den 18.03.2017, 15:19 +0100 schrieb Christian König:
> Am 17.03.2017 um 15:58 schrieb Lucas Stach:
> > Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM
> > Linux:
> > > On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:
> > > > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp
> > > > Zabel:
> > > > > Hi Gustavo,
> > > > > 
> > > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > > > > [...]
> > > > > > 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.
> > > > > 
> > > > > Why would we have to fail if somebody feeds us our own
> > > > > fences? Wouldn't
> > > > > it be enough to just wait if there are foreign fences in the
> > > > > array?
> > > > 
> > > > Yes, skipping the wait if all fences are from our own context
> > > > is an
> > > > optimization and it's certainly not an issue if someone feeds
> > > > us our own
> > > > fences.
> > > 
> > > Are you sure about that - what if we have two GPUs, a 2D and 3D
> > > GPU,
> > > and we're fed an etnaviv fence for one GPU when submitting to the
> > > other GPU.
> > > 
> > > So we do end up being fed our own fences, and we have to respect
> > > them
> > > otherwise we lose inter-GPU synchronisation, and that will break
> > > existing userspace.
> > > 
> > 
> > The etnaviv GPUs, while being on the same DRM device, have distinct
> > fence contexts. So the 3D GPU will consider a fence from the 2D GPU
> > as
> > foreign and properly wait on it.
> > 
> > It's only when we get an in fence that has been generated as an out
> > fence by one (or multiple) submits to the same GPU, that we are
> > able to
> > skip the wait and enqueue the command without waiting for the fence
> > to
> > signal.
> 
> BTW: Do you still have the needs for a GPU scheduler?
> 
> The scheduler amdgpu uses is hopefully still hardware agnostic and
> has 
> all that handling already included.
> 
> Using it can avoid blocking for foreign fences during your command 
> submission and I won't mind seeing that moved into common drm code.

Yes, it's still on my list of features to enable for etnaviv. It's just
 that other things like enabling more hardware and getting performance
up had priority over this.

I've looked at the amdgpu scheduler and I agree that it's probably the
right thing to move this out into common code and make use of it in
etnaviv.

Regards,
Lucas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Christian König]

Am 17.03.2017 um 15:58 schrieb Lucas Stach:

Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM
Linux:

On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:

Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:

Hi Gustavo,

On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
[...]

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.

Why would we have to fail if somebody feeds us our own fences? Wouldn't
it be enough to just wait if there are foreign fences in the array?

Yes, skipping the wait if all fences are from our own context is an
optimization and it's certainly not an issue if someone feeds us our own
fences.

Are you sure about that - what if we have two GPUs, a 2D and 3D GPU,
and we're fed an etnaviv fence for one GPU when submitting to the
other GPU.

So we do end up being fed our own fences, and we have to respect them
otherwise we lose inter-GPU synchronisation, and that will break
existing userspace.


The etnaviv GPUs, while being on the same DRM device, have distinct
fence contexts. So the 3D GPU will consider a fence from the 2D GPU as
foreign and properly wait on it.

It's only when we get an in fence that has been generated as an out
fence by one (or multiple) submits to the same GPU, that we are able to
skip the wait and enqueue the command without waiting for the fence to
signal.


BTW: Do you still have the needs for a GPU scheduler?

The scheduler amdgpu uses is hopefully still hardware agnostic and has 
all that handling already included.


Using it can avoid blocking for foreign fences during your command 
submission and I won't mind seeing that moved into common drm code.


Regards,
Christian.



Regards,
Lucas

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Chris Healy]

On Fri, Mar 17, 2017 at 7:58 AM, Lucas Stach  wrote:
> Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM
> Linux:
>> On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:
>> > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
>> > > Hi Gustavo,
>> > >
>> > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
>> > > [...]
>> > > > 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.
>> > >
>> > > Why would we have to fail if somebody feeds us our own fences? Wouldn't
>> > > it be enough to just wait if there are foreign fences in the array?
>> >
>> > Yes, skipping the wait if all fences are from our own context is an
>> > optimization and it's certainly not an issue if someone feeds us our own
>> > fences.
>>
>> Are you sure about that - what if we have two GPUs, a 2D and 3D GPU,
>> and we're fed an etnaviv fence for one GPU when submitting to the
>> other GPU.
>>
>> So we do end up being fed our own fences, and we have to respect them
>> otherwise we lose inter-GPU synchronisation, and that will break
>> existing userspace.
>>
> The etnaviv GPUs, while being on the same DRM device, have distinct
> fence contexts. So the 3D GPU will consider a fence from the 2D GPU as
> foreign and properly wait on it.
>
> It's only when we get an in fence that has been generated as an out
> fence by one (or multiple) submits to the same GPU, that we are able to
> skip the wait and enqueue the command without waiting for the fence to
> signal.

With regard to the 2D and 3D GPU case, it seems to me that a good
example use case would be where the 3D GPU is used in Android for all
the surface generation using OpenGL and then the 2D GPU would get used
to composite all those surfaces together, leaving the 3D open to work
on other stuff.  As I understand it, the 2D GPU is much faster at 2D
compositing than the 3D GPU would be, (not to mention less power
hungry.)

>
> Regards,
> Lucas
>
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Russell King - ARM Linux]

On Fri, Mar 17, 2017 at 03:58:27PM +0100, Lucas Stach wrote:
> Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM
> Linux:
> > On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:
> > > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
> > > > Hi Gustavo,
> > > > 
> > > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > > > [...]
> > > > > 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.
> > > > 
> > > > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > > > it be enough to just wait if there are foreign fences in the array?
> > > 
> > > Yes, skipping the wait if all fences are from our own context is an
> > > optimization and it's certainly not an issue if someone feeds us our own
> > > fences.
> > 
> > Are you sure about that - what if we have two GPUs, a 2D and 3D GPU,
> > and we're fed an etnaviv fence for one GPU when submitting to the
> > other GPU.
> > 
> > So we do end up being fed our own fences, and we have to respect them
> > otherwise we lose inter-GPU synchronisation, and that will break
> > existing userspace.
> > 
> The etnaviv GPUs, while being on the same DRM device, have distinct
> fence contexts. So the 3D GPU will consider a fence from the 2D GPU as
> foreign and properly wait on it.
> 
> It's only when we get an in fence that has been generated as an out
> fence by one (or multiple) submits to the same GPU, that we are able to
> skip the wait and enqueue the command without waiting for the fence to
> signal.

Sounds fine then.  Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Russell King - ARM Linux]

On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:
> Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
> > Hi Gustavo,
> > 
> > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > [...]
> > > 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.
> > 
> > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > it be enough to just wait if there are foreign fences in the array?
> 
> Yes, skipping the wait if all fences are from our own context is an
> optimization and it's certainly not an issue if someone feeds us our own
> fences.

Are you sure about that - what if we have two GPUs, a 2D and 3D GPU,
and we're fed an etnaviv fence for one GPU when submitting to the
other GPU.

So we do end up being fed our own fences, and we have to respect them
otherwise we lose inter-GPU synchronisation, and that will break
existing userspace.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Lucas Stach]

Am Freitag, den 17.03.2017, 14:42 + schrieb Russell King - ARM
Linux:
> On Fri, Mar 17, 2017 at 03:10:21PM +0100, Lucas Stach wrote:
> > Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
> > > Hi Gustavo,
> > > 
> > > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > > [...]
> > > > 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.
> > > 
> > > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > > it be enough to just wait if there are foreign fences in the array?
> > 
> > Yes, skipping the wait if all fences are from our own context is an
> > optimization and it's certainly not an issue if someone feeds us our own
> > fences.
> 
> Are you sure about that - what if we have two GPUs, a 2D and 3D GPU,
> and we're fed an etnaviv fence for one GPU when submitting to the
> other GPU.
> 
> So we do end up being fed our own fences, and we have to respect them
> otherwise we lose inter-GPU synchronisation, and that will break
> existing userspace.
> 
The etnaviv GPUs, while being on the same DRM device, have distinct
fence contexts. So the 3D GPU will consider a fence from the 2D GPU as
foreign and properly wait on it.

It's only when we get an in fence that has been generated as an out
fence by one (or multiple) submits to the same GPU, that we are able to
skip the wait and enqueue the command without waiting for the fence to
signal.

Regards,
Lucas

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Lucas Stach]

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 :
> > 
> > > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan  
> > > 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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Lucas Stach]

Am Donnerstag, den 16.03.2017, 12:05 +0100 schrieb Philipp Zabel:
> Hi Gustavo,
> 
> On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> [...]
> > 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.
> 
> Why would we have to fail if somebody feeds us our own fences? Wouldn't
> it be enough to just wait if there are foreign fences in the array?

Yes, skipping the wait if all fences are from our own context is an
optimization and it's certainly not an issue if someone feeds us our own
fences.

> 
> How about something like this:
> 
> --8<--
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 364fe50d020de..0b0bdaf4406d4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit 
> *submit)
>   kfree(submit);
>  }
>  
> +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)

dma_fence_match_context?

> +{
> + if (dma_fence_is_array(fence)) {
> + struct dma_fence_array *array = to_dma_fence_array(fence);
> + int i;
> +
> + for (i = 0; i < array->num_fences; i++) {
> + if (array->fences[i]->context != context)
> + return false;
> + }
> + return true;
> + }
> +
> + return fence->context == context;
> +}
> +

This looks good to me. Please add this to a common location in the next
submission.

>  int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
>   struct drm_file *file)
>  {
> @@ -413,12 +429,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, 
> void *data,
>   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..
> + /*
> +  * Wait if the fence is from a foreign context, or if the fence
> +  * array contains any fence from a foreign context.
>*/
> -
> - if (in_fence->context != gpu->fence_context) {
> + if (!dma_fence_all_in_context(in_fence, gpu->fence_context)) {
>   ret = dma_fence_wait(in_fence, true);
>   if (ret)
>   goto err_submit_objects;
> -->8--
> 
> [...]
> > > > > @@ -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.
> 
> Yes, that would be welcome.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Philipp Zabel]

On Fri, 2017-03-17 at 10:55 -0300, Gustavo Padovan wrote:
> 2017-03-16 Rob Clark :
> 
> > On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan  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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Philipp Zabel]

On Fri, 2017-03-17 at 11:00 -0300, Gustavo Padovan wrote:
> 2017-03-16 Philipp Zabel :
> 
> > Hi Gustavo,
> > 
> > On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> > [...]
> > > 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.
> > 
> > Why would we have to fail if somebody feeds us our own fences? Wouldn't
> > it be enough to just wait if there are foreign fences in the array?
> 
> You don't need to fail necessarily. In my mind I had the use case that
> maybe some driver could deadlock waiting for his own fence. What you
> do with the information that the array has it a fence with the driver's
> context is entirely up to the driver to decide.
[...]
> If we don't mind having fences with our own context, why should we check
> them?

My understanding was that for foreign fences we have to wait, for
example before resolving into a framebuffer that is still being scanned
out. But if we are fed fences from our own context, we can just ignore
them because etnaviv just has a single command queue, so anything
waiting for a fence from our own context will be queued after that fence
is signaled anyway.

regards
Philipp

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Gustavo Padovan]

2017-03-16 Philipp Zabel :

> Hi Gustavo,
> 
> On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
> [...]
> > 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.
> 
> Why would we have to fail if somebody feeds us our own fences? Wouldn't
> it be enough to just wait if there are foreign fences in the array?

You don't need to fail necessarily. In my mind I had the use case that
maybe some driver could deadlock waiting for his own fence. What you
do with the information that the array has it a fence with the driver's
context is entirely up to the driver to decide.

> 
> How about something like this:
> 
> --8<--
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 364fe50d020de..0b0bdaf4406d4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit 
> *submit)
>   kfree(submit);
>  }
>  
> +static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
> +{
> + if (dma_fence_is_array(fence)) {
> + struct dma_fence_array *array = to_dma_fence_array(fence);
> + int i;
> +
> + for (i = 0; i < array->num_fences; i++) {
> + if (array->fences[i]->context != context)
> + return false;
> + }
> + return true;
> + }
> +
> + return fence->context == context;
> +}

If we don't mind having fences with our own context, why should we check
them?

Gustavo
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Gustavo Padovan]

2017-03-16 Rob Clark :

> On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan  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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Rob Clark]

On Wed, Mar 8, 2017 at 9:37 AM, Gustavo Padovan  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)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Philipp Zabel]

Hi Gustavo,

On Mon, 2017-03-13 at 14:37 -0300, Gustavo Padovan wrote:
[...]
> 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.

Why would we have to fail if somebody feeds us our own fences? Wouldn't
it be enough to just wait if there are foreign fences in the array?

How about something like this:

--8<--
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 364fe50d020de..0b0bdaf4406d4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -296,6 +296,22 @@ static void submit_cleanup(struct etnaviv_gem_submit 
*submit)
kfree(submit);
 }
 
+static bool dma_fence_all_in_context(struct dma_fence *fence, u64 context)
+{
+   if (dma_fence_is_array(fence)) {
+   struct dma_fence_array *array = to_dma_fence_array(fence);
+   int i;
+
+   for (i = 0; i < array->num_fences; i++) {
+   if (array->fences[i]->context != context)
+   return false;
+   }
+   return true;
+   }
+
+   return fence->context == context;
+}
+
 int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
struct drm_file *file)
 {
@@ -413,12 +429,11 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void 
*data,
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..
+   /*
+* Wait if the fence is from a foreign context, or if the fence
+* array contains any fence from a foreign context.
 */
-
-   if (in_fence->context != gpu->fence_context) {
+   if (!dma_fence_all_in_context(in_fence, gpu->fence_context)) {
ret = dma_fence_wait(in_fence, true);
if (ret)
goto err_submit_objects;
-->8--

[...]
> > > > @@ -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.

Yes, that would be welcome.

regards
Philipp

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Gustavo Padovan]

Hi Philipp,

2017-03-13 Philipp Zabel :

> 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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Philipp Zabel]

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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/etnaviv: submit support for in-fences

[Gustavo Padovan]

Hi Philipp,

2017-03-08 Philipp Zabel :

> 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 
> ---
>  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 
> +#include 
>  #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 

[PATCH 1/3] drm/etnaviv: submit support for in-fences

[Philipp Zabel]

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 
---
 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 
+#include 
 #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