[PATCH 0/6] vc4 miscellaneous v3d fixes

[PATCH 0/6] vc4 miscellaneous v3d fixes

[Eric Anholt]

Here are a bunch of miscellaneous fixes for 3D I've come up with while
doing a bunch of piglit runs.  One has a new IGT test sent out, and
I've got a test almost ready for large CLs.

Eric Anholt (6):
  drm/vc4: Use drm_free_large() on handles to match its allocation.
  drm/vc4: Use drm_malloc_ab to fix large rendering jobs.
  drm/vc4: Fix handling of a pm_runtime_get_sync() success case.
  drm/vc4: Free hang state before destroying BO cache.
  drm/vc4: Fix overflow mem unreferencing when the binner runs dry.
  drm/vc4: Fix oops when userspace hands in a bad BO.

 drivers/gpu/drm/vc4/vc4_drv.c |  6 +++---
 drivers/gpu/drm/vc4/vc4_drv.h |  9 +++++++++
 drivers/gpu/drm/vc4/vc4_gem.c | 18 +++++++++---------
 drivers/gpu/drm/vc4/vc4_irq.c |  4 +++-
 4 files changed, 24 insertions(+), 13 deletions(-)

-- 
2.8.1

[PATCH 1/6] drm/vc4: Use drm_free_large() on handles to match its allocation.

[Eric Anholt]

If you managed to exceed the limit to switch to vmalloc, we'd use the
wrong free.

Signed-off-by: Eric Anholt <eric@anholt.net>
Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/vc4/vc4_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 46899d6de675..f9b13b54c86b 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -574,7 +574,7 @@ vc4_cl_lookup_bos(struct drm_device *dev,
 	spin_unlock(&file_priv->table_lock);
 
 fail:
-	kfree(handles);
+	drm_free_large(handles);
 	return 0;
 }
 
-- 
2.8.1

[PATCH 2/6] drm/vc4: Use drm_malloc_ab to fix large rendering jobs.

[Eric Anholt]

If you exceeded the size that kmalloc would return, you'd get a dmesg
warning and an error from the job submit.  We can handle much larger
allocations with vmalloc, and drm_malloc_ab makes that decision.

Fixes failure in piglit's scissor-many.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_gem.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index f9b13b54c86b..fb2b1c526646 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -536,8 +536,8 @@ vc4_cl_lookup_bos(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	exec->bo = kcalloc(exec->bo_count, sizeof(struct drm_gem_cma_object *),
-			   GFP_KERNEL);
+	exec->bo = drm_calloc_large(exec->bo_count,
+				    sizeof(struct drm_gem_cma_object *));
 	if (!exec->bo) {
 		DRM_ERROR("Failed to allocate validated BO pointers\n");
 		return -ENOMEM;
@@ -610,7 +610,7 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec)
 	 * read the contents back for validation, and I think the
 	 * bo->vaddr is uncached access.
 	 */
-	temp = kmalloc(temp_size, GFP_KERNEL);
+	temp = drm_malloc_ab(temp_size, 1);
 	if (!temp) {
 		DRM_ERROR("Failed to allocate storage for copying "
 			  "in bin/render CLs.\n");
@@ -677,7 +677,7 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec)
 	ret = vc4_validate_shader_recs(dev, exec);
 
 fail:
-	kfree(temp);
+	drm_free_large(temp);
 	return ret;
 }
 
