[PATCH 0/3] three fixes for mtk-mdp3

[PATCH 0/3] three fixes for mtk-mdp3

[Sun Ke]

three fixes for mtk-mdp3

Sun Ke (3):
  media: platform: mtk-mdp3: use devm_kfree to free memory allocated
    with devm_kmalloc()
  media: platform: mtk-mdp3: fix PM reference leak in
    mdp_comp_clock_on()
  media: platform: mtk-mdp3: fix error return code in mdp_vpu_dev_init()

 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c | 7 ++++---
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c  | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.31.1

[PATCH 1/3] media: platform: mtk-mdp3: use devm_kfree to free memory allocated with devm_kmalloc()

[Sun Ke]

comp is allocated with devm_kmalloc(), use devm_kfree to free it.

Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
Signed-off-by: Sun Ke <sunke32@huawei.com>
---
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
index e62abf3587bf..9a6ba5851ccb 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
@@ -869,7 +869,7 @@ static struct mdp_comp *mdp_comp_create(struct mdp_dev *mdp,
 
 	ret = mdp_comp_init(mdp, node, comp, id);
 	if (ret) {
-		kfree(comp);
+		devm_kfree(dev, comp);
 		return ERR_PTR(ret);
 	}
 	mdp->comp[id] = comp;
@@ -930,7 +930,7 @@ void mdp_comp_destroy(struct mdp_dev *mdp)
 		if (mdp->comp[i]) {
 			pm_runtime_disable(mdp->comp[i]->comp_dev);
 			mdp_comp_deinit(mdp->comp[i]);
-			kfree(mdp->comp[i]);
+			devm_kfree(mdp->comp[i]->comp_dev, mdp->comp[i]);
 			mdp->comp[i] = NULL;
 		}
 	}
-- 
2.31.1

[PATCH 2/3] media: platform: mtk-mdp3: fix PM reference leak in mdp_comp_clock_on()

[Sun Ke]

mdp_comp_clock_on will increase runtime PM usage counter,
and mdp_comp_clock_off will decrease the runtime PM usage counter.
so, if mdp_comp_clock_on failed after increment runtime PM usage
counter, it should decrease it before return a error code.

pm_runtime_get_sync will increment pm usage counter even it failed.
Forgetting to putting operation will result in reference leak here.
Fix it by replacing it with pm_runtime_resume_and_get to keep usage
counter balanced.

