2022-12-05 14:23:52

by Melissa Wen

[permalink] [raw]

Subject: [PATCH v2 2/2] drm/v3d: replace obj lookup steps with drm_gem_objects_lookup

---

As v3d_lookup_bos() performs the same steps as drm_gem_objects_lookup(),
replace the explicit code in v3d to simply use the DRM function.

Signed-off-by: Melissa Wen <[email protected]>
---
drivers/gpu/drm/v3d/v3d_gem.c | 49 +++--------------------------------
1 file changed, 3 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 31a37572c11d..6e152ef26358 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -299,10 +299,6 @@ v3d_lookup_bos(struct drm_device *dev,
u64 bo_handles,
u32 bo_count)
{
- u32 *handles;
- int ret = 0;
- int i;
-
job->bo_count = bo_count;

if (!job->bo_count) {
@@ -313,48 +309,9 @@ v3d_lookup_bos(struct drm_device *dev,
return -EINVAL;
}

- job->bo = kvmalloc_array(job->bo_count,
- sizeof(struct drm_gem_dma_object *),
- GFP_KERNEL | __GFP_ZERO);
- if (!job->bo) {
- DRM_DEBUG("Failed to allocate validated BO pointers\n");
- return -ENOMEM;
- }
-
- handles = kvmalloc_array(job->bo_count, sizeof(u32), GFP_KERNEL);
- if (!handles) {
- ret = -ENOMEM;
- DRM_DEBUG("Failed to allocate incoming GEM handles\n");
- goto fail;
- }
-
- if (copy_from_user(handles,
- (void __user *)(uintptr_t)bo_handles,
- job->bo_count * sizeof(u32))) {
- ret = -EFAULT;
- DRM_DEBUG("Failed to copy in GEM handles\n");
- goto fail;
- }
-
- spin_lock(&file_priv->table_lock);
- for (i = 0; i < job->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 = -ENOENT;
- spin_unlock(&file_priv->table_lock);
- goto fail;
- }
- drm_gem_object_get(bo);
- job->bo[i] = bo;
- }
- spin_unlock(&file_priv->table_lock);
-
-fail:
- kvfree(handles);
- return ret;
+ return drm_gem_objects_lookup(file_priv,
+ (void __user *)(uintptr_t)bo_handles,
+ job->bo_count, &job->bo);
}

static void
--
2.35.1

2022-12-06 01:07:27

by Maíra Canal

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] drm/v3d: replace obj lookup steps with drm_gem_objects_lookup

---

On 12/5/22 10:55, Melissa Wen wrote:
> As v3d_lookup_bos() performs the same steps as drm_gem_objects_lookup(),
> replace the explicit code in v3d to simply use the DRM function.
>
> Signed-off-by: Melissa Wen <[email protected]>

Reviewed-by: Maíra Canal <[email protected]>

Best Regards,
- Maíra Canal

> ---
> drivers/gpu/drm/v3d/v3d_gem.c | 49 +++--------------------------------
> 1 file changed, 3 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index 31a37572c11d..6e152ef26358 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -299,10 +299,6 @@ v3d_lookup_bos(struct drm_device *dev,
> u64 bo_handles,
> u32 bo_count)
> {
> - u32 *handles;
> - int ret = 0;
> - int i;
> -
> job->bo_count = bo_count;
>
> if (!job->bo_count) {
> @@ -313,48 +309,9 @@ v3d_lookup_bos(struct drm_device *dev,
> return -EINVAL;
> }
>
> - job->bo = kvmalloc_array(job->bo_count,
> - sizeof(struct drm_gem_dma_object *),
> - GFP_KERNEL | __GFP_ZERO);
> - if (!job->bo) {
> - DRM_DEBUG("Failed to allocate validated BO pointers\n");
> - return -ENOMEM;
> - }
> -
> - handles = kvmalloc_array(job->bo_count, sizeof(u32), GFP_KERNEL);
> - if (!handles) {
> - ret = -ENOMEM;
> - DRM_DEBUG("Failed to allocate incoming GEM handles\n");
> - goto fail;
> - }
> -
> - if (copy_from_user(handles,
> - (void __user *)(uintptr_t)bo_handles,
> - job->bo_count * sizeof(u32))) {
> - ret = -EFAULT;
> - DRM_DEBUG("Failed to copy in GEM handles\n");
> - goto fail;
> - }
> -
> - spin_lock(&file_priv->table_lock);
> - for (i = 0; i < job->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 = -ENOENT;
> - spin_unlock(&file_priv->table_lock);
> - goto fail;
> - }
> - drm_gem_object_get(bo);
> - job->bo[i] = bo;
> - }
> - spin_unlock(&file_priv->table_lock);
> -
> -fail:
> - kvfree(handles);
> - return ret;
> + return drm_gem_objects_lookup(file_priv,
> + (void __user *)(uintptr_t)bo_handles,
> + job->bo_count, &job->bo);
> }
>
> static void