@@ -692,7 +692,7 @@ vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec)
 	if (exec->bo) {
 		for (i = 0; i < exec->bo_count; i++)
 			drm_gem_object_unreference(&exec->bo[i]->base);
-		kfree(exec->bo);
+		drm_free_large(exec->bo);
 	}
 
 	while (!list_empty(&exec->unref_list)) {
-- 
2.8.1

[PATCH 3/6] drm/vc4: Fix handling of a pm_runtime_get_sync() success case.

[Eric Anholt]

If the device was already up, a 1 is returned instead of 0.  We were
erroring out, leading the 3D driver to sometimes fail at screen
initialization (generally with ENOENT returned to it).

Signed-off-by: Eric Anholt <eric@anholt.net>
Fixes: af713795c59f ("drm/vc4: Add a getparam ioctl for getting the V3D identity regs.")
---
 drivers/gpu/drm/vc4/vc4_drv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 9435894822d5..fb20e03ec7e2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -57,21 +57,21 @@ static int vc4_get_param_ioctl(struct drm_device *dev, void *data,
 	switch (args->param) {
 	case DRM_VC4_PARAM_V3D_IDENT0:
 		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret)
+		if (ret < 0)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT0);
 		pm_runtime_put(&vc4->v3d->pdev->dev);
 		break;
 	case DRM_VC4_PARAM_V3D_IDENT1:
 		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret)
+		if (ret < 0)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT1);
 		pm_runtime_put(&vc4->v3d->pdev->dev);
 		break;
 	case DRM_VC4_PARAM_V3D_IDENT2:
 		ret = pm_runtime_get_sync(&vc4->v3d->pdev->dev);
-		if (ret)
+		if (ret < 0)
 			return ret;
 		args->value = V3D_READ(V3D_IDENT2);
 		pm_runtime_put(&vc4->v3d->pdev->dev);
-- 
2.8.1

[PATCH 4/6] drm/vc4: Free hang state before destroying BO cache.

[Eric Anholt]

The BO cache will complain if BOs are still allocated when we try to
destroy it (since freeing those BOs would try to hit the cache).  You
could hit this if you were to unload the module after a GPU hang.

Signed-off-by: Eric Anholt <eric@anholt.net>
Fixes: 214613656b51 ("drm/vc4: Add an interface for capturing the GPU state after a hang.")
---
 drivers/gpu/drm/vc4/vc4_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index fb2b1c526646..3f60036834e0 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -947,8 +947,8 @@ vc4_gem_destroy(struct drm_device *dev)
 		vc4->overflow_mem = NULL;
 	}
 
-	vc4_bo_cache_destroy(dev);
-
 	if (vc4->hang_state)
 		vc4_free_hang_state(dev, vc4->hang_state);
+
+	vc4_bo_cache_destroy(dev);
 }
-- 
2.8.1

[PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Eric Anholt]

Overflow memory handling is tricky: While it's still referenced by the
BPO registers, we want to keep it from being freed.  When we are
putting a new set of overflow memory in the registers, we need to
assign the old one to the last rendering job using it.

We were looking at "what's currently running in the binner", but since
the bin/render submission split, we may end up with the binner
completing and having no new job while the renderer is still
processing.  So, if we don't find a bin job at all, look at the
highest-seqno (last) render job to attach our overflow to.

Signed-off-by: Eric Anholt <eric@anholt.net>
Fixes: ca26d28bbaa3 ("drm/vc4: improve throughput by pipelining binning and rendering jobs")
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/vc4/vc4_drv.h | 9 +++++++++
 drivers/gpu/drm/vc4/vc4_irq.c | 4 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 0ced289d7696..87f727932af2 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -321,6 +321,15 @@ vc4_first_render_job(struct vc4_dev *vc4)
 				struct vc4_exec_info, head);
 }
 
