[libcamera-devel,v2,3/4] libcamera: Add link handling functions

Message ID 20190108170407.4770-4-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: media device: Add link handling
Related show

Commit Message

Jacopo Mondi Jan. 8, 2019, 5:04 p.m. UTC
Add a function to the MediaLink class to set the state of a link to
enabled or disabled. The function makes use of an internal MediaDevice
method, which is defined private and only accessible by the MediaLink
setEnable() function itself.

Also add to MediaDevice a function to reset all links registered in the
media graph.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
v1->v2:
- Add resetLinks() function
- s/enable()/setEnable()
- s/setLink()/setupLink()
- Pass MediaLink and not two MediaPads to MediaDevice::setupLink

 src/libcamera/include/media_device.h |  4 ++
 src/libcamera/include/media_object.h |  1 +
 src/libcamera/media_device.cpp       | 75 ++++++++++++++++++++++++++++
 src/libcamera/media_object.cpp       | 29 +++++++++++
 4 files changed, 109 insertions(+)

--
2.20.1

Comments

Laurent Pinchart Jan. 8, 2019, 6:18 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tuesday, 8 January 2019 19:04:06 EET Jacopo Mondi wrote:
> Add a function to the MediaLink class to set the state of a link to
> enabled or disabled. The function makes use of an internal MediaDevice
> method, which is defined private and only accessible by the MediaLink
> setEnable() function itself.
> 
> Also add to MediaDevice a function to reset all links registered in the
> media graph.

I would have split this to a separate patch, but it's not a big deal.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
> v1->v2:
> - Add resetLinks() function
> - s/enable()/setEnable()
> - s/setLink()/setupLink()
> - Pass MediaLink and not two MediaPads to MediaDevice::setupLink
> 
>  src/libcamera/include/media_device.h |  4 ++
>  src/libcamera/include/media_object.h |  1 +
>  src/libcamera/media_device.cpp       | 75 ++++++++++++++++++++++++++++
>  src/libcamera/media_object.cpp       | 29 +++++++++++
>  4 files changed, 109 insertions(+)
> 
> diff --git a/src/libcamera/include/media_device.h
> b/src/libcamera/include/media_device.h index 397d349..0f423aa 100644
> --- a/src/libcamera/include/media_device.h
> +++ b/src/libcamera/include/media_device.h
> @@ -45,6 +45,7 @@ public:
>  	MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
>  			const MediaEntity *sink, unsigned int sinkIdx);
>  	MediaLink *link(const MediaPad *source, const MediaPad *sink);
> +	int resetLinks();

I wonder if this should be called disableLinks() or disableAllLinks() to 
better reflect the purpose.

> 
>  private:
>  	std::string driver_;
> @@ -65,6 +66,9 @@ private:
>  	bool populateEntities(const struct media_v2_topology &topology);
>  	bool populatePads(const struct media_v2_topology &topology);
>  	bool populateLinks(const struct media_v2_topology &topology);
> +
> +	friend int MediaLink::setEnable(bool enable);
> +	int setupLink(const MediaLink *link, unsigned int flags);
>  };
> 
>  } /* namespace libcamera */
> diff --git a/src/libcamera/include/media_object.h
> b/src/libcamera/include/media_object.h index d92aab3..bb46fac 100644
> --- a/src/libcamera/include/media_object.h
> +++ b/src/libcamera/include/media_object.h
> @@ -41,6 +41,7 @@ public:
>  	MediaPad *source() const { return source_; }
>  	MediaPad *sink() const { return sink_; }
>  	unsigned int flags() const { return flags_; }
> +	int setEnable(bool enable);

s/setEnable/setEnabled/ ?