2022-12-19 00:27:42

by Melissa Wen

[permalink] [raw]

Subject: Re: [PATCH v2 2/2] drm/v3d: replace obj lookup steps with drm_gem_objects_lookup

---

On 12/05, Ma?ra Canal wrote:
> On 12/5/22 10:55, Melissa Wen wrote:
> > As v3d_lookup_bos() performs the same steps as drm_gem_objects_lookup(),
> > replace the explicit code in v3d to simply use the DRM function.
> >
> > Signed-off-by: Melissa Wen <[email protected]>
>
> Reviewed-by: Ma?ra Canal <[email protected]>

Applied this series to drm-misc-next.

Thanks,

Melissa

>
> Best Regards,
> - Ma?ra Canal
>
> > ---
> > drivers/gpu/drm/v3d/v3d_gem.c | 49 +++--------------------------------
> > 1 file changed, 3 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> > index 31a37572c11d..6e152ef26358 100644
> > --- a/drivers/gpu/drm/v3d/v3d_gem.c
> > +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> > @@ -299,10 +299,6 @@ v3d_lookup_bos(struct drm_device *dev,
> > u64 bo_handles,
> > u32 bo_count)
> > {
> > - u32 *handles;
> > - int ret = 0;
> > - int i;
> > -
> > job->bo_count = bo_count;
> >
> > if (!job->bo_count) {
> > @@ -313,48 +309,9 @@ v3d_lookup_bos(struct drm_device *dev,
> > return -EINVAL;
> > }
> >
> > - job->bo = kvmalloc_array(job->bo_count,
> > - sizeof(struct drm_gem_dma_object *),
> > - GFP_KERNEL | __GFP_ZERO);
> > - if (!job->bo) {
> > - DRM_DEBUG("Failed to allocate validated BO pointers\n");
> > - return -ENOMEM;
> > - }
> > -
> > - handles = kvmalloc_array(job->bo_count, sizeof(u32), GFP_KERNEL);
> > - if (!handles) {
> > - ret = -ENOMEM;
> > - DRM_DEBUG("Failed to allocate incoming GEM handles\n");
> > - goto fail;
> > - }
> > -
> > - if (copy_from_user(handles,
> > - (void __user *)(uintptr_t)bo_handles,
> > - job->bo_count * sizeof(u32))) {
> > - ret = -EFAULT;
> > - DRM_DEBUG("Failed to copy in GEM handles\n");
> > - goto fail;
> > - }
> > -
> > - spin_lock(&file_priv->table_lock);
> > - for (i = 0; i < job->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 = -ENOENT;
> > - spin_unlock(&file_priv->table_lock);
> > - goto fail;
> > - }
> > - drm_gem_object_get(bo);
> > - job->bo[i] = bo;
> > - }
> > - spin_unlock(&file_priv->table_lock);
> > -
> > -fail:
> > - kvfree(handles);
> > - return ret;
> > + return drm_gem_objects_lookup(file_priv,
> > + (void __user *)(uintptr_t)bo_handles,
> > + job->bo_count, &job->bo);
> > }
> >
> > static void

---

Attachments:

(No filename) (2.55 kB)
signature.asc (849.00 B)
Download all attachments