+static inline struct vc4_exec_info *
+vc4_last_render_job(struct vc4_dev *vc4)
+{
+	if (list_empty(&vc4->render_job_list))
+		return NULL;
+	return list_last_entry(&vc4->render_job_list,
+			       struct vc4_exec_info, head);
+}
+
 /**
  * struct vc4_texture_sample_info - saves the offsets into the UBO for texture
  * setup parameters.
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index b0104a346a74..094bc6a475c1 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -83,8 +83,10 @@ vc4_overflow_mem_work(struct work_struct *work)
 
 		spin_lock_irqsave(&vc4->job_lock, irqflags);
 		current_exec = vc4_first_bin_job(vc4);
+		if (!current_exec)
+			current_exec = vc4_last_render_job(vc4);
 		if (current_exec) {
-			vc4->overflow_mem->seqno = vc4->finished_seqno + 1;
+			vc4->overflow_mem->seqno = current_exec->seqno;
 			list_add_tail(&vc4->overflow_mem->unref_head,
 				      &current_exec->unref_list);
 			vc4->overflow_mem = NULL;
-- 
2.8.1

Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Rob Clark]

On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
> Overflow memory handling is tricky: While it's still referenced by the
> BPO registers, we want to keep it from being freed.  When we are
> putting a new set of overflow memory in the registers, we need to
> assign the old one to the last rendering job using it.
>
> We were looking at "what's currently running in the binner", but since
> the bin/render submission split, we may end up with the binner
> completing and having no new job while the renderer is still
> processing.  So, if we don't find a bin job at all, look at the
> highest-seqno (last) render job to attach our overflow to.

so, drive-by comment.. but can you allocate gem bo's without backing
them immediately with pages?  If so, just always allocate the bo
up-front and attach it as a dependency of the batch, and only pin it
to actual pages when you have to overflow?

BR,
-R

> Signed-off-by: Eric Anholt <eric@anholt.net>
> Fixes: ca26d28bbaa3 ("drm/vc4: improve throughput by pipelining binning and rendering jobs")
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/vc4/vc4_drv.h | 9 +++++++++
>  drivers/gpu/drm/vc4/vc4_irq.c | 4 +++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 0ced289d7696..87f727932af2 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -321,6 +321,15 @@ vc4_first_render_job(struct vc4_dev *vc4)
>                                 struct vc4_exec_info, head);
>  }
>
> +static inline struct vc4_exec_info *
> +vc4_last_render_job(struct vc4_dev *vc4)
> +{
> +       if (list_empty(&vc4->render_job_list))
> +               return NULL;
> +       return list_last_entry(&vc4->render_job_list,
> +                              struct vc4_exec_info, head);
> +}
> +
>  /**
>   * struct vc4_texture_sample_info - saves the offsets into the UBO for texture
>   * setup parameters.
> diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
> index b0104a346a74..094bc6a475c1 100644
> --- a/drivers/gpu/drm/vc4/vc4_irq.c
> +++ b/drivers/gpu/drm/vc4/vc4_irq.c
> @@ -83,8 +83,10 @@ vc4_overflow_mem_work(struct work_struct *work)
>
>                 spin_lock_irqsave(&vc4->job_lock, irqflags);
>                 current_exec = vc4_first_bin_job(vc4);
> +               if (!current_exec)
> +                       current_exec = vc4_last_render_job(vc4);
>                 if (current_exec) {
> -                       vc4->overflow_mem->seqno = vc4->finished_seqno + 1;
> +                       vc4->overflow_mem->seqno = current_exec->seqno;
>                         list_add_tail(&vc4->overflow_mem->unref_head,
>                                       &current_exec->unref_list);
>                         vc4->overflow_mem = NULL;
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

Rob Clark <robdclark@gmail.com> writes:

> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
>> Overflow memory handling is tricky: While it's still referenced by the
>> BPO registers, we want to keep it from being freed.  When we are
>> putting a new set of overflow memory in the registers, we need to
>> assign the old one to the last rendering job using it.
>>
>> We were looking at "what's currently running in the binner", but since
>> the bin/render submission split, we may end up with the binner
>> completing and having no new job while the renderer is still
>> processing.  So, if we don't find a bin job at all, look at the
>> highest-seqno (last) render job to attach our overflow to.
>
> so, drive-by comment.. but can you allocate gem bo's without backing
> them immediately with pages?  If so, just always allocate the bo
> up-front and attach it as a dependency of the batch, and only pin it
> to actual pages when you have to overflow?

The amount of overflow for a given CL is arbitrary, depending on the
geometry submitted, and the overflow pool just gets streamed into by the
hardware as you submit bin jobs.  You'll end up allocating [0,n] new
overflows per bin job.  I don't see where "allocate gem BOs without
backing them immediately with pages" idea would fit into this.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Rob Clark]

On Tue, Jul 26, 2016 at 7:11 PM, Eric Anholt <eric@anholt.net> wrote:
> Rob Clark <robdclark@gmail.com> writes:
>
>> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Overflow memory handling is tricky: While it's still referenced by the
>>> BPO registers, we want to keep it from being freed.  When we are
>>> putting a new set of overflow memory in the registers, we need to
>>> assign the old one to the last rendering job using it.
>>>
>>> We were looking at "what's currently running in the binner", but since
>>> the bin/render submission split, we may end up with the binner
>>> completing and having no new job while the renderer is still
>>> processing.  So, if we don't find a bin job at all, look at the
>>> highest-seqno (last) render job to attach our overflow to.
>>
>> so, drive-by comment.. but can you allocate gem bo's without backing
>> them immediately with pages?  If so, just always allocate the bo
>> up-front and attach it as a dependency of the batch, and only pin it
>> to actual pages when you have to overflow?
>
> The amount of overflow for a given CL is arbitrary, depending on the
> geometry submitted, and the overflow pool just gets streamed into by the
> hardware as you submit bin jobs.  You'll end up allocating [0,n] new
> overflows per bin job.  I don't see where "allocate gem BOs without
> backing them immediately with pages" idea would fit into this.

well, even not knowing the size up front shouldn't really be a
show-stopper, unless you had to mmap it to userspace, perhaps..
normally backing pages aren't allocated until drm_gem_get_pages() so
allocating the gem bo as placeholder to track dependencies of the
batch/submit shouldn't be an issue.  But I noticed you don't use
drm_gem_get_pages().. maybe w/ cma helpers it is harder to decouple
allocation of the drm_gem_object from the backing store.

BR,
-R

Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 2860 bytes --]

Rob Clark <robdclark@gmail.com> writes:

> On Tue, Jul 26, 2016 at 7:11 PM, Eric Anholt <eric@anholt.net> wrote:
>> Rob Clark <robdclark@gmail.com> writes:
>>
>>> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
>>>> Overflow memory handling is tricky: While it's still referenced by the
>>>> BPO registers, we want to keep it from being freed.  When we are
>>>> putting a new set of overflow memory in the registers, we need to
>>>> assign the old one to the last rendering job using it.
>>>>
>>>> We were looking at "what's currently running in the binner", but since
>>>> the bin/render submission split, we may end up with the binner
>>>> completing and having no new job while the renderer is still
>>>> processing.  So, if we don't find a bin job at all, look at the
>>>> highest-seqno (last) render job to attach our overflow to.
>>>
>>> so, drive-by comment.. but can you allocate gem bo's without backing
>>> them immediately with pages?  If so, just always allocate the bo
>>> up-front and attach it as a dependency of the batch, and only pin it
>>> to actual pages when you have to overflow?
>>
>> The amount of overflow for a given CL is arbitrary, depending on the
>> geometry submitted, and the overflow pool just gets streamed into by the
>> hardware as you submit bin jobs.  You'll end up allocating [0,n] new
>> overflows per bin job.  I don't see where "allocate gem BOs without
>> backing them immediately with pages" idea would fit into this.
>
> well, even not knowing the size up front shouldn't really be a
> show-stopper, unless you had to mmap it to userspace, perhaps..
> normally backing pages aren't allocated until drm_gem_get_pages() so
> allocating the gem bo as placeholder to track dependencies of the
> batch/submit shouldn't be an issue.  But I noticed you don't use
> drm_gem_get_pages().. maybe w/ cma helpers it is harder to decouple
> allocation of the drm_gem_object from the backing store.

There's no period of time between "I need to allocate an overflow BO"
and "I need pages in the BO", though.

I could have a different setup that allocated a massive (all of CMA?),
fresh overflow BO per CL and populated page ranges in it as I overflow,
but with CMA you really need to never do new allocations in the hot path
because you get to stop and wait approximately forever.  So you'd want
to chunk it up so you could cache the groups of contiguous pages of
overflow, and it turns out we already have a thing for this in the form
of GEM BOs.  Anyway, doing that that means you're losing out on the rest
of the last overflow BO for the new CL, expanding the working set in
your precious 256MB CMA area.

Well, OK, actually I *do* allocate a fresh overflow BO per CL today,
because of leftover bringup code that I think I could just delete at
this point.  I'm not doing that in a -fixes commit, though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry.

[Rob Clark]

On Wed, Jul 27, 2016 at 1:37 AM, Eric Anholt <eric@anholt.net> wrote:
> Rob Clark <robdclark@gmail.com> writes:
>
>> On Tue, Jul 26, 2016 at 7:11 PM, Eric Anholt <eric@anholt.net> wrote:
>>> Rob Clark <robdclark@gmail.com> writes:
>>>
>>>> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt <eric@anholt.net> wrote:
>>>>> Overflow memory handling is tricky: While it's still referenced by the
>>>>> BPO registers, we want to keep it from being freed.  When we are
>>>>> putting a new set of overflow memory in the registers, we need to
>>>>> assign the old one to the last rendering job using it.
>>>>>
>>>>> We were looking at "what's currently running in the binner", but since
>>>>> the bin/render submission split, we may end up with the binner
>>>>> completing and having no new job while the renderer is still
>>>>> processing.  So, if we don't find a bin job at all, look at the
>>>>> highest-seqno (last) render job to attach our overflow to.
>>>>
>>>> so, drive-by comment.. but can you allocate gem bo's without backing
>>>> them immediately with pages?  If so, just always allocate the bo
>>>> up-front and attach it as a dependency of the batch, and only pin it
>>>> to actual pages when you have to overflow?
>>>
>>> The amount of overflow for a given CL is arbitrary, depending on the
>>> geometry submitted, and the overflow pool just gets streamed into by the
>>> hardware as you submit bin jobs.  You'll end up allocating [0,n] new
>>> overflows per bin job.  I don't see where "allocate gem BOs without
>>> backing them immediately with pages" idea would fit into this.
>>
>> well, even not knowing the size up front shouldn't really be a
>> show-stopper, unless you had to mmap it to userspace, perhaps..
>> normally backing pages aren't allocated until drm_gem_get_pages() so
>> allocating the gem bo as placeholder to track dependencies of the
>> batch/submit shouldn't be an issue.  But I noticed you don't use
>> drm_gem_get_pages().. maybe w/ cma helpers it is harder to decouple
>> allocation of the drm_gem_object from the backing store.
>
> There's no period of time between "I need to allocate an overflow BO"
> and "I need pages in the BO", though.

oh, ok, so this is some memory that is already being used by the GPU,
not something that starts to be used when you hit overflow condition..
I'd assumed it was something you were allocating in response to the
overflow irq, but looks like you are actually *re*allocating.

BR,
-R

> I could have a different setup that allocated a massive (all of CMA?),
> fresh overflow BO per CL and populated page ranges in it as I overflow,
> but with CMA you really need to never do new allocations in the hot path
> because you get to stop and wait approximately forever.  So you'd want
> to chunk it up so you could cache the groups of contiguous pages of
> overflow, and it turns out we already have a thing for this in the form
> of GEM BOs.  Anyway, doing that that means you're losing out on the rest
> of the last overflow BO for the new CL, expanding the working set in
> your precious 256MB CMA area.
>
> Well, OK, actually I *do* allocate a fresh overflow BO per CL today,
> because of leftover bringup code that I think I could just delete at
> this point.  I'm not doing that in a -fixes commit, though.

[PATCH 6/6] drm/vc4: Fix oops when userspace hands in a bad BO.

[Eric Anholt]

We'd end up NULL pointer dereferencing because we didn't take the
error path out in the parent.  Fixes igt vc4_lookup_fail test.

Signed-off-by: Eric Anholt <eric@anholt.net>
Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/vc4/vc4_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index 3f60036834e0..9da024fffbcf 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -575,7 +575,7 @@ vc4_cl_lookup_bos(struct drm_device *dev,
 
 fail:
 	drm_free_large(handles);
-	return 0;
+	return ret;
 }
 
 static int
-- 
2.8.1