> 
>  private:
>  	friend class MediaDevice;
> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> index 7ce5c22..9900c3f 100644
> --- a/src/libcamera/media_device.cpp
> +++ b/src/libcamera/media_device.cpp
> @@ -385,6 +385,35 @@ MediaLink *MediaDevice::link(const MediaPad *source,
> const MediaPad *sink) return nullptr;
>  }
> 
> +/**
> + * \brief Reset all links in the media device

How about "Disable all links in the media device" ?

> + *
> + * Reset the media device links, clearing the MEDIA_LNK_FL_ENABLED flag
> + * on links which are not flagged as IMMUTABLE.

And updating this accordingly. The "reset" concept isn't clearly defined in 
the MC API, and why media-ctl implements it, I think stating the purpose more 
explicitly would be clearer.

> + * \return 0 for success, or a negative error code otherwise

s/for/on/

> + */
> +int MediaDevice::resetLinks()
> +{
> +	for (MediaEntity *entity : entities_) {
> +		for (MediaPad *pad : entity->pads()) {
> +			if (pad->flags() & MEDIA_PAD_FL_SINK)

I'd rather write this !(pad->flags() & MEDIA_PAD_FL_SOURCE) in case we later 
get pads that have neither flag set.

> +				continue;
> +
> +			for (MediaLink *link : pad->links()) {
> +				if (link->flags() & MEDIA_LNK_FL_IMMUTABLE)
> +					continue;
> +
> +				int ret = link->setEnable(false);
> +				if (ret)
> +					return ret;
> +			}
> +		}
> +	}

We don't need to do so in this patch, but do you think we should keep a list 
of links internally in MediaDevice for the purpose of iterating over them 
easily here ?

> +	return 0;
> +}
> +
>  /**
>   * \var MediaDevice::objects_
>   * \brief Global map of media objects (entities, pads, links) keyed by
> their @@ -601,4 +630,50 @@ bool MediaDevice::populateLinks(const struct
> media_v2_topology &topology) return true;
>  }
> 
> +/**
> + * \brief Apply \a flags to a link between two pads
> + * \param link The link to apply flags on

s/on/to/

> + * \param flags The flags to apply to the link
> + *
> + * This function applies the link \a flags (as defined by the
> MEDIA_LNK_FL_* + * macros from the Media Controller API) to the given \a
> link. It implements + * low-level link setup as it performs no checks on
> the validity of the \a + * flags, and assumes that the supplied \a flags
> are valid for the link (e.g. + * immutable links cannot be disabled)."

s/"//

> +*
> + * \sa MediaLink::setEnable(bool enable)
> + *
> + * \return 0 for success, or a negative error code otherwise

s/for/on/

> + */
> +int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
> +{
> +	struct media_link_desc linkDesc = { };
> +	MediaPad *source = link->source();
> +	MediaPad *sink = link->sink();
> +
> +	linkDesc.source.entity = source->entity()->id();
> +	linkDesc.source.index = source->index();
> +	linkDesc.source.flags = MEDIA_PAD_FL_SOURCE;
> +
> +	linkDesc.sink.entity = sink->entity()->id();
> +	linkDesc.sink.index = sink->index();
> +	linkDesc.sink.flags = MEDIA_PAD_FL_SINK;
> +
> +	linkDesc.flags = flags;
> +
> +	int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
> +	if (ret) {
> +		ret = -errno;
> +		LOG(Error) << "Failed to setup link: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	LOG(Debug) << source->entity()->name() << "["
> +		   << source->index() << "] -> "
> +		   << sink->entity()->name() << "["
> +		   << sink->index() << "]: " << flags;
> +
> +	return 0;
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> index 612550d..c6cb02b 100644
> --- a/src/libcamera/media_object.cpp
> +++ b/src/libcamera/media_object.cpp
> @@ -15,6 +15,7 @@
>  #include <linux/media.h>
> 
>  #include "log.h"
> +#include "media_device.h"
>  #include "media_object.h"
> 
>  /**
> @@ -89,6 +90,34 @@ namespace libcamera {
>   * Each link is referenced in the link array of both of the pads it
> connect. */
> 
> +/**
> + * \brief Enable or disable a lik

s/lik/link/

> + * \param enable True to enable the link, false to disable it
> + *
> + * Set the status of a link, according to the value of \a enable.

s/link,/link/

> + * Links between two pads can be set to the enabled or disabled state
> freely, + * unless they're immutable links, whose status cannot be changed.
> + * Enabling an immutable link is not considered an error, while trying to
> + * disable it is.
> + *
> + * Enabling a link establishes a data connection between two pads, while
> + * disabling it interrupts such a connections.

s/connections/connection/

or possibly better

s/such a connections/that connection/

> + *
> + * \return 0 on success, or a negative error code otherwise
> + */
> +int MediaLink::setEnable(bool enable)
> +{
> +	unsigned int flags = enable ? MEDIA_LNK_FL_ENABLED : 0;
> +
> +	int ret = dev_->setupLink(this, flags);
> +	if (ret)
> +		return ret;
> +
> +	flags_ = flags;
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Construct a MediaLink
>   * \param link The media link kernel data
Jacopo Mondi Jan. 8, 2019, 7:52 p.m. UTC | #2
Hi Laurent,

On Tue, Jan 08, 2019 at 08:18:45PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tuesday, 8 January 2019 19:04:06 EET Jacopo Mondi wrote:
> > Add a function to the MediaLink class to set the state of a link to
> > enabled or disabled. The function makes use of an internal MediaDevice
> > method, which is defined private and only accessible by the MediaLink
> > setEnable() function itself.
> >
> > Also add to MediaDevice a function to reset all links registered in the
> > media graph.
>
> I would have split this to a separate patch, but it's not a big deal.
>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> > v1->v2:
> > - Add resetLinks() function
> > - s/enable()/setEnable()
> > - s/setLink()/setupLink()
> > - Pass MediaLink and not two MediaPads to MediaDevice::setupLink
> >
> >  src/libcamera/include/media_device.h |  4 ++
> >  src/libcamera/include/media_object.h |  1 +
> >  src/libcamera/media_device.cpp       | 75 ++++++++++++++++++++++++++++
> >  src/libcamera/media_object.cpp       | 29 +++++++++++
> >  4 files changed, 109 insertions(+)
> >
> > diff --git a/src/libcamera/include/media_device.h
> > b/src/libcamera/include/media_device.h index 397d349..0f423aa 100644
> > --- a/src/libcamera/include/media_device.h
> > +++ b/src/libcamera/include/media_device.h
> > @@ -45,6 +45,7 @@ public:
> >  	MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
> >  			const MediaEntity *sink, unsigned int sinkIdx);
> >  	MediaLink *link(const MediaPad *source, const MediaPad *sink);
> > +	int resetLinks();
>
> I wonder if this should be called disableLinks() or disableAllLinks() to
> better reflect the purpose.
>

I find reset confusing as well, although it is used already, but I
find the enable/disable terms better... I'll go with disableLinks()
> >
> >  private:
> >  	std::string driver_;
> > @@ -65,6 +66,9 @@ private:
> >  	bool populateEntities(const struct media_v2_topology &topology);
> >  	bool populatePads(const struct media_v2_topology &topology);
> >  	bool populateLinks(const struct media_v2_topology &topology);
> > +
> > +	friend int MediaLink::setEnable(bool enable);
> > +	int setupLink(const MediaLink *link, unsigned int flags);
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/include/media_object.h
> > b/src/libcamera/include/media_object.h index d92aab3..bb46fac 100644
> > --- a/src/libcamera/include/media_object.h
> > +++ b/src/libcamera/include/media_object.h
> > @@ -41,6 +41,7 @@ public:
> >  	MediaPad *source() const { return source_; }
> >  	MediaPad *sink() const { return sink_; }
> >  	unsigned int flags() const { return flags_; }
> > +	int setEnable(bool enable);
>
> s/setEnable/setEnabled/ ?
>

It's maybe stupid but, since we have disableLinks() should we have an
enable() and a disable() function?

> >
> >  private:
> >  	friend class MediaDevice;
> > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
> > index 7ce5c22..9900c3f 100644
> > --- a/src/libcamera/media_device.cpp
> > +++ b/src/libcamera/media_device.cpp
> > @@ -385,6 +385,35 @@ MediaLink *MediaDevice::link(const MediaPad *source,
> > const MediaPad *sink) return nullptr;
> >  }
> >
> > +/**
> > + * \brief Reset all links in the media device
>
> How about "Disable all links in the media device" ?
>
> > + *
> > + * Reset the media device links, clearing the MEDIA_LNK_FL_ENABLED flag
> > + * on links which are not flagged as IMMUTABLE.
>
> And updating this accordingly. The "reset" concept isn't clearly defined in
> the MC API, and why media-ctl implements it, I think stating the purpose more
> explicitly would be clearer.
>

Agree

> > + * \return 0 for success, or a negative error code otherwise
>
> s/for/on/
>
> > + */
> > +int MediaDevice::resetLinks()
> > +{
> > +	for (MediaEntity *entity : entities_) {
> > +		for (MediaPad *pad : entity->pads()) {
> > +			if (pad->flags() & MEDIA_PAD_FL_SINK)
>
> I'd rather write this !(pad->flags() & MEDIA_PAD_FL_SOURCE) in case we later
> get pads that have neither flag set.
>

ack

> > +				continue;
> > +
> > +			for (MediaLink *link : pad->links()) {
> > +				if (link->flags() & MEDIA_LNK_FL_IMMUTABLE)
> > +					continue;
> > +
> > +				int ret = link->setEnable(false);
> > +				if (ret)
> > +					return ret;
> > +			}
> > +		}
> > +	}
>
> We don't need to do so in this patch, but do you think we should keep a list
> of links internally in MediaDevice for the purpose of iterating over them
> easily here ?
>

I thought about it... If it is just for this maybe no.. it's not a big
deal though, it's just a list of references...

> > +	return 0;
> > +}
> > +
> >  /**
> >   * \var MediaDevice::objects_
> >   * \brief Global map of media objects (entities, pads, links) keyed by
> > their @@ -601,4 +630,50 @@ bool MediaDevice::populateLinks(const struct
> > media_v2_topology &topology) return true;
> >  }
> >
> > +/**
> > + * \brief Apply \a flags to a link between two pads
> > + * \param link The link to apply flags on
>
> s/on/to/
>
> > + * \param flags The flags to apply to the link
> > + *
> > + * This function applies the link \a flags (as defined by the
> > MEDIA_LNK_FL_* + * macros from the Media Controller API) to the given \a
> > link. It implements + * low-level link setup as it performs no checks on
> > the validity of the \a + * flags, and assumes that the supplied \a flags
> > are valid for the link (e.g. + * immutable links cannot be disabled)."
>
> s/"//
>
> > +*
> > + * \sa MediaLink::setEnable(bool enable)
> > + *
> > + * \return 0 for success, or a negative error code otherwise
>
> s/for/on/
>
> > + */
> > +int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
> > +{
> > +	struct media_link_desc linkDesc = { };
> > +	MediaPad *source = link->source();
> > +	MediaPad *sink = link->sink();
> > +
> > +	linkDesc.source.entity = source->entity()->id();
> > +	linkDesc.source.index = source->index();
> > +	linkDesc.source.flags = MEDIA_PAD_FL_SOURCE;
> > +
> > +	linkDesc.sink.entity = sink->entity()->id();
> > +	linkDesc.sink.index = sink->index();
> > +	linkDesc.sink.flags = MEDIA_PAD_FL_SINK;
> > +
> > +	linkDesc.flags = flags;
> > +
> > +	int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
> > +	if (ret) {
> > +		ret = -errno;
> > +		LOG(Error) << "Failed to setup link: " << strerror(-ret);
> > +		return ret;
> > +	}
> > +
> > +	LOG(Debug) << source->entity()->name() << "["
> > +		   << source->index() << "] -> "
> > +		   << sink->entity()->name() << "["
> > +		   << sink->index() << "]: " << flags;
> > +
> > +	return 0;
> > +}
> > +
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
> > index 612550d..c6cb02b 100644
> > --- a/src/libcamera/media_object.cpp
> > +++ b/src/libcamera/media_object.cpp
> > @@ -15,6 +15,7 @@
> >  #include <linux/media.h>
> >
> >  #include "log.h"
> > +#include "media_device.h"
> >  #include "media_object.h"
> >
> >  /**
> > @@ -89,6 +90,34 @@ namespace libcamera {
> >   * Each link is referenced in the link array of both of the pads it
> > connect. */
> >
> > +/**
> > + * \brief Enable or disable a lik
>
> s/lik/link/
>
> > + * \param enable True to enable the link, false to disable it
> > + *
> > + * Set the status of a link, according to the value of \a enable.
>
> s/link,/link/
>
> > + * Links between two pads can be set to the enabled or disabled state
> > freely, + * unless they're immutable links, whose status cannot be changed.
> > + * Enabling an immutable link is not considered an error, while trying to
> > + * disable it is.
> > + *
> > + * Enabling a link establishes a data connection between two pads, while
> > + * disabling it interrupts such a connections.
>
> s/connections/connection/
>
> or possibly better
>
> s/such a connections/that connection/
>

Yes, better

> > + *
> > + * \return 0 on success, or a negative error code otherwise
> > + */
> > +int MediaLink::setEnable(bool enable)
> > +{
> > +	unsigned int flags = enable ? MEDIA_LNK_FL_ENABLED : 0;
> > +
> > +	int ret = dev_->setupLink(this, flags);
> > +	if (ret)
> > +		return ret;
> > +
> > +	flags_ = flags;
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * \brief Construct a MediaLink
> >   * \param link The media link kernel data

Thanks
  j

>
> --
> Regards,
>
> Laurent Pinchart
>
>
>
Laurent Pinchart Jan. 8, 2019, 8:26 p.m. UTC | #3
Hi Jacopo,

On Tuesday, 8 January 2019 21:52:42 EET Jacopo Mondi wrote:
> On Tue, Jan 08, 2019 at 08:18:45PM +0200, Laurent Pinchart wrote:
> > On Tuesday, 8 January 2019 19:04:06 EET Jacopo Mondi wrote:
> >> Add a function to the MediaLink class to set the state of a link to
> >> enabled or disabled. The function makes use of an internal MediaDevice
> >> method, which is defined private and only accessible by the MediaLink
> >> setEnable() function itself.
> >> 
> >> Also add to MediaDevice a function to reset all links registered in the
> >> media graph.
> > 
> > I would have split this to a separate patch, but it's not a big deal.
> > 
> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >> ---
> >> v1->v2:
> >> - Add resetLinks() function
> >> - s/enable()/setEnable()
> >> - s/setLink()/setupLink()
> >> - Pass MediaLink and not two MediaPads to MediaDevice::setupLink
> >> 
> >>  src/libcamera/include/media_device.h |  4 ++
> >>  src/libcamera/include/media_object.h |  1 +
> >>  src/libcamera/media_device.cpp       | 75 ++++++++++++++++++++++++++++
> >>  src/libcamera/media_object.cpp       | 29 +++++++++++
> >>  4 files changed, 109 insertions(+)
> >> 
> >> diff --git a/src/libcamera/include/media_device.h
> >> b/src/libcamera/include/media_device.h index 397d349..0f423aa 100644
> >> --- a/src/libcamera/include/media_device.h
> >> +++ b/src/libcamera/include/media_device.h
> >> @@ -45,6 +45,7 @@ public:
> >>  	MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
> >>  			const MediaEntity *sink, unsigned int sinkIdx);
> >>  	MediaLink *link(const MediaPad *source, const MediaPad *sink);
> >> 
> > > +	int resetLinks();
> > 
> > I wonder if this should be called disableLinks() or disableAllLinks() to
> > better reflect the purpose.
> 
> I find reset confusing as well, although it is used already, but I
> find the enable/disable terms better... I'll go with disableLinks()
> 
> >>  private:
> >>  	std::string driver_;
> >> 
> >> @@ -65,6 +66,9 @@ private:
> >>  	bool populateEntities(const struct media_v2_topology &topology);
> >>  	bool populatePads(const struct media_v2_topology &topology);
> >>  	bool populateLinks(const struct media_v2_topology &topology);
> >> +
> >> +	friend int MediaLink::setEnable(bool enable);
> >> +	int setupLink(const MediaLink *link, unsigned int flags);
> >>  };
> >>  
> >>  } /* namespace libcamera */
> >> diff --git a/src/libcamera/include/media_object.h
> >> b/src/libcamera/include/media_object.h index d92aab3..bb46fac 100644
> >> --- a/src/libcamera/include/media_object.h
> >> +++ b/src/libcamera/include/media_object.h
> >> @@ -41,6 +41,7 @@ public:
> >>  	MediaPad *source() const { return source_; }
> >>  	MediaPad *sink() const { return sink_; }
> >>  	unsigned int flags() const { return flags_; }
> >> +	int setEnable(bool enable);
> > 
> > s/setEnable/setEnabled/ ?
> 
> It's maybe stupid but, since we have disableLinks() should we have an
> enable() and a disable() function?

