| Message ID | 20190305185150.20776-10-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | v4l: add support for multiplexed streams | expand |
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. > * >
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
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. *