[PATCH v2] media: mali-c55: Require an enabled link to start a video node

[PATCH v2] media: mali-c55: Require an enabled link to start a video node

[David Carlier]

mali_c55_pipeline_ready() skips queues whose link is disabled, but such
a queue can still start streaming on its own, as the video node pads
lack MEDIA_PAD_FL_MUST_CONNECT. Stopping it later sees a ready pipeline
and disables the ISP under the connected queues still capturing.

Mark the video node pads MUST_CONNECT so that starting a disconnected
node fails with -ENOLINK.

Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
Changes in v2:
- Add missing Signed-off-by.
- Cc stable, as d5f281f3dd29 is in the stable tree.

Build tested only (W=1, checkpatch --strict clean). Not tested on
hardware; the disconnected-node STREAMON/STREAMOFF sequence is from
code reading.
 drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 2 +-
 drivers/media/platform/arm/mali-c55/mali-c55-params.c  | 2 +-
 drivers/media/platform/arm/mali-c55/mali-c55-stats.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
index ff01553026fb..2dbce550d089 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
@@ -855,7 +855,7 @@ static int mali_c55_register_cap_dev(struct mali_c55 *mali_c55,
 		goto err_destroy_mutex;
 	}
 
-	cap_dev->pad.flags = MEDIA_PAD_FL_SINK;
+	cap_dev->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
 	ret = media_entity_pads_init(&cap_dev->vdev.entity, 1, &cap_dev->pad);
 	if (ret)
 		goto err_destroy_mutex;
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
index 16e895e3488e..0888dc4a340e 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
@@ -1014,7 +1014,7 @@ int mali_c55_register_params(struct mali_c55 *mali_c55)
 	INIT_LIST_HEAD(&params->buffers.queue);
 	spin_lock_init(&params->buffers.lock);
 
-	params->pad.flags = MEDIA_PAD_FL_SOURCE;
+	params->pad.flags = MEDIA_PAD_FL_SOURCE | MEDIA_PAD_FL_MUST_CONNECT;
 	ret = media_entity_pads_init(&params->vdev.entity, 1, &params->pad);
 	if (ret)
 		goto err_destroy_mutex;
diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-stats.c b/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
index a9c28b090b2a..a9172781925c 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
@@ -275,7 +275,7 @@ int mali_c55_register_stats(struct mali_c55 *mali_c55)
 	INIT_LIST_HEAD(&stats->buffers.queue);
 	spin_lock_init(&stats->buffers.lock);
 
-	stats->pad.flags = MEDIA_PAD_FL_SINK;
+	stats->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
 	ret = media_entity_pads_init(&stats->vdev.entity, 1, &stats->pad);
 	if (ret)
 		goto err_destroy_mutex;
-- 
2.55.0

Re: [PATCH v2] media: mali-c55: Require an enabled link to start a video node

[Linus Walleij]

On Fri, Nov 6, 2026 at 12:16 AM David Carlier <devnexen@gmail.com> wrote:

> mali_c55_pipeline_ready() skips queues whose link is disabled, but such
> a queue can still start streaming on its own, as the video node pads
> lack MEDIA_PAD_FL_MUST_CONNECT. Stopping it later sees a ready pipeline
> and disables the ISP under the connected queues still capturing.
>
> Mark the video node pads MUST_CONNECT so that starting a disconnected
> node fails with -ENOLINK.
>
> Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>

It checks out:
Reviewed-by: Linus Walleij <linusw@kernel.org>

> Changes in v2:
> - Add missing Signed-off-by.
> - Cc stable, as d5f281f3dd29 is in the stable tree.

No Cc: stable@vger.kernel.org actually added?

But Jacopo can certainly add that when applying the patch.

Yours,
Linus Walleij

Re: [PATCH v2] media: mali-c55: Require an enabled link to start a video node

[Jacopo Mondi]

Hi David

On Thu, Nov 05, 2026 at 11:15:59PM +0000, David Carlier wrote:
> mali_c55_pipeline_ready() skips queues whose link is disabled, but such
> a queue can still start streaming on its own, as the video node pads

Can you elaborate on how this could happen ?

How can streaming start if you VIDIOC_STREAMON() on a capture device
without an enabled link to the resizer subdev ?

> lack MEDIA_PAD_FL_MUST_CONNECT. Stopping it later sees a ready pipeline
> and disables the ISP under the connected queues still capturing.
>
> Mark the video node pads MUST_CONNECT so that starting a disconnected
> node fails with -ENOLINK.
>
> Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> Changes in v2:
> - Add missing Signed-off-by.
> - Cc stable, as d5f281f3dd29 is in the stable tree.
>
> Build tested only (W=1, checkpatch --strict clean). Not tested on
> hardware; the disconnected-node STREAMON/STREAMOFF sequence is from
> code reading.
>  drivers/media/platform/arm/mali-c55/mali-c55-capture.c | 2 +-
>  drivers/media/platform/arm/mali-c55/mali-c55-params.c  | 2 +-
>  drivers/media/platform/arm/mali-c55/mali-c55-stats.c   | 2 +-

Why not the resizer ?

>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> index ff01553026fb..2dbce550d089 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-capture.c
> @@ -855,7 +855,7 @@ static int mali_c55_register_cap_dev(struct mali_c55 *mali_c55,
>  		goto err_destroy_mutex;
>  	}
>
> -	cap_dev->pad.flags = MEDIA_PAD_FL_SINK;
> +	cap_dev->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
>  	ret = media_entity_pads_init(&cap_dev->vdev.entity, 1, &cap_dev->pad);
>  	if (ret)
>  		goto err_destroy_mutex;
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-params.c b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> index 16e895e3488e..0888dc4a340e 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-params.c
> @@ -1014,7 +1014,7 @@ int mali_c55_register_params(struct mali_c55 *mali_c55)
>  	INIT_LIST_HEAD(&params->buffers.queue);
>  	spin_lock_init(&params->buffers.lock);
>
> -	params->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	params->pad.flags = MEDIA_PAD_FL_SOURCE | MEDIA_PAD_FL_MUST_CONNECT;
>  	ret = media_entity_pads_init(&params->vdev.entity, 1, &params->pad);
>  	if (ret)
>  		goto err_destroy_mutex;
> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-stats.c b/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
> index a9c28b090b2a..a9172781925c 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-stats.c
> @@ -275,7 +275,7 @@ int mali_c55_register_stats(struct mali_c55 *mali_c55)
>  	INIT_LIST_HEAD(&stats->buffers.queue);
>  	spin_lock_init(&stats->buffers.lock);
>
> -	stats->pad.flags = MEDIA_PAD_FL_SINK;
> +	stats->pad.flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
>  	ret = media_entity_pads_init(&stats->vdev.entity, 1, &stats->pad);
>  	if (ret)
>  		goto err_destroy_mutex;
> --
> 2.55.0
>
>

Re: [PATCH v2] media: mali-c55: Require an enabled link to start a video node

[Jacopo Mondi]

Hi David

please don't cut out context from replies, especially if they're not
super long.

On Mon, Nov 09, 2026 at 07:57:31PM +0000, David CARLIER wrote:
> Hi Jacopo,
>
> On Mon, Nov 09, 2026 at 04:52:50PM +0100, Jacopo Mondi wrote:
> > Can you elaborate on how this could happen ?
> >
> > How can streaming start if you VIDIOC_STREAMON() on a capture device
> > without an enabled link to the resizer subdev ?
>
> video_device_pipeline_alloc_start() only walks enabled links, so with
> the link disabled the pipeline is the video node pad alone. Nothing in
> it fails validation, as the only -ENOLINK check in
> __media_pipeline_start() is for MUST_CONNECT pads. start_streaming()
> then enables cap_dev->rsz directly rather than through the graph.

What does it mean "rather than through the graph" ?
enable_streams() is propagated by drivers along the pipeline by design

> So with the ds link disabled: STREAMON ds, then fr, params and stats
> (the last one starts the ISP), then STREAMOFF ds.
> mali_c55_pipeline_ready() skips ds and returns true, so the ISP is
> stopped under fr.

so a STREAMOFF on a disconnected capture node could potentially stop
the other. Feels like mali_c55_pipeline_ready() is not the right
function to use, but if we mandate that a capture node has to be
connected if STREAMON is called on it, then we're safe ?

>
> > Why not the resizer ?
>
> The ISP to resizer links are immutable and enabled, so the sink pads
> are always connected. Flagging the resizer source pad would break the
> valid case instead: starting fr walks through the ISP into resizer ds

Is this a typo ? From FR we can't go to the DS resizer.

> and its source pad via the internal route, so fr only capture with ds
> disabled would fail with -ENOLINK. The video node pad is where the walk
> starts and what pipeline_ready() checks, so the flag goes there.

Anyway, yes, there is a single link between the resizer's source pad
and the capture device, if the SINK pad of the capture device is
marked as MUST_CONNECT then the link between the two is enabled.

>
> Cheers.

Re: [PATCH v2] media: mali-c55: Require an enabled link to start a video node

[Jacopo Mondi]

On Mon, Nov 09, 2026 at 10:26:01AM +0100, Linus Walleij wrote:
> On Fri, Nov 6, 2026 at 12:16 AM David Carlier <devnexen@gmail.com> wrote:
>
> > mali_c55_pipeline_ready() skips queues whose link is disabled, but such
> > a queue can still start streaming on its own, as the video node pads
> > lack MEDIA_PAD_FL_MUST_CONNECT. Stopping it later sees a ready pipeline
> > and disables the ISP under the connected queues still capturing.
> >
> > Mark the video node pads MUST_CONNECT so that starting a disconnected
> > node fails with -ENOLINK.
> >
> > Fixes: d5f281f3dd29 ("media: mali-c55: Add Mali-C55 ISP driver")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
>
> It checks out:
> Reviewed-by: Linus Walleij <linusw@kernel.org>
>
> > Changes in v2:
> > - Add missing Signed-off-by.
> > - Cc stable, as d5f281f3dd29 is in the stable tree.
>
> No Cc: stable@vger.kernel.org actually added?
>
> But Jacopo can certainly add that when applying the patch.

I don't have any C55 with the DS pipe fitted to test with, so I'll
just
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

and I can Cc stable indeed

>
> Yours,
> Linus Walleij

Thanks
   j