Re: [PATCH v2 5/11] drm: malidp: Use devm_of_reserved_mem_device_init()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Permanent CMA capacity leak due to clearing dev->cma_area before GEM objects are freed during device unbind.
--

commit a1f947cbe50ede0e9b650dcb9e31cc36cb85952d
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

drm: malidp: Use devm_of_reserved_mem_device_init()

This commit simplifies the driver by transitioning to the devres-managed
devm_of_reserved_mem_device_init(), removing manual cleanup calls during
bind failures and unbind.

> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 23fa942ae4bbe..3e17513c31147 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -758,7 +758,7 @@ static int malidp_bind(struct device *dev)
>  		return PTR_ERR(hwdev->pxlclk);
>  
>  	/* Get the optional framebuffer memory resource */
> -	ret = of_reserved_mem_device_init(dev);
> +	ret = devm_of_reserved_mem_device_init(dev);

[Severity: High]
This isn't a bug introduced by this patch, but does this devres conversion
preserve a pre-existing CMA memory capacity leak during device unbind?

In malidp_bind(), devm_drm_dev_alloc() is called before this initialization
function. Because devres actions execute in reverse order of allocation, the
cleanup action registered here will execute before the drm_dev_put() action.

When malidp_unbind() returns, the component framework immediately releases
the component's devres group, clearing dev->cma_area. Later, when the DRM
device is freed (either immediately or when userspace eventually closes its
file descriptors), drm_gem_object_free() calls dma_free_contiguous().

Because dev->cma_area is already NULL at that point, it bypasses
cma_release() and falls back to the buddy allocator, permanently leaking
the CMA region's bitmap capacity.

The previous code suffered from this exact same ordering issue since the
manual of_reserved_mem_device_release() call happened before drm_dev_put()
ran. Is there a way to adjust the cleanup order so the CMA area outlives
the GEM objects?

>  	if (ret && ret != -ENODEV)
>  		return ret;
>  

[ ... ]
> @@ -913,7 +912,6 @@ static void malidp_unbind(struct device *dev)
>  	else
>  		malidp_runtime_pm_suspend(dev);
>  	dev_set_drvdata(dev, NULL);
> -	of_reserved_mem_device_release(dev);
>  }
>  
>  static const struct component_master_ops malidp_master_ops = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com?part=5