[PATCHv6 3/6] drm/arm/malidp: Factor-in framebuffer creation

[Emil Velikov]

Hi Andrzej,

On Tue, 3 Mar 2020 at 12:01, Andrzej Pietrasiewicz
<andrzej.p at collabora.com> wrote:
>
> Consolidating framebuffer creation into one function will make it easier
> to transition to generic afbc-aware helpers.
>
I'd suggest keeping the refactor a bit simpler.
Say - first folds the functions together. Then do the modifier[0] reshuffle.

As-is the patch seems to introduce a leak:


> +               objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
> +               if (!objs) {
> +                       DRM_DEBUG_KMS("Failed to lookup GEM object\n");
> +                       return ERR_PTR(-EINVAL);
> +               }
>
> -       objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
> -       if (!objs) {
> -               DRM_DEBUG_KMS("Failed to lookup GEM object\n");
> -               return false;
> -       }
> +               if (objs->size < afbc_size) {
> +                       DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
> +                                     objs->size, afbc_size);
> +                       drm_gem_object_put_unlocked(objs);
> +                       return ERR_PTR(-EINVAL);
> +               }
>
We're missing the drm_gem_object_put_unlocked() after the if block.

> -       if (objs->size < afbc_size) {
> -               DRM_DEBUG_KMS("buffer size (%zu) too small for AFBC buffer size = %u\n",
> -                             objs->size, afbc_size);
>                 drm_gem_object_put_unlocked(objs);
> -               return false;
> -       }
> -
> -       drm_gem_object_put_unlocked(objs);
> -
... namely this one ^^.


-Emil