| Message ID | 20230102124535.139202-3-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Improve drm_gem_object handling | expand |
Hi Maíra, Thank you for your patch, it really simplifies the code. I think it's OK as it is, so I'll give my Reviewed-by: André Almeida <andrealmeid@igalia.com> for this series. But I would also like to notice that VC4 had some extra DRM_ log functions that the DRM version hasn't, so I think they would be a good addition in a different patch. On 1/2/23 09:45, Maíra Canal wrote: > As vc4_cl_lookup_bos() performs the same steps as drm_gem_objects_lookup(), > replace the open-coded implementation in vc4 to simply use the DRM function. > > Signed-off-by: Maíra Canal <mcanal@igalia.com> > --- > drivers/gpu/drm/vc4/vc4_gem.c | 43 ++--------------------------------- > 1 file changed, 2 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c > index d6985d067e34..03648f954985 100644 > --- a/drivers/gpu/drm/vc4/vc4_gem.c > +++ b/drivers/gpu/drm/vc4/vc4_gem.c > @@ -746,7 +746,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, > struct vc4_exec_info *exec) > { > struct drm_vc4_submit_cl *args = exec->args; > - uint32_t *handles; > int ret = 0; > int i; > > @@ -760,43 +759,8 @@ vc4_cl_lookup_bos(struct drm_device *dev, > return -EINVAL; > } > > - exec->bo = kvmalloc_array(exec->bo_count, > - sizeof(struct drm_gem_dma_object *), > - GFP_KERNEL | __GFP_ZERO); > - if (!exec->bo) { > - DRM_ERROR("Failed to allocate validated BO pointers\n"); > - return -ENOMEM; > - } > - > - handles = kvmalloc_array(exec->bo_count, sizeof(uint32_t), GFP_KERNEL); > - if (!handles) { > - ret = -ENOMEM; > - DRM_ERROR("Failed to allocate incoming GEM handles\n"); > - goto fail; > - } > - > - if (copy_from_user(handles, u64_to_user_ptr(args->bo_handles), > - exec->bo_count * sizeof(uint32_t))) { > - ret = -EFAULT; > - DRM_ERROR("Failed to copy in GEM handles\n"); > - goto fail; > - } > - > - spin_lock(&file_priv->table_lock); > - for (i = 0; i < exec->bo_count; i++) { > - struct drm_gem_object *bo = idr_find(&file_priv->object_idr, > - handles[i]); > - if (!bo) { > - DRM_DEBUG("Failed to look up GEM BO %d: %d\n", > - i, handles[i]); > - ret = -EINVAL; > - break; > - } > - > - drm_gem_object_get(bo); > - exec->bo[i] = bo; > - } > - spin_unlock(&file_priv->table_lock); > + ret = drm_gem_objects_lookup(file_priv, u64_to_user_ptr(args->bo_handles), > + exec->bo_count, &exec->bo); > > if (ret) > goto fail_put_bo; > @@ -807,7 +771,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, > goto fail_dec_usecnt; > } > > - kvfree(handles); > return 0; > > fail_dec_usecnt: > @@ -827,8 +790,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, > for (i = 0; i < exec->bo_count && exec->bo[i]; i++) > drm_gem_object_put(exec->bo[i]); > > -fail: > - kvfree(handles); > kvfree(exec->bo); > exec->bo = NULL; > return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index d6985d067e34..03648f954985 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -746,7 +746,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, struct vc4_exec_info *exec) { struct drm_vc4_submit_cl *args = exec->args; - uint32_t *handles; int ret = 0; int i; @@ -760,43 +759,8 @@ vc4_cl_lookup_bos(struct drm_device *dev, return -EINVAL; } - exec->bo = kvmalloc_array(exec->bo_count, - sizeof(struct drm_gem_dma_object *), - GFP_KERNEL | __GFP_ZERO); - if (!exec->bo) { - DRM_ERROR("Failed to allocate validated BO pointers\n"); - return -ENOMEM; - } - - handles = kvmalloc_array(exec->bo_count, sizeof(uint32_t), GFP_KERNEL); - if (!handles) { - ret = -ENOMEM; - DRM_ERROR("Failed to allocate incoming GEM handles\n"); - goto fail; - } - - if (copy_from_user(handles, u64_to_user_ptr(args->bo_handles), - exec->bo_count * sizeof(uint32_t))) { - ret = -EFAULT; - DRM_ERROR("Failed to copy in GEM handles\n"); - goto fail; - } - - spin_lock(&file_priv->table_lock); - for (i = 0; i < exec->bo_count; i++) { - struct drm_gem_object *bo = idr_find(&file_priv->object_idr, - handles[i]); - if (!bo) { - DRM_DEBUG("Failed to look up GEM BO %d: %d\n", - i, handles[i]); - ret = -EINVAL; - break; - } - - drm_gem_object_get(bo); - exec->bo[i] = bo; - } - spin_unlock(&file_priv->table_lock); + ret = drm_gem_objects_lookup(file_priv, u64_to_user_ptr(args->bo_handles), + exec->bo_count, &exec->bo); if (ret) goto fail_put_bo; @@ -807,7 +771,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, goto fail_dec_usecnt; } - kvfree(handles); return 0; fail_dec_usecnt: @@ -827,8 +790,6 @@ vc4_cl_lookup_bos(struct drm_device *dev, for (i = 0; i < exec->bo_count && exec->bo[i]; i++) drm_gem_object_put(exec->bo[i]); -fail: - kvfree(handles); kvfree(exec->bo); exec->bo = NULL; return ret;
As vc4_cl_lookup_bos() performs the same steps as drm_gem_objects_lookup(), replace the open-coded implementation in vc4 to simply use the DRM function. Signed-off-by: Maíra Canal <mcanal@igalia.com> --- drivers/gpu/drm/vc4/vc4_gem.c | 43 ++--------------------------------- 1 file changed, 2 insertions(+), 41 deletions(-)