[PATCH] media: mediatek: vcodec: Initialize context lock before firmware setup

[PATCH] media: mediatek: vcodec: Initialize context lock before firmware setup

[Runyu Xiao]

The VPU firmware setup registers a watchdog callback that accesses the
encoder context lock. Initialize the lock before selecting the firmware
implementation so the callback cannot observe an uninitialized lock during
probe.

Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encoder context list")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 .../media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
index 4e4541b2f..e6cd3817a 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
@@ -255,6 +255,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&dev->ctx_list);
 	dev->plat_dev = pdev;
+	spin_lock_init(&dev->dev_ctx_lock);
 
 	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
 				  &rproc_phandle)) {
@@ -308,7 +309,6 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->enc_mutex);
 	mutex_init(&dev->dev_mutex);
-	spin_lock_init(&dev->dev_ctx_lock);
 	spin_lock_init(&dev->irqlock);
 
 	snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
-- 
2.34.1

Re: [PATCH] media: mediatek: vcodec: Initialize context lock before firmware setup

[Nicolas Dufresne]

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

Hi,

Le dimanche 30 août 2026 à 22:48 +0800, Runyu Xiao a écrit :
> The VPU firmware setup registers a watchdog callback that accesses the
> encoder context lock. Initialize the lock before selecting the firmware
> implementation so the callback cannot observe an uninitialized lock during
> probe.
> 
> Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encoder context list")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:GPT-5
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  .../media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> index 4e4541b2f..e6cd3817a 100644
> --- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
> @@ -255,6 +255,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
>  
>  	INIT_LIST_HEAD(&dev->ctx_list);
>  	dev->plat_dev = pdev;
> +	spin_lock_init(&dev->dev_ctx_lock);

I'll have to reject this one for now since the theory of a callback taking the
lock while the probe is happening does not hold. The very first place the lock
will be taken is in _open() and that cannot happen before
video_register_device(), which is kept last. Please demonstrate what the issue
you are suggesting if you'd like to see this merged.

Nicolas

>  
>  	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
>  				  &rproc_phandle)) {
> @@ -308,7 +309,6 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
>  
>  	mutex_init(&dev->enc_mutex);
>  	mutex_init(&dev->dev_mutex);
> -	spin_lock_init(&dev->dev_ctx_lock);
>  	spin_lock_init(&dev->irqlock);
>  
>  	snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",

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

Re: [PATCH] media: mediatek: vcodec: Initialize context lock before firmware setup

[Runyu Xiao]

Hi Nicolas,

On Sun, 4 Oct 2026 07:17:43 -0400, Nicolas Dufresne wrote:

> The very first place the lock will be taken is in _open() and that cannot
> happen before video_register_device(), which is kept last. Please
> demonstrate what the issue you are suggesting if you'd like to see this
> merged.

The normal encoder open path is as you describe. On VPU-backed systems,
firmware setup registers the encoder reset callback before dev_ctx_lock is
initialized. A timeout on the shared VPU while another client is active
queues reset work, which invokes registered callbacks. If that work runs
before spin_lock_init(), the callback takes the uninitialized lock. The
context list is initialized but empty during probe, so I cannot claim list
corruption or a user-visible failure. With CONFIG_DEBUG_SPINLOCK, the
zeroed lock fails the magic check.

Thanks,
Runyu

Re: [PATCH] media: mediatek: vcodec: Initialize context lock before firmware setup

[Nicolas Dufresne]

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

Le dimanche 04 octobre 2026 à 20:39 +0800, Runyu Xiao a écrit :
> Hi Nicolas,
> 
> On Sun, 4 Oct 2026 07:17:43 -0400, Nicolas Dufresne wrote:
> 
> > The very first place the lock will be taken is in _open() and that cannot
> > happen before video_register_device(), which is kept last. Please
> > demonstrate what the issue you are suggesting if you'd like to see this
> > merged.
> 
> The normal encoder open path is as you describe. On VPU-backed systems,
> firmware setup registers the encoder reset callback before dev_ctx_lock is
> initialized. A timeout on the shared VPU while another client is active
> queues reset work, which invokes registered callbacks. If that work runs
> before spin_lock_init(), the callback takes the uninitialized lock. The
> context list is initialized but empty during probe, so I cannot claim list
> corruption or a user-visible failure. With CONFIG_DEBUG_SPINLOCK, the
> zeroed lock fails the magic check.

Then please make a v2 that includes the mention of the encoder path, and that
includes the debug output when CONFIG_DEBUG_SPINLOCK in your message. That will
show that you actually tested it. Its extra effort, but please understand that I
receive dozen of hypothetical fixes (sometimes multiple time the same fix made
by LLM users that may or may not follow the list).

cheers,
Nicolas

> 
> Thanks,
> Runyu

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

[PATCH v2] media: mediatek: vcodec: Initialize context lock before firmware setup

[Runyu Xiao]

The normal encoder open path cannot take dev_ctx_lock before
video_register_device(), but there is another path during probe.

On VPU-backed devices, mtk_vcodec_probe() selects firmware before it
initializes dev_ctx_lock. VPU firmware setup registers the encoder watchdog
callback during this selection. A watchdog timeout on the shared VPU
queues reset work, which invokes the registered callbacks. If it runs
before spin_lock_init(), the encoder callback takes the uninitialized lock.

With CONFIG_DEBUG_SPINLOCK=y, an x86_64 QEMU test used a zeroed lock,
initialized an empty context list, and invoked a callback with matching
lock and list operations. It reported:

    [    1.761312] BUG: spinlock bad magic on CPU#0, busybox/58

The lock dump showed .magic: 00000000.

The test reproduces the debug lock diagnostic, not a real VPU watchdog
timeout or its timing during probe. The context list is initialized but
remains empty until video registration, so this shows invalid lock use, not
list corruption or a user-visible failure.

Initialize dev_ctx_lock before firmware setup registers the callback.

Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encoder context list")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 .../media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
index 4e4541b2f..e6cd3817a 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
@@ -255,6 +255,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&dev->ctx_list);
 	dev->plat_dev = pdev;
+	spin_lock_init(&dev->dev_ctx_lock);
 
 	if (!of_property_read_u32(pdev->dev.of_node, "mediatek,vpu",
 				  &rproc_phandle)) {
@@ -308,7 +309,6 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->enc_mutex);
 	mutex_init(&dev->dev_mutex);
-	spin_lock_init(&dev->dev_ctx_lock);
 	spin_lock_init(&dev->irqlock);
 
 	snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",
-- 
2.34.1