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

Hi Andrzej,

On Tue, 3 Mar 2020 at 12:01, Andrzej Pietrasiewicz andrzej.p@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