LinuxTV Patchwork [v3,09/31] media: entity: Add media_has_route() function

login
register
mail settings
Submitter Jacopo Mondi
Date March 5, 2019, 6:51 p.m.
Message ID <20190305185150.20776-10-jacopo+renesas@jmondi.org>
Download mbox | patch
Permalink /patch/54861/
State Superseded
Headers show

Comments

Jacopo Mondi - March 5, 2019, 6:51 p.m.
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This is a wrapper around the media entity has_route operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/media-entity.c | 19 +++++++++++++++++++
 include/media/media-entity.h | 17 +++++++++++++++++
 2 files changed, 36 insertions(+)
Luca Ceresoli - March 14, 2019, 2:45 p.m.
Hi,

in the Subject line:
s/media_has_route/media_entity_has_route/

On 05/03/19 19:51, Jacopo Mondi wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> This is a wrapper around the media entity has_route operation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/media-entity.c | 19 +++++++++++++++++++
>  include/media/media-entity.h | 17 +++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 6f5196d05894..8e0ca8b1cfa2 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -238,6 +238,25 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
>   * Graph traversal
>   */
>  
> +bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> +			    unsigned int pad1)
> +{
> +	if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
> +		return false;
> +
> +	if (pad0 == pad1)
> +		return true;
> +
> +	if (!entity->ops || !entity->ops->has_route)
> +		return true;

Entities that implement has_route in following patches return false if
called with two sink pads or two source pads. This code behaves
differently. Which behavior is correct? IOW, how do you define "two
entity pads are connected internally"?

> +	if (entity->pads[pad1].index < entity->pads[pad0].index)
> +		swap(pad0, pad1);
> +
> +	return entity->ops->has_route(entity, pad0, pad1);
> +}
> +EXPORT_SYMBOL_GPL(media_entity_has_route);
> +
>  static struct media_pad *
>  media_pad_other(struct media_pad *pad, struct media_link *link)
>  {
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 675bc27b8b3c..205561545d7e 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -919,6 +919,23 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
>  __must_check int media_graph_walk_init(
>  	struct media_graph *graph, struct media_device *mdev);
>  
> +/**
> + * media_entity_has_route - Check if two entity pads are connected internally
> + *
> + * @entity: The entity
> + * @pad0: The first pad index
> + * @pad1: The second pad index
> + *
> + * This function can be used to check whether two pads of an entity are
> + * connected internally in the entity.
> + *
> + * The caller must hold entity->graph_obj.mdev->mutex.
> + *
> + * Return: true if the pads are connected internally and false otherwise.
> + */
> +bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> +			    unsigned int pad1);
> +
>  /**
>   * media_graph_walk_cleanup - Release resources used by graph walk.
>   *
>
jacopo@jmondi.org - March 15, 2019, 9:22 a.m.
Hi Luca,
   thanks for the comments

On Thu, Mar 14, 2019 at 03:45:00PM +0100, Luca Ceresoli wrote:
> Hi,
>
> in the Subject line:
> s/media_has_route/media_entity_has_route/

Ah! Thanks for noticing this.

>
> On 05/03/19 19:51, Jacopo Mondi wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > This is a wrapper around the media entity has_route operation.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/media-entity.c | 19 +++++++++++++++++++
> >  include/media/media-entity.h | 17 +++++++++++++++++
> >  2 files changed, 36 insertions(+)
> >
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index 6f5196d05894..8e0ca8b1cfa2 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -238,6 +238,25 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
> >   * Graph traversal
> >   */
> >
> > +bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> > +			    unsigned int pad1)
> > +{
> > +	if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
> > +		return false;
> > +
> > +	if (pad0 == pad1)
> > +		return true;
> > +
> > +	if (!entity->ops || !entity->ops->has_route)
> > +		return true;
>
> Entities that implement has_route in following patches return false if
> called with two sink pads or two source pads. This code behaves
> differently. Which behavior is correct? IOW, how do you define "two
> entity pads are connected internally"?

The handling of "indirect routes" (aka routes that connects two
sources or two sinks) is totally up to the device driver and we
decided not to make any assumption (nor introduce helpers, as it was
in v2) to support that in the framework.

Have a look at:
[PATCH v2 15/30] media: entity: Look for indirect routes
where Sakari implemented an helper to support the most common use case
of two source pads connected to the same sink pad. In this case the
two sources are reported as connected, but we decided for now to let
the drivers handle this, and more complex indirect routes, internaly.

The devices for which "has_route" has been implemented in this series
do not support indirect routes, for now, but the framework does not
make assumptions on this.

Thanks
   j

>
> > +	if (entity->pads[pad1].index < entity->pads[pad0].index)
> > +		swap(pad0, pad1);
> > +
> > +	return entity->ops->has_route(entity, pad0, pad1);
> > +}
> > +EXPORT_SYMBOL_GPL(media_entity_has_route);
> > +
> >  static struct media_pad *
> >  media_pad_other(struct media_pad *pad, struct media_link *link)
> >  {
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 675bc27b8b3c..205561545d7e 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -919,6 +919,23 @@ int media_entity_get_fwnode_pad(struct media_entity *entity,
> >  __must_check int media_graph_walk_init(
> >  	struct media_graph *graph, struct media_device *mdev);
> >
> > +/**
> > + * media_entity_has_route - Check if two entity pads are connected internally
> > + *
> > + * @entity: The entity
> > + * @pad0: The first pad index
> > + * @pad1: The second pad index
> > + *
> > + * This function can be used to check whether two pads of an entity are
> > + * connected internally in the entity.
> > + *
> > + * The caller must hold entity->graph_obj.mdev->mutex.
> > + *
> > + * Return: true if the pads are connected internally and false otherwise.
> > + */
> > +bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> > +			    unsigned int pad1);
> > +
> >  /**
> >   * media_graph_walk_cleanup - Release resources used by graph walk.
> >   *
> >
>
> --
> Luca

Patch

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 6f5196d05894..8e0ca8b1cfa2 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -238,6 +238,25 @@  EXPORT_SYMBOL_GPL(media_entity_pads_init);
  * Graph traversal
  */
 
+bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
+			    unsigned int pad1)
+{
+	if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
+		return false;
+
+	if (pad0 == pad1)
+		return true;
+
+	if (!entity->ops || !entity->ops->has_route)
+		return true;
+
+	if (entity->pads[pad1].index < entity->pads[pad0].index)
+		swap(pad0, pad1);
+
+	return entity->ops->has_route(entity, pad0, pad1);
+}
+EXPORT_SYMBOL_GPL(media_entity_has_route);
+
 static struct media_pad *
 media_pad_other(struct media_pad *pad, struct media_link *link)
 {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 675bc27b8b3c..205561545d7e 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -919,6 +919,23 @@  int media_entity_get_fwnode_pad(struct media_entity *entity,
 __must_check int media_graph_walk_init(
 	struct media_graph *graph, struct media_device *mdev);
 
+/**
+ * media_entity_has_route - Check if two entity pads are connected internally
+ *
+ * @entity: The entity
+ * @pad0: The first pad index
+ * @pad1: The second pad index
+ *
+ * This function can be used to check whether two pads of an entity are
+ * connected internally in the entity.
+ *
+ * The caller must hold entity->graph_obj.mdev->mutex.
+ *
+ * Return: true if the pads are connected internally and false otherwise.
+ */
+bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
+			    unsigned int pad1);
+
 /**
  * media_graph_walk_cleanup - Release resources used by graph walk.
  *

Privacy Policy