Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-23 Thread Steven Price
On 17/09/2019 08:15, Boris Brezillon wrote:
> On Mon, 16 Sep 2019 17:20:28 -0500
> Rob Herring  wrote:
> 
>> On Fri, Sep 13, 2019 at 6:17 AM Boris Brezillon
>>  wrote:
>>>
>>> The READ/WRITE flags are particularly useful if we want to avoid
>>> serialization of jobs that read from the same BO but never write to it.  
>>
>> Any data on performance differences?
> 
> Unfortunately no. When I initially added support for BO flags I thought
> it would fix a regression I had on one glmark2 test (ideas), but the
> problem ended up being something completely different (overhead of
> calling ioctl(WAIT_BO) on already idle BOs).
> 
> I just ran glmark2 again, and there doesn't seem to be a noticeable
> improvement with those 2 patches applied (and mesa patched to use the
> new flags). This being said, the improvement is likely to be workload
> dependent, so I wouldn't consider these patches as useless, but I'm
> fine putting them on hold until we see a real need.
> 
> Maybe Steven has some real use cases that could help outline the
> benefit of these patches.

To be honest I don't really know. The DDK internally does track this
read/write information - but then it doesn't involve the kernel in
tracking it. I was presuming that Mesa (because it exports the buffer
usage to the kernel) would benefit from being able to have multiple
readers - for example of a buffer being used as a texture for multiple
render passes. It's possible we don't see this benefit because we
haven't got the queuing of jobs merged yet?

There might also be some benefits when it comes to interaction with
other drivers, but I don't have any concrete examples.

>>
>>> The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
>>> shared but jobs are using different portions of the buffer.  
>>
>> Why don't we add this when it is useful rather than might be?
> 
> I don't have a need for that one yet, but etanviv has it in place so I
> thought I'd add both at the same time.
> Note that it could help us reduce the number of fence returned by
> panfrost_job_dependency(), but I'm not sure it makes a difference in
> terms of perfs.

I'm not aware of any reason for NO_IMPLICIT_FENCE. I found it somewhat
odd that it is effectively one way (it doesn't wait for fences, but
others could be waiting on the fence added by this usage). If we don't
have a use yet in Mesa then it's probably best to wait until we know how
this is actually going to be used.

There is of course already the option of simply omitting the BO in the
job to prevent any dependencies being created :)

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

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-17 Thread Alyssa Rosenzweig
> > +   /**
> > +* Pointer to a u32 array of _panfrost_submit_bo_desc objects. This
> > +* field is meant to replace _panfrost_submit.bo_handles which did
> > +* not provide enough information to relax synchronization between
> > +* jobs that only only read the BO they share. When both
> > +* _panfrost_submit.bo_handles and _panfrost_submit.bo_descs
> > +* are provided, drm_panfrost_submit.bo_handles is ignored.
> > +*/
> > +   __u64 bo_descs;
> > +
> > +   /**
> > +* Number of BO descriptors passed in (size is that times
> > +* sizeof(drm_panfrost_submit_bo_desc)).
> > +*/
> > +   __u32 bo_desc_count;
> 
> We don't really need another count field. bo_handle_count could be
> re-used. Indeed this could even be handled with just a flags field with
> a new flag specifying that bo_handles no longer points to handles but to
> bo_desc objects instead.

This seems like the cleaniest approach (also bumping the ABI version).
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-17 Thread Boris Brezillon
On Mon, 16 Sep 2019 17:20:28 -0500
Rob Herring  wrote:

> On Fri, Sep 13, 2019 at 6:17 AM Boris Brezillon
>  wrote:
> >
> > The READ/WRITE flags are particularly useful if we want to avoid
> > serialization of jobs that read from the same BO but never write to it.  
> 
> Any data on performance differences?

Unfortunately no. When I initially added support for BO flags I thought
it would fix a regression I had on one glmark2 test (ideas), but the
problem ended up being something completely different (overhead of
calling ioctl(WAIT_BO) on already idle BOs).