It's a good question. As the property is "enabled", having enabled() and 
setEnabled() accessors is simple and logical. enable() and disable() would be 
as well, but it would be a special case. I'm tempted to stick to setEnabled() 
to follow the generic rule.

> >>  private:
> >>  	friend class MediaDevice;
> >> 

[snip]

Patch

diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h
index 397d349..0f423aa 100644
--- a/src/libcamera/include/media_device.h
+++ b/src/libcamera/include/media_device.h
@@ -45,6 +45,7 @@  public:
 	MediaLink *link(const MediaEntity *source, unsigned int sourceIdx,
 			const MediaEntity *sink, unsigned int sinkIdx);
 	MediaLink *link(const MediaPad *source, const MediaPad *sink);
+	int resetLinks();

 private:
 	std::string driver_;
@@ -65,6 +66,9 @@  private:
 	bool populateEntities(const struct media_v2_topology &topology);
 	bool populatePads(const struct media_v2_topology &topology);
 	bool populateLinks(const struct media_v2_topology &topology);
+
+	friend int MediaLink::setEnable(bool enable);
+	int setupLink(const MediaLink *link, unsigned int flags);
 };

 } /* namespace libcamera */
diff --git a/src/libcamera/include/media_object.h b/src/libcamera/include/media_object.h
index d92aab3..bb46fac 100644
--- a/src/libcamera/include/media_object.h
+++ b/src/libcamera/include/media_object.h
@@ -41,6 +41,7 @@  public:
 	MediaPad *source() const { return source_; }
 	MediaPad *sink() const { return sink_; }
 	unsigned int flags() const { return flags_; }
