Re: [PATCH v2 09/30] media: entity: Swap pads if route is checked from source to sink

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Tue, Jan 22, 2019 at 05:15:06PM +0200, Sakari Ailus wrote:
> On Wed, Jan 16, 2019 at 12:57:43AM +0200, Laurent Pinchart wrote:
> >> 
> >> This way the pads are always passed to the has_route() op sink pad first.
> >> Makes sense.
> > 
> > Is there anything in the API that mandates one pad to be a sink and the
> > other pad to the a source ? I had designed the operation to allow
> > sink-sink and source-source connections to be checked too.
> 
> Do you have a use case in mind for sink--sink or source--source routes? The
> routes are about flows of data, so I'd presume only source--sink or
> sink--source routes are meaningful.
> 
> If you did, then the driver would have to handle that by itself. This still
> simplifies the implementation for drivers that do not.

I don't have use cases for such routes, but we use the has_route
operation when traversing pipelines, and at that point we need to get
all the internally connected pads. In another patch in this series you
implement a helper function that handles this, but its implementation
isn't complete. I explained in my review of that patch that I fear a
correct generic implementation would become quite complex, while the
complexity should be easy to handle on the driver side as the code can
then be specialized for the case at hand.

> > If your goal is to simplify the implementation of the .has_route()
> > operation in drivers, I would instead sort pad0 and pad1 by value.
> 
> That'd be another option to make the order deterministic for the driver.
> I'm fine with that as well.
> 
> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> ---
> >>  drivers/media/media-entity.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> >> index 3c0e7425c8983b45..33f00e35ccd92c6f 100644
> >> --- a/drivers/media/media-entity.c
> >> +++ b/drivers/media/media-entity.c
> >> @@ -249,6 +249,10 @@ bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> >>  	if (!entity->ops || !entity->ops->has_route)
> >>  		return true;
> >>  
> >> +	if (entity->pads[pad0].flags & MEDIA_PAD_FL_SOURCE
> >> +	    && entity->pads[pad1].flags & MEDIA_PAD_FL_SINK)
> >> +		swap(pad0, pad1);
> >> +
> >>  	return entity->ops->has_route(entity, pad0, pad1);
> >>  }
> >>  EXPORT_SYMBOL_GPL(media_entity_has_route);

-- 
Regards,

Laurent Pinchart