Hi Niklas,
Thank you for the patch.
On Fri, Nov 02, 2018 at 12:31:29AM +0100, Niklas Söderlund wrote:
> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>
> Two pads are considered having an active route for the purpose of
> has_route() if an indirect active route can be found between the two pads.
> An simple example of this is that a source pad has an active route to
> another source pad if both of the pads have an active route to the same
> sink pad.
>
> Make media_entity_has_route() return true in that case, and do not rely on
> drivers performing this by themselves.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> drivers/media/media-entity.c | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 42977634d7102852..e45fc2549017615a 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -240,6 +240,9 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
> bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> unsigned int pad1)
> {
> + unsigned int i;
> + bool has_route;
> +
> if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
> return false;
>
> @@ -253,7 +256,34 @@ bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> && entity->pads[pad1].flags & MEDIA_PAD_FL_SINK)
> swap(pad0, pad1);
>
> - return entity->ops->has_route(entity, pad0, pad1);
> + has_route = entity->ops->has_route(entity, pad0, pad1);
> + /* A direct route is returned immediately */
> + if (has_route ||
> + (entity->pads[pad0].flags & MEDIA_PAD_FL_SINK &&
> + entity->pads[pad1].flags & MEDIA_PAD_FL_SOURCE))
> + return true;
This will return true if pad0 is a sink and pad1 a source, regardless of
has_route. I don't think that was intended. return has_route; would be
an easy fix.
> +
> + /* Look for indirect routes */
> + for (i = 0; i < entity->num_pads; i++) {
> + if (i == pad0 || i == pad1)
> + continue;
> +
> + /*
> + * There are no direct routes between same types of
> + * pads, so skip checking this route
> + */
> + if (!((entity->pads[pad0].flags ^ entity->pads[i].flags) &
> + (MEDIA_PAD_FL_SOURCE | MEDIA_PAD_FL_SINK)))
> + continue;
> +
> + /* Is there an indirect route? */
> + if (entity->ops->has_route(entity, i, pad0) &&
> + entity->ops->has_route(entity, i, pad1))
> + return true;
> + }
Isn't this best implemented in drivers ? I fear the complexity you need
here isn't worth it, especially given that you would also need to
support cases such as
Pads 0, 1 and 2 are sink, pads 3 and 4 are sources. has_route(0, 3),
has_route(1, 3), has_route(1, 4) and has_route(2, 4) are all true,
has_route(0, 4) and has_route(2, 3) are all false.
media_entity_has_route(0, 2) should return true.
> + return false;
> +
> }
> EXPORT_SYMBOL_GPL(media_entity_has_route);
>
--
Regards,
Laurent Pinchart