I just ran glmark2 again, and there doesn't seem to be a noticeable
improvement with those 2 patches applied (and mesa patched to use the
new flags). This being said, the improvement is likely to be workload
dependent, so I wouldn't consider these patches as useless, but I'm
fine putting them on hold until we see a real need.

Maybe Steven has some real use cases that could help outline the
benefit of these patches.

> 
> > The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
> > shared but jobs are using different portions of the buffer.  
> 
> Why don't we add this when it is useful rather than might be?

I don't have a need for that one yet, but etanviv has it in place so I
thought I'd add both at the same time.
Note that it could help us reduce the number of fence returned by
panfrost_job_dependency(), but I'm not sure it makes a difference in
terms of perfs.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-16 Thread Rob Herring
On Fri, Sep 13, 2019 at 8:44 AM Steven Price  wrote:
>
> On 13/09/2019 12:17, Boris Brezillon wrote:
> > The READ/WRITE flags are particularly useful if we want to avoid
> > serialization of jobs that read from the same BO but never write to it.
> > The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
> > shared but jobs are using different portions of the buffer.
> >
> > Signed-off-by: Boris Brezillon 
>
> Good feature - we could do with an (easy) way of the user driver
> detecting this - so it might be worth bumping the driver version for this?
>
> Some more comments below.
>
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c |  72 +--
> >  drivers/gpu/drm/panfrost/panfrost_job.c | 164 +++-
> >  drivers/gpu/drm/panfrost/panfrost_job.h |  11 +-
> >  include/uapi/drm/panfrost_drm.h |  41 ++
> >  4 files changed, 247 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> > b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index d74442d71048..08082fd557c3 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -119,20 +119,76 @@ panfrost_lookup_bos(struct drm_device *dev,
> > struct drm_panfrost_submit *args,
> > struct panfrost_job *job)
> >  {
> > - job->bo_count = args->bo_handle_count;
> > + struct drm_panfrost_submit_bo *bo_descs = NULL;
> > + u32 *handles = NULL;
> > + u32 i, bo_count;
> > + int ret = 0;
> >
> > - if (!job->bo_count)
> > + bo_count = args->bo_desc_count ?
> > +args->bo_desc_count : args->bo_handle_count;
> > + if (!bo_count)
> >   return 0;
> >
> > - job->implicit_fences = kvmalloc_array(job->bo_count,
> > -   sizeof(struct dma_fence *),
> > + job->bos = kvmalloc_array(bo_count, sizeof(*job->bos),
> > GFP_KERNEL | __GFP_ZERO);
> > - if (!job->implicit_fences)
> > + if (!job->bos)
> >   return -ENOMEM;
> >
> > - return drm_gem_objects_lookup(file_priv,
> > -   (void __user 
> > *)(uintptr_t)args->bo_handles,
> > -   job->bo_count, >bos);
> > + job->bo_count = bo_count;
> > + bo_descs = kvmalloc_array(bo_count, sizeof(*bo_descs),
> > +   GFP_KERNEL | __GFP_ZERO);
> > + if (!bo_descs) {
> > + ret = -ENOMEM;
> > + goto out;
>
> This can be just "return -ENOMEM" - both handles and bo_descs will be NULL.
>
> > + }
> > +
> > + if (!args->bo_desc_count) {
> > + handles = kvmalloc_array(bo_count, sizeof(*handles),
> > +  GFP_KERNEL);
> > + if (!handles) {
> > + ret =-ENOMEM;
> > + goto out;
> > + }
> > +
> > + if (copy_from_user(handles,
> > +(void __user *)(uintptr_t)args->bo_handles,
> > +job->bo_count * sizeof(*handles))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > +
> > + for (i = 0; i < job->bo_count; i++) {
> > + bo_descs[i].handle = handles[i];
> > + bo_descs[i].flags = PANFROST_SUBMIT_BO_WRITE |
> > + PANFROST_SUBMIT_BO_READ;
>
> You can use PANFROST_SUBMIT_BO_RW here.
>
> > + }
> > + } else if (copy_from_user(bo_descs,
> > +   (void __user *)(uintptr_t)args->bo_descs,
> > +   job->bo_count * sizeof(*bo_descs))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > +
> > + for (i = 0; i < job->bo_count; i++) {
> > + if ((bo_descs[i].flags & ~PANFROST_SUBMIT_BO_VALID_FLAGS) ||
> > +!(bo_descs[i].flags & PANFROST_SUBMIT_BO_RW)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + job->bos[i].flags = bo_descs[i].flags;
> > + job->bos[i].obj = drm_gem_object_lookup(file_priv,
> > + bo_descs[i].handle);
> > + if (!job->bos[i].obj) {
> > + ret = -ENOENT;
> > + goto out;
> > + }
> > + }
> > +
> > +out:
> > + kvfree(handles);
> > + kvfree(bo_descs);
> > + return ret;
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
> > b/drivers/gpu/drm/panfrost/panfrost_job.c
> > index 05c85f45a0de..e4b74fde9339 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> > @@ -193,24 +193,116 @@ static void panfrost_job_hw_submit(struct 
> > panfrost_job *job, int js)
> >   

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-16 Thread Rob Herring
On Fri, Sep 13, 2019 at 6:17 AM Boris Brezillon
 wrote:
>
> The READ/WRITE flags are particularly useful if we want to avoid
> serialization of jobs that read from the same BO but never write to it.

Any data on performance differences?

> The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
> shared but jobs are using different portions of the buffer.

Why don't we add this when it is useful rather than might be?

>
> Signed-off-by: Boris Brezillon 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-16 Thread Boris Brezillon
On Fri, 13 Sep 2019 14:44:14 +0100
Steven Price  wrote:

> On 13/09/2019 12:17, Boris Brezillon wrote:
> > The READ/WRITE flags are particularly useful if we want to avoid
> > serialization of jobs that read from the same BO but never write to it.
> > The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
> > shared but jobs are using different portions of the buffer.
> > 
> > Signed-off-by: Boris Brezillon   
> 
> Good feature - we could do with an (easy) way of the user driver
> detecting this - so it might be worth bumping the driver version for this?

I was trying to support this feature without adding a new feature
flag or bumping the minor version, but I guess there's no good
reason to do that.

> 
> Some more comments below.
> 
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_drv.c |  72 +--
> >  drivers/gpu/drm/panfrost/panfrost_job.c | 164 +++-
> >  drivers/gpu/drm/panfrost/panfrost_job.h |  11 +-
> >  include/uapi/drm/panfrost_drm.h |  41 ++
> >  4 files changed, 247 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> > b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index d74442d71048..08082fd557c3 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -119,20 +119,76 @@ panfrost_lookup_bos(struct drm_device *dev,
> >   struct drm_panfrost_submit *args,
> >   struct panfrost_job *job)
> >  {
> > -   job->bo_count = args->bo_handle_count;
> > +   struct drm_panfrost_submit_bo *bo_descs = NULL;
> > +   u32 *handles = NULL;
> > +   u32 i, bo_count;
> > +   int ret = 0;
> >  
> > -   if (!job->bo_count)
> > +   bo_count = args->bo_desc_count ?
> > +  args->bo_desc_count : args->bo_handle_count;
> > +   if (!bo_count)
> > return 0;
> >  
> > -   job->implicit_fences = kvmalloc_array(job->bo_count,
> > - sizeof(struct dma_fence *),
> > +   job->bos = kvmalloc_array(bo_count, sizeof(*job->bos),
> >   GFP_KERNEL | __GFP_ZERO);
> > -   if (!job->implicit_fences)
> > +   if (!job->bos)
> > return -ENOMEM;
> >  
> > -   return drm_gem_objects_lookup(file_priv,
> > - (void __user 
> > *)(uintptr_t)args->bo_handles,
> > - job->bo_count, >bos);
> > +   job->bo_count = bo_count;
> > +   bo_descs = kvmalloc_array(bo_count, sizeof(*bo_descs),
> > + GFP_KERNEL | __GFP_ZERO);
> > +   if (!bo_descs) {
> > +   ret = -ENOMEM;
> > +   goto out;  
> 
> This can be just "return -ENOMEM" - both handles and bo_descs will be NULL.

Will fix that.

> 
> > +   }
> > +
> > +   if (!args->bo_desc_count) {
> > +   handles = kvmalloc_array(bo_count, sizeof(*handles),
> > +GFP_KERNEL);
> > +   if (!handles) {
> > +   ret =-ENOMEM;
> > +   goto out;
> > +   }
> > +
> > +   if (copy_from_user(handles,
> > +  (void __user *)(uintptr_t)args->bo_handles,
> > +  job->bo_count * sizeof(*handles))) {
> > +   ret = -EFAULT;
> > +   goto out;
> > +   }
> > +
> > +   for (i = 0; i < job->bo_count; i++) {
> > +   bo_descs[i].handle = handles[i];
> > +   bo_descs[i].flags = PANFROST_SUBMIT_BO_WRITE |
> > +   PANFROST_SUBMIT_BO_READ;  
> 
> You can use PANFROST_SUBMIT_BO_RW here.

That as well.

> 
> > +   }
> > +   } else if (copy_from_user(bo_descs,
> > + (void __user *)(uintptr_t)args->bo_descs,
> > + job->bo_count * sizeof(*bo_descs))) {
> > +   ret = -EFAULT;
> > +   goto out;
> > +   }
> > +
> > +   for (i = 0; i < job->bo_count; i++) {
> > +   if ((bo_descs[i].flags & ~PANFROST_SUBMIT_BO_VALID_FLAGS) ||
> > +!(bo_descs[i].flags & PANFROST_SUBMIT_BO_RW)) {
> > +   ret = -EINVAL;
> > +   goto out;
> > +   }
> > +
> > +   job->bos[i].flags = bo_descs[i].flags;
> > +   job->bos[i].obj = drm_gem_object_lookup(file_priv,
> > +   bo_descs[i].handle);
> > +   if (!job->bos[i].obj) {
> > +   ret = -ENOENT;
> > +   goto out;
> > +   }
> > +   }
> > +
> > +out:
> > +   kvfree(handles);
> > +   kvfree(bo_descs);
> > +   return ret;
> >  }
> >  
> >  /**
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
> > b/drivers/gpu/drm/panfrost/panfrost_job.c
> > index 05c85f45a0de..e4b74fde9339 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> > @@ -193,24 +193,116 @@ static void 

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-13 Thread Steven Price
On 13/09/2019 12:17, Boris Brezillon wrote:
> The READ/WRITE flags are particularly useful if we want to avoid
> serialization of jobs that read from the same BO but never write to it.
> The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
> shared but jobs are using different portions of the buffer.
> 
> Signed-off-by: Boris Brezillon 

Good feature - we could do with an (easy) way of the user driver
detecting this - so it might be worth bumping the driver version for this?

Some more comments below.

> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c |  72 +--
>  drivers/gpu/drm/panfrost/panfrost_job.c | 164 +++-
>  drivers/gpu/drm/panfrost/panfrost_job.h |  11 +-
>  include/uapi/drm/panfrost_drm.h |  41 ++
>  4 files changed, 247 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
> b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d74442d71048..08082fd557c3 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -119,20 +119,76 @@ panfrost_lookup_bos(struct drm_device *dev,
> struct drm_panfrost_submit *args,
> struct panfrost_job *job)
>  {
> - job->bo_count = args->bo_handle_count;
> + struct drm_panfrost_submit_bo *bo_descs = NULL;
> + u32 *handles = NULL;
> + u32 i, bo_count;
> + int ret = 0;
>  
> - if (!job->bo_count)
> + bo_count = args->bo_desc_count ?
> +args->bo_desc_count : args->bo_handle_count;
> + if (!bo_count)
>   return 0;
>  
> - job->implicit_fences = kvmalloc_array(job->bo_count,
> -   sizeof(struct dma_fence *),
> + job->bos = kvmalloc_array(bo_count, sizeof(*job->bos),
> GFP_KERNEL | __GFP_ZERO);
> - if (!job->implicit_fences)
> + if (!job->bos)
>   return -ENOMEM;
>  
> - return drm_gem_objects_lookup(file_priv,
> -   (void __user 
> *)(uintptr_t)args->bo_handles,
> -   job->bo_count, >bos);
> + job->bo_count = bo_count;
> + bo_descs = kvmalloc_array(bo_count, sizeof(*bo_descs),
> +   GFP_KERNEL | __GFP_ZERO);
> + if (!bo_descs) {
> + ret = -ENOMEM;
> + goto out;

This can be just "return -ENOMEM" - both handles and bo_descs will be NULL.

> + }
> +
> + if (!args->bo_desc_count) {
> + handles = kvmalloc_array(bo_count, sizeof(*handles),
> +  GFP_KERNEL);
> + if (!handles) {
> + ret =-ENOMEM;
> + goto out;
> + }
> +
> + if (copy_from_user(handles,
> +(void __user *)(uintptr_t)args->bo_handles,
> +job->bo_count * sizeof(*handles))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + for (i = 0; i < job->bo_count; i++) {
> + bo_descs[i].handle = handles[i];
> + bo_descs[i].flags = PANFROST_SUBMIT_BO_WRITE |
> + PANFROST_SUBMIT_BO_READ;

You can use PANFROST_SUBMIT_BO_RW here.

> + }
> + } else if (copy_from_user(bo_descs,
> +   (void __user *)(uintptr_t)args->bo_descs,
> +   job->bo_count * sizeof(*bo_descs))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + for (i = 0; i < job->bo_count; i++) {
> + if ((bo_descs[i].flags & ~PANFROST_SUBMIT_BO_VALID_FLAGS) ||
> +!(bo_descs[i].flags & PANFROST_SUBMIT_BO_RW)) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + job->bos[i].flags = bo_descs[i].flags;
> + job->bos[i].obj = drm_gem_object_lookup(file_priv,
> + bo_descs[i].handle);
> + if (!job->bos[i].obj) {
> + ret = -ENOENT;
> + goto out;
> + }
> + }
> +
> +out:
> + kvfree(handles);
> + kvfree(bo_descs);
> + return ret;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
> b/drivers/gpu/drm/panfrost/panfrost_job.c
> index 05c85f45a0de..e4b74fde9339 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
> @@ -193,24 +193,116 @@ static void panfrost_job_hw_submit(struct panfrost_job 
> *job, int js)
>   pm_runtime_put_autosuspend(pfdev->dev);
>  }
>  
> -static void panfrost_acquire_object_fences(struct drm_gem_object **bos,
> -int bo_count,
> -struct dma_fence **implicit_fences)
> +static int 

[PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-13 Thread Boris Brezillon
The READ/WRITE flags are particularly useful if we want to avoid
serialization of jobs that read from the same BO but never write to it.
The NO_IMPLICIT_FENCE might be useful when the user knows the BO is
shared but jobs are using different portions of the buffer.

Signed-off-by: Boris Brezillon 
---
 drivers/gpu/drm/panfrost/panfrost_drv.c |  72 +--
 drivers/gpu/drm/panfrost/panfrost_job.c | 164 +++-
 drivers/gpu/drm/panfrost/panfrost_job.h |  11 +-
 include/uapi/drm/panfrost_drm.h |  41 ++
 4 files changed, 247 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index d74442d71048..08082fd557c3 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -119,20 +119,76 @@ panfrost_lookup_bos(struct drm_device *dev,
  struct drm_panfrost_submit *args,
  struct panfrost_job *job)
 {
-   job->bo_count = args->bo_handle_count;
+   struct drm_panfrost_submit_bo *bo_descs = NULL;
+   u32 *handles = NULL;
+   u32 i, bo_count;
+   int ret = 0;
 
-   if (!job->bo_count)
+   bo_count = args->bo_desc_count ?
+  args->bo_desc_count : args->bo_handle_count;
+   if (!bo_count)
return 0;
 
-   job->implicit_fences = kvmalloc_array(job->bo_count,
- sizeof(struct dma_fence *),
+   job->bos = kvmalloc_array(bo_count, sizeof(*job->bos),
  GFP_KERNEL | __GFP_ZERO);
-   if (!job->implicit_fences)
+   if (!job->bos)
return -ENOMEM;
 
-   return drm_gem_objects_lookup(file_priv,
- (void __user 
*)(uintptr_t)args->bo_handles,
- job->bo_count, >bos);
+   job->bo_count = bo_count;
+   bo_descs = kvmalloc_array(bo_count, sizeof(*bo_descs),
+ GFP_KERNEL | __GFP_ZERO);
+   if (!bo_descs) {
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   if (!args->bo_desc_count) {
+   handles = kvmalloc_array(bo_count, sizeof(*handles),
+GFP_KERNEL);
+   if (!handles) {
+   ret =-ENOMEM;
+   goto out;
+   }
+
+   if (copy_from_user(handles,
+  (void __user *)(uintptr_t)args->bo_handles,
+  job->bo_count * sizeof(*handles))) {
+   ret = -EFAULT;
+   goto out;
+   }
+
+   for (i = 0; i < job->bo_count; i++) {
+   bo_descs[i].handle = handles[i];
+   bo_descs[i].flags = PANFROST_SUBMIT_BO_WRITE |
+   PANFROST_SUBMIT_BO_READ;
+   }
+   } else if (copy_from_user(bo_descs,
+ (void __user *)(uintptr_t)args->bo_descs,
+ job->bo_count * sizeof(*bo_descs))) {
+   ret = -EFAULT;
+   goto out;
+   }
+
+   for (i = 0; i < job->bo_count; i++) {
+   if ((bo_descs[i].flags & ~PANFROST_SUBMIT_BO_VALID_FLAGS) ||
+!(bo_descs[i].flags & PANFROST_SUBMIT_BO_RW)) {
+   ret = -EINVAL;
+   goto out;
+   }
+
+   job->bos[i].flags = bo_descs[i].flags;
+   job->bos[i].obj = drm_gem_object_lookup(file_priv,
+   bo_descs[i].handle);
+   if (!job->bos[i].obj) {
+   ret = -ENOENT;
+   goto out;
+   }
+   }
+
+out:
+   kvfree(handles);
+   kvfree(bo_descs);
+   return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 05c85f45a0de..e4b74fde9339 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -193,24 +193,116 @@ static void panfrost_job_hw_submit(struct panfrost_job 
*job, int js)
pm_runtime_put_autosuspend(pfdev->dev);
 }
 
-static void panfrost_acquire_object_fences(struct drm_gem_object **bos,
-  int bo_count,
-  struct dma_fence **implicit_fences)
+static int panfrost_acquire_object_fences(struct panfrost_job *job)
 {
-   int i;
+   int i, ret;
 
-   for (i = 0; i < bo_count; i++)
-   implicit_fences[i] = dma_resv_get_excl_rcu(bos[i]->resv);
+   for (i = 0; i < job->bo_count; i++) {
+   struct panfrost_job_bo_desc *bo = >bos[i];
+   struct dma_resv *robj = bo->obj->resv;
+
+   if (!(job->bos[i].flags & PANFROST_SUBMIT_BO_WRITE)) {
+