[PATCH v2 0/2] media: mediatek: fix VPU device leaks on probe

[PATCH v2 0/2] media: mediatek: fix VPU device leaks on probe

[Johan Hovold]

This series fixes VPU device leaks during probe of the mdp driver.

Included is also a minor documentation update to make it clear that the
VPU lookup helper returns the device with an incremented refcount.

Johan


Changes in v2
 - drop incorrect vcodec patch since that reference leak has already
   been fixed by commit bf1d556ad4e0 ("media: mtk-vcodec: abstract
   firmware interface") which added the codec release callback


Johan Hovold (2):
  media: mediatek: mdp: fix device leak on probe
  media: mediatek: amend vpu_get_plat_device() documentation

 drivers/media/platform/mediatek/mdp/mtk_mdp_core.c | 7 +++++--
 drivers/media/platform/mediatek/vpu/mtk_vpu.h      | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.51.0

[PATCH v2 1/2] media: mediatek: mdp: fix device leak on probe

[Johan Hovold]

Make sure to drop the reference taken when looking up the VPU firmware
device during probe on probe failure (e.g. probe deferral) and on driver
unbind.

Fixes: c8eb2d7e8202 ("[media] media: Add Mediatek MDP Driver")
Cc: stable@vger.kernel.org	# 4.10
Cc: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/mediatek/mdp/mtk_mdp_core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c b/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
index 80fdc6ff57e0..5c7dcf4090f4 100644
--- a/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mediatek/mdp/mtk_mdp_core.c
@@ -198,7 +198,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 				  VPU_RST_MDP);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register reset handler\n");
-		goto err_m2m_register;
+		goto err_put_vpu;
 	}
 
 	platform_set_drvdata(pdev, mdp);
@@ -206,7 +206,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 	ret = vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to set vb2 dma mag seg size\n");
-		goto err_m2m_register;
+		goto err_put_vpu;
 	}
 
 	pm_runtime_enable(dev);
@@ -214,6 +214,8 @@ static int mtk_mdp_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_put_vpu:
+	put_device(&mdp->vpu_dev->dev);
 err_m2m_register:
 	v4l2_device_unregister(&mdp->v4l2_dev);
 
@@ -241,6 +243,7 @@ static void mtk_mdp_remove(struct platform_device *pdev)
 	struct mtk_mdp_comp *comp, *comp_temp;
 
 	pm_runtime_disable(&pdev->dev);
+	put_device(&mdp->vpu_dev->dev);
 	vb2_dma_contig_clear_max_seg_size(&pdev->dev);
 	mtk_mdp_unregister_m2m_device(mdp);
 	v4l2_device_unregister(&mdp->v4l2_dev);
-- 
2.51.0

[PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

[Johan Hovold]

Add a comment to the vpu_get_plat_device() documentation to make it
clear that the VPU platform device is returned with an incremented
reference count (which needs to be dropped after use).

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
index da05f3e74081..5808311d2b15 100644
--- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
+++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
@@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
  *		device for using VPU API.
  *
  * Return: Return NULL if it is failed.
- * otherwise it is VPU's platform device
+ * otherwise it is VPU's platform device with incremented reference count
  **/
 struct platform_device *vpu_get_plat_device(struct platform_device *pdev);
 
-- 
2.51.0

Re: [PATCH v2 0/2] media: mediatek: fix VPU device leaks on probe

[Johan Hovold]

On Tue, Oct 28, 2025 at 11:04:52AM +0100, Johan Hovold wrote:
> This series fixes VPU device leaks during probe of the mdp driver.
> 
> Included is also a minor documentation update to make it clear that the
> VPU lookup helper returns the device with an incremented refcount.
> 
> Johan
> 
> 
> Changes in v2
>  - drop incorrect vcodec patch since that reference leak has already
>    been fixed by commit bf1d556ad4e0 ("media: mtk-vcodec: abstract
>    firmware interface") which added the codec release callback
> 
> 
> Johan Hovold (2):
>   media: mediatek: mdp: fix device leak on probe
>   media: mediatek: amend vpu_get_plat_device() documentation

Looks like I forgot to add Chen-Yu's reviewed-by when respinning:

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>

Re: [PATCH v2 0/2] media: mediatek: fix VPU device leaks on probe

[AngeloGioacchino Del Regno]

Il 28/10/25 11:04, Johan Hovold ha scritto:
> This series fixes VPU device leaks during probe of the mdp driver.
> 
> Included is also a minor documentation update to make it clear that the
> VPU lookup helper returns the device with an incremented refcount.
> 
> Johan
> 
> 
> Changes in v2
>   - drop incorrect vcodec patch since that reference leak has already
>     been fixed by commit bf1d556ad4e0 ("media: mtk-vcodec: abstract
>     firmware interface") which added the codec release callback
> 
> 
> Johan Hovold (2):
>    media: mediatek: mdp: fix device leak on probe
>    media: mediatek: amend vpu_get_plat_device() documentation
> 
>   drivers/media/platform/mediatek/mdp/mtk_mdp_core.c | 7 +++++--
>   drivers/media/platform/mediatek/vpu/mtk_vpu.h      | 2 +-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 


While series is

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

Re: [PATCH v2 0/2] media: mediatek: fix VPU device leaks on probe

[Johan Hovold]

On Tue, Oct 28, 2025 at 11:04:52AM +0100, Johan Hovold wrote:
> This series fixes VPU device leaks during probe of the mdp driver.
> 
> Included is also a minor documentation update to make it clear that the
> VPU lookup helper returns the device with an incremented refcount.

> Changes in v2
>  - drop incorrect vcodec patch since that reference leak has already
>    been fixed by commit bf1d556ad4e0 ("media: mtk-vcodec: abstract
>    firmware interface") which added the codec release callback
> 
> 
> Johan Hovold (2):
>   media: mediatek: mdp: fix device leak on probe
>   media: mediatek: amend vpu_get_plat_device() documentation

Can these be picked up for 6.19?

Johan

Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Hi,

Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> Add a comment to the vpu_get_plat_device() documentation to make it
> clear that the VPU platform device is returned with an incremented
> reference count (which needs to be dropped after use).
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> index da05f3e74081..5808311d2b15 100644
> --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
>   *		device for using VPU API.
>   *
>   * Return: Return NULL if it is failed.
> - * otherwise it is VPU's platform device
> + * otherwise it is VPU's platform device with incremented reference count

I picked this patch but rewrote with what felt like better and dense.

- * Return: Return NULL if it is failed.
- * otherwise it is VPU's platform device
+ * Return: a reference to the VPU's platform device, or NULL on failure.

hope its ok with you,

cheers,
Nicolas

>   **/
>  struct platform_device *vpu_get_plat_device(struct platform_device *pdev);
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 1664 bytes --]

On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> Hi,
> 
> Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> > Add a comment to the vpu_get_plat_device() documentation to make it
> > clear that the VPU platform device is returned with an incremented
> > reference count (which needs to be dropped after use).
> > 
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > index da05f3e74081..5808311d2b15 100644
> > --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> >   *		device for using VPU API.
> >   *
> >   * Return: Return NULL if it is failed.
> > - * otherwise it is VPU's platform device
> > + * otherwise it is VPU's platform device with incremented reference count
> 
> I picked this patch but rewrote with what felt like better and dense.
> 
> - * Return: Return NULL if it is failed.
> - * otherwise it is VPU's platform device
> + * Return: a reference to the VPU's platform device, or NULL on failure.
> 
> hope its ok with you,

Sure, my only concern is that just saying "reference" is too subtle,
that's why I explicitly mentioned the refcount.

Btw, why is patch 2/2 marked obsolete? That leak is still there both on
probe errors (which I saw someone else posted a fix for) and on driver
unbind.

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

Le mercredi 10 décembre 2025 à 12:21 +0900, Johan Hovold a écrit :
> On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> > Hi,
> > 
> > Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :
> > > Add a comment to the vpu_get_plat_device() documentation to make it
> > > clear that the VPU platform device is returned with an incremented
> > > reference count (which needs to be dropped after use).
> > > 
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  drivers/media/platform/mediatek/vpu/mtk_vpu.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > index da05f3e74081..5808311d2b15 100644
> > > --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.h
> > > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> > >   *		device for using VPU API.
> > >   *
> > >   * Return: Return NULL if it is failed.
> > > - * otherwise it is VPU's platform device
> > > + * otherwise it is VPU's platform device with incremented reference count
> > 
> > I picked this patch but rewrote with what felt like better and dense.
> > 
> > - * Return: Return NULL if it is failed.
> > - * otherwise it is VPU's platform device
> > + * Return: a reference to the VPU's platform device, or NULL on failure.
> > 
> > hope its ok with you,
> 
> Sure, my only concern is that just saying "reference" is too subtle,
> that's why I explicitly mentioned the refcount.

For me everyone should read "a reference" as a kref based reference counted
structure.  A quick grep across out documentation, this is the vast majority of
the wording. Though, I spent limited time looking.

> 
> Btw, why is patch 2/2 marked obsolete? That leak is still there both on
> probe errors (which I saw someone else posted a fix for) and on driver
> unbind.

I had two patches fixing the same thing, it just happen that I ended up picking
the other one first, and liked you documentation fix, except it was replicating
obvious weird english such as "if it is failed", and documenting the error
before the expected outcome (opposite of my preference, not sure there is any
rules or guidelines).

https://lore.kernel.org/all/20251008090156.14224-1-haoxiang_li2024@163.com/

cheers,
Nicolas

> 
> Johan

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] media: mediatek: amend vpu_get_plat_device() documentation

[Johan Hovold]

[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

On Wed, Dec 10, 2025 at 10:58:41AM -0500, Nicolas Dufresne wrote:
> Le mercredi 10 décembre 2025 à 12:21 +0900, Johan Hovold a écrit :
> > On Tue, Dec 09, 2025 at 03:43:30PM -0500, Nicolas Dufresne wrote:
> > > Le mardi 28 octobre 2025 à 11:04 +0100, Johan Hovold a écrit :

> > > > Add a comment to the vpu_get_plat_device() documentation to make it
> > > > clear that the VPU platform device is returned with an incremented
> > > > reference count (which needs to be dropped after use).

> > > > @@ -120,7 +120,7 @@ int vpu_ipi_send(struct platform_device *pdev,
> > > >   *		device for using VPU API.
> > > >   *
> > > >   * Return: Return NULL if it is failed.
> > > > - * otherwise it is VPU's platform device
> > > > + * otherwise it is VPU's platform device with incremented reference count
> > > 
> > > I picked this patch but rewrote with what felt like better and dense.
> > > 
> > > - * Return: Return NULL if it is failed.
> > > - * otherwise it is VPU's platform device
> > > + * Return: a reference to the VPU's platform device, or NULL on failure.
> > > 
> > > hope its ok with you,
> > 
> > Sure, my only concern is that just saying "reference" is too subtle,
> > that's why I explicitly mentioned the refcount.
> 
> For me everyone should read "a reference" as a kref based reference counted
> structure.  A quick grep across out documentation, this is the vast majority of
> the wording. Though, I spent limited time looking.

It should be sufficient but given how many people miss this it may still
be worth being more explicit.

> > Btw, why is patch 2/2 marked obsolete? That leak is still there both on
> > probe errors (which I saw someone else posted a fix for) and on driver
> > unbind.
> 
> I had two patches fixing the same thing, it just happen that I ended up picking
> the other one first

Ah, I had missed that and the fix is not in linux-next yet either so
without a reply it wasn't obvious.

> and liked you documentation fix, except it was replicating
> obvious weird english such as "if it is failed", and documenting the error
> before the expected outcome (opposite of my preference, not sure there is any
> rules or guidelines).

Yeah, I considered rewriting the whole comment too but given that the
surrounding comments used similar language I just amended what was
there.

But I'm totally fine with the update you did.

> https://lore.kernel.org/all/20251008090156.14224-1-haoxiang_li2024@163.com/

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]