Re: [PATCH v2 15/30] media: entity: Look for indirect routes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux