[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

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

[Jacopo Mondi]

Hi David

On Tue, Nov 10, 2026 at 08:32:00AM +0000, David CARLIER wrote:
> Fair point, sorry for the shortcuts.
>
> > What does it mean "rather than through the graph" ?
> > enable_streams() is propagated by drivers along the pipeline by design
>
> Badly worded, sorry. I only meant that start_streaming() takes the
> resizer from cap_dev->rsz rather than from the remote end of an enabled
> link, so it enables it even with the link disabled.
>
> > Is this a typo ? From FR we can't go to the DS resizer.
>
> Not for data, no. I meant the media_pipeline_populate() walk: from the

Ah ok

> ISP source video pad it follows the immutable link into the DS resizer
> sink pad and, through the internal route, its source pad. A
> MUST_CONNECT on that pad would then fail FR only streaming with the DS
> link disabled.

This means (and reading Documentation/admin-guide/media/mali-c55.rst I
think it was intended) that if you link-enable the DS-pipe you will have to
start streaming on the DS capture node, otherwise streaming will never
start due to mali_c55_pipeline_ready() returning false ?

I wish I had a system with a DS pipe to test with.

What I'm after here: I'm trying to figure out if your change
introduces any requirement on how pipe have to be handled and if this
has any implication on how libcamera works on mali.

A use case is continous capture from DS while only taking seldom
pictures from FR. In this case I have to link both pipes, start both
of them and feed buffers continuously to DS and only sporadically to
FR. The key here is the requirement to call STREAMON on both capture
nodes if they're linked.

In other words, if you want to use both pipes you need to link both of them
and call STREAMON on both capture devices, even if in a session you will be
capturing from one of them.

I think it's fine and matches the documentation in mali-c55.rst and
the libcamera pipeline handler.

>
> Thanks for the reviews.
> Cheers.