+	int setEnable(bool enable);

 private:
 	friend class MediaDevice;
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
index 7ce5c22..9900c3f 100644
--- a/src/libcamera/media_device.cpp
+++ b/src/libcamera/media_device.cpp
@@ -385,6 +385,35 @@  MediaLink *MediaDevice::link(const MediaPad *source, const MediaPad *sink)
 	return nullptr;
 }

+/**
+ * \brief Reset all links in the media device
+ *
+ * Reset the media device links, clearing the MEDIA_LNK_FL_ENABLED flag
+ * on links which are not flagged as IMMUTABLE.
+ *
+ * \return 0 for success, or a negative error code otherwise
+ */
+int MediaDevice::resetLinks()
+{
+	for (MediaEntity *entity : entities_) {
+		for (MediaPad *pad : entity->pads()) {
+			if (pad->flags() & MEDIA_PAD_FL_SINK)
+				continue;
+
+			for (MediaLink *link : pad->links()) {
+				if (link->flags() & MEDIA_LNK_FL_IMMUTABLE)
+					continue;
+
+				int ret = link->setEnable(false);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
 /**
  * \var MediaDevice::objects_
  * \brief Global map of media objects (entities, pads, links) keyed by their
@@ -601,4 +630,50 @@  bool MediaDevice::populateLinks(const struct media_v2_topology &topology)
 	return true;
 }

+/**
+ * \brief Apply \a flags to a link between two pads
+ * \param link The link to apply flags on
+ * \param flags The flags to apply to the link
+ *
+ * This function applies the link \a flags (as defined by the MEDIA_LNK_FL_*
+ * macros from the Media Controller API) to the given \a link. It implements
+ * low-level link setup as it performs no checks on the validity of the \a
+ * flags, and assumes that the supplied \a flags are valid for the link (e.g.
+ * immutable links cannot be disabled)."
+*
+ * \sa MediaLink::setEnable(bool enable)
+ *
+ * \return 0 for success, or a negative error code otherwise
+ */
+int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
+{
+	struct media_link_desc linkDesc = { };
+	MediaPad *source = link->source();
+	MediaPad *sink = link->sink();
+
+	linkDesc.source.entity = source->entity()->id();
+	linkDesc.source.index = source->index();
+	linkDesc.source.flags = MEDIA_PAD_FL_SOURCE;
+
+	linkDesc.sink.entity = sink->entity()->id();
+	linkDesc.sink.index = sink->index();
+	linkDesc.sink.flags = MEDIA_PAD_FL_SINK;
+
+	linkDesc.flags = flags;
+
+	int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
+	if (ret) {
+		ret = -errno;
+		LOG(Error) << "Failed to setup link: " << strerror(-ret);
+		return ret;
+	}
+
+	LOG(Debug) << source->entity()->name() << "["
+		   << source->index() << "] -> "
+		   << sink->entity()->name() << "["
+		   << sink->index() << "]: " << flags;
+
+	return 0;
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/media_object.cpp b/src/libcamera/media_object.cpp
index 612550d..c6cb02b 100644
--- a/src/libcamera/media_object.cpp
+++ b/src/libcamera/media_object.cpp
@@ -15,6 +15,7 @@ 
 #include <linux/media.h>

 #include "log.h"
+#include "media_device.h"
 #include "media_object.h"

 /**
@@ -89,6 +90,34 @@  namespace libcamera {
  * Each link is referenced in the link array of both of the pads it connect.
  */

+/**
+ * \brief Enable or disable a lik
+ * \param enable True to enable the link, false to disable it
+ *
+ * Set the status of a link, according to the value of \a enable.
+ * Links between two pads can be set to the enabled or disabled state freely,
+ * unless they're immutable links, whose status cannot be changed.
+ * Enabling an immutable link is not considered an error, while trying to
+ * disable it is.
+ *
+ * Enabling a link establishes a data connection between two pads, while
+ * disabling it interrupts such a connections.
+ *
+ * \return 0 on success, or a negative error code otherwise
+ */
+int MediaLink::setEnable(bool enable)
+{
+	unsigned int flags = enable ? MEDIA_LNK_FL_ENABLED : 0;
+
+	int ret = dev_->setupLink(this, flags);
+	if (ret)
+		return ret;
+
+	flags_ = flags;
+
+	return 0;
+}
+
 /**
  * \brief Construct a MediaLink
  * \param link The media link kernel data