And if failed to enable clk, add pm_runtime_put() to decrease the
runtime PM usage counter.

Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
Signed-off-by: Sun Ke <sunke32@huawei.com>
---
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
index 9a6ba5851ccb..d3eaf8884412 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
@@ -682,7 +682,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
 	int i, ret;
 
 	if (comp->comp_dev) {
-		ret = pm_runtime_get_sync(comp->comp_dev);
+		ret = pm_runtime_resume_and_get(comp->comp_dev);
 		if (ret < 0) {
 			dev_err(dev,
 				"Failed to get power, err %d. type:%d id:%d\n",
@@ -699,6 +699,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
 			dev_err(dev,
 				"Failed to enable clk %d. type:%d id:%d\n",
 				i, comp->type, comp->id);
+			pm_runtime_put(comp->comp_dev);
 			return ret;
 		}
 	}
-- 
2.31.1

[PATCH 3/3] media: platform: mtk-mdp3: fix error return code in mdp_vpu_dev_init()

[Sun Ke]

If mdp_vpu_shared_mem_alloc failed, mdp_vpu_dev_init should return -ENOMEM.

Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
Signed-off-by: Sun Ke <sunke32@huawei.com>
---
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
index 9f5844385c8f..078040b7f65e 100644
--- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
+++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
@@ -175,6 +175,7 @@ int mdp_vpu_dev_init(struct mdp_vpu_dev *vpu, struct mtk_scp *scp,
 	mem_size = vpu_alloc_size;
 	if (mdp_vpu_shared_mem_alloc(vpu)) {
 		dev_err(&mdp->pdev->dev, "VPU memory alloc fail!");
+		err = -ENOMEM;
 		goto err_mem_alloc;
 	}
 
-- 
2.31.1

Re: [PATCH 1/3] media: platform: mtk-mdp3: use devm_kfree to free memory allocated with devm_kmalloc()

[AngeloGioacchino Del Regno]

Il 02/09/22 10:58, Sun Ke ha scritto:
> comp is allocated with devm_kmalloc(), use devm_kfree to free it.
> 
> Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
> Signed-off-by: Sun Ke <sunke32@huawei.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 3/3] media: platform: mtk-mdp3: fix error return code in mdp_vpu_dev_init()

[AngeloGioacchino Del Regno]

Il 02/09/22 10:58, Sun Ke ha scritto:
> If mdp_vpu_shared_mem_alloc failed, mdp_vpu_dev_init should return -ENOMEM.
> 
> Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
> Signed-off-by: Sun Ke <sunke32@huawei.com>

Hello Sun,
thanks for the patch! However, someone else already sent the same fix and it's
in a better shape. Please look at [1].

Thanks,
Angelo

[1]: https://patchwork.kernel.org/project/linux-mediatek/patch/YxDGFMwyeNXFPaig@kili/

> ---
>   drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
> index 9f5844385c8f..078040b7f65e 100644
> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
> @@ -175,6 +175,7 @@ int mdp_vpu_dev_init(struct mdp_vpu_dev *vpu, struct mtk_scp *scp,
>   	mem_size = vpu_alloc_size;
>   	if (mdp_vpu_shared_mem_alloc(vpu)) {
>   		dev_err(&mdp->pdev->dev, "VPU memory alloc fail!");
> +		err = -ENOMEM;
>   		goto err_mem_alloc;
>   	}
>   
> 

Re: [PATCH 3/3] media: platform: mtk-mdp3: fix error return code in mdp_vpu_dev_init()

[Sun Ke]

在 2022/9/2 17:10, AngeloGioacchino Del Regno 写道:
> Il 02/09/22 10:58, Sun Ke ha scritto:
>> If mdp_vpu_shared_mem_alloc failed, mdp_vpu_dev_init should return 
>> -ENOMEM.
>>
>> Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 
>> driver")
>> Signed-off-by: Sun Ke <sunke32@huawei.com>
> 
> Hello Sun,
> thanks for the patch! However, someone else already sent the same fix 
> and it's
> in a better shape. Please look at [1].
> 
> Thanks,
> Angelo
> 
> [1]: 
> https://patchwork.kernel.org/project/linux-mediatek/patch/YxDGFMwyeNXFPaig@kili/ 

Oh. I am late.

Thanks,
Sun Ke

> 
> 
>> ---
>>   drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c 
>> b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
>> index 9f5844385c8f..078040b7f65e 100644
>> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
>> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
>> @@ -175,6 +175,7 @@ int mdp_vpu_dev_init(struct mdp_vpu_dev *vpu, 
>> struct mtk_scp *scp,
>>       mem_size = vpu_alloc_size;
>>       if (mdp_vpu_shared_mem_alloc(vpu)) {
>>           dev_err(&mdp->pdev->dev, "VPU memory alloc fail!");
>> +        err = -ENOMEM;
>>           goto err_mem_alloc;
>>       }
>>
> 
> 
> .

Re: [PATCH 2/3] media: platform: mtk-mdp3: fix PM reference leak in mdp_comp_clock_on()

[Sun Ke]

ping...

在 2022/9/2 16:58, Sun Ke 写道:
> mdp_comp_clock_on will increase runtime PM usage counter,
> and mdp_comp_clock_off will decrease the runtime PM usage counter.
> so, if mdp_comp_clock_on failed after increment runtime PM usage
> counter, it should decrease it before return a error code.
> 
> pm_runtime_get_sync will increment pm usage counter even it failed.
> Forgetting to putting operation will result in reference leak here.
> Fix it by replacing it with pm_runtime_resume_and_get to keep usage
> counter balanced.
> 
> And if failed to enable clk, add pm_runtime_put() to decrease the
> runtime PM usage counter.
> 
> Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
>   drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
> index 9a6ba5851ccb..d3eaf8884412 100644
> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
> @@ -682,7 +682,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
>   	int i, ret;
>   
>   	if (comp->comp_dev) {
> -		ret = pm_runtime_get_sync(comp->comp_dev);
> +		ret = pm_runtime_resume_and_get(comp->comp_dev);
>   		if (ret < 0) {
>   			dev_err(dev,
>   				"Failed to get power, err %d. type:%d id:%d\n",
> @@ -699,6 +699,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
>   			dev_err(dev,
>   				"Failed to enable clk %d. type:%d id:%d\n",
>   				i, comp->type, comp->id);
> +			pm_runtime_put(comp->comp_dev);
>   			return ret;
>   		}
>   	}
> 

Re: [PATCH 2/3] media: platform: mtk-mdp3: fix PM reference leak in mdp_comp_clock_on()

[Hans Verkuil]

It's in a pull request, waiting to be merged.

Regards,

	Hans

On 23/09/2022 08:25, Sun Ke wrote:
> ping...
> 
> 在 2022/9/2 16:58, Sun Ke 写道:
>> mdp_comp_clock_on will increase runtime PM usage counter,
>> and mdp_comp_clock_off will decrease the runtime PM usage counter.
>> so, if mdp_comp_clock_on failed after increment runtime PM usage
>> counter, it should decrease it before return a error code.
>>
>> pm_runtime_get_sync will increment pm usage counter even it failed.
>> Forgetting to putting operation will result in reference leak here.
>> Fix it by replacing it with pm_runtime_resume_and_get to keep usage
>> counter balanced.
>>
>> And if failed to enable clk, add pm_runtime_put() to decrease the
>> runtime PM usage counter.
>>
>> Fixes: 61890ccaefaf ("media: platform: mtk-mdp3: add MediaTek MDP3 driver")
>> Signed-off-by: Sun Ke <sunke32@huawei.com>
>> ---
>>   drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
>> index 9a6ba5851ccb..d3eaf8884412 100644
>> --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
>> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
>> @@ -682,7 +682,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
>>       int i, ret;
>>         if (comp->comp_dev) {
>> -        ret = pm_runtime_get_sync(comp->comp_dev);
>> +        ret = pm_runtime_resume_and_get(comp->comp_dev);
>>           if (ret < 0) {
>>               dev_err(dev,
>>                   "Failed to get power, err %d. type:%d id:%d\n",
>> @@ -699,6 +699,7 @@ int mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
>>               dev_err(dev,
>>                   "Failed to enable clk %d. type:%d id:%d\n",
>>                   i, comp->type, comp->id);
>> +            pm_runtime_put(comp->comp_dev);
>>               return ret;
>>           }
>>       }
>>