| Message ID | 20181101233144.31507-10-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | v4l: add support for multiplexed streams | expand |
Hi Sakari, Thank you for the patch. On Fri, Nov 02, 2018 at 12:31:23AM +0100, Niklas Söderlund wrote: > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > 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. If your goal is to simplify the implementation of the .has_route() operation in drivers, I would instead sort pad0 and pad1 by value. > 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);
Hi Laurent, On Wed, Jan 16, 2019 at 12:57:43AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Fri, Nov 02, 2018 at 12:31:23AM +0100, Niklas Söderlund wrote: > > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > 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. > > 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
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);
Hi Laurent, Sakari, On Tue, Jan 22, 2019 at 05:20:30PM +0200, Laurent Pinchart wrote: > 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. > As a compromise, in v3 I'm thinking of maintaining support for the most common case of two sources connected to the same sink, as Sakari's patch does, but let more complex cases be handled by the driver implementation of has_route(). Ack? > > > 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. > > In v3 I have taken both suggestions in: try the "sink then source" order first, then order by index in case the pads are of the same time. This needs to be documented in has_route() operation definition though. Would that be fine with you? Thanks j > > >> 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
Hi Jacopo, On Mon, Feb 18, 2019 at 10:21:07AM +0100, Jacopo Mondi wrote: > On Tue, Jan 22, 2019 at 05:20:30PM +0200, Laurent Pinchart wrote: > > 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. > > > > As a compromise, in v3 I'm thinking of maintaining support for the > most common case of two sources connected to the same sink, as > Sakari's patch does, but let more complex cases be handled by the > driver implementation of has_route(). > > Ack? I fear this will be confusing for subdevs, as they would have to implement part of the operation. Could it be that the subdev has_route operation isn't the best API for the job, if it gets that complex ? I wonder if it would be easier to create another operation that takes a pad index as argument, and returns the list of pads (possibly as a bitmask ?) or connected pads. media_entity_has_route() could easily be implemented on top of that, and these new semantics may be easier for subdevs to implement. > >>> 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. > > In v3 I have taken both suggestions in: try the "sink then source" order > first, then order by index in case the pads are of the same time. This > needs to be documented in has_route() operation definition though. > > Would that be fine with you? I think that's the worst of both worlds from a subdev point of view :-) > >>>> 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);
Hi Laurent, On Fri, Feb 22, 2019 at 02:18:11PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Mon, Feb 18, 2019 at 10:21:07AM +0100, Jacopo Mondi wrote: > > On Tue, Jan 22, 2019 at 05:20:30PM +0200, Laurent Pinchart wrote: > > > 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. > > > > > > > As a compromise, in v3 I'm thinking of maintaining support for the > > most common case of two sources connected to the same sink, as > > Sakari's patch does, but let more complex cases be handled by the > > driver implementation of has_route(). > > > > Ack? > > I fear this will be confusing for subdevs, as they would have to > implement part of the operation. > > Could it be that the subdev has_route operation isn't the best API for > the job, if it gets that complex ? I wonder if it would be easier to > create another operation that takes a pad index as argument, and returns > the list of pads (possibly as a bitmask ?) or connected pads. > media_entity_has_route() could easily be implemented on top of that, and > these new semantics may be easier for subdevs to implement. > I see, but if subdevs can easily elaborate that list, they could as well easily check if the pad provided as argument is on that list. > > >>> 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. > > > > In v3 I have taken both suggestions in: try the "sink then source" order > > first, then order by index in case the pads are of the same time. This > > needs to be documented in has_route() operation definition though. > > > > Would that be fine with you? > > I think that's the worst of both worlds from a subdev point of view :-) > Possibly :) Should we drop completely the sink-source ordering in favour of ordering by value, and drop [15/30] that adds support for trivial indirect routes? Let's reach consensus so I could send v3. Thanks j > > >>>> 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
Hi Jacopo, On Mon, Mar 04, 2019 at 01:35:36PM +0100, Jacopo Mondi wrote: > On Fri, Feb 22, 2019 at 02:18:11PM +0200, Laurent Pinchart wrote: > > On Mon, Feb 18, 2019 at 10:21:07AM +0100, Jacopo Mondi wrote: > >> On Tue, Jan 22, 2019 at 05:20:30PM +0200, Laurent Pinchart wrote: > >>> 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. > >>> > >> > >> As a compromise, in v3 I'm thinking of maintaining support for the > >> most common case of two sources connected to the same sink, as > >> Sakari's patch does, but let more complex cases be handled by the > >> driver implementation of has_route(). > >> > >> Ack? > > > > I fear this will be confusing for subdevs, as they would have to > > implement part of the operation. > > > > Could it be that the subdev has_route operation isn't the best API for > > the job, if it gets that complex ? I wonder if it would be easier to > > create another operation that takes a pad index as argument, and returns > > the list of pads (possibly as a bitmask ?) or connected pads. > > media_entity_has_route() could easily be implemented on top of that, and > > these new semantics may be easier for subdevs to implement. > > > > I see, but if subdevs can easily elaborate that list, they could as > well easily check if the pad provided as argument is on that list. Possibly. In any case, if we keep this operation as-is, I wouldn't try to split the logic between the subdev drivers and the core, that would be asking for trouble. If it gets too complex to implement for subdev drivers, then we need a different operation with a different logic in the subdev API, and a helper that wraps around it. > >>>>> 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. > >> > >> In v3 I have taken both suggestions in: try the "sink then source" order > >> first, then order by index in case the pads are of the same time. This > >> needs to be documented in has_route() operation definition though. > >> > >> Would that be fine with you? > > > > I think that's the worst of both worlds from a subdev point of view :-) > > Possibly :) > > Should we drop completely the sink-source ordering in favour of > ordering by value, and drop [15/30] that adds support for trivial > indirect routes? > > Let's reach consensus so I could send v3. I would certainly drop 15/30, and I don't think ordering by value would help subdev drivers much. > >>>>>> 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);
HI Laurent, On Tue, Mar 05, 2019 at 10:04:58PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Mon, Mar 04, 2019 at 01:35:36PM +0100, Jacopo Mondi wrote: > > On Fri, Feb 22, 2019 at 02:18:11PM +0200, Laurent Pinchart wrote: > > > On Mon, Feb 18, 2019 at 10:21:07AM +0100, Jacopo Mondi wrote: > > >> On Tue, Jan 22, 2019 at 05:20:30PM +0200, Laurent Pinchart wrote: > > >>> 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. > > >>> > > >> > > >> As a compromise, in v3 I'm thinking of maintaining support for the > > >> most common case of two sources connected to the same sink, as > > >> Sakari's patch does, but let more complex cases be handled by the > > >> driver implementation of has_route(). > > >> > > >> Ack? > > > > > > I fear this will be confusing for subdevs, as they would have to > > > implement part of the operation. > > > > > > Could it be that the subdev has_route operation isn't the best API for > > > the job, if it gets that complex ? I wonder if it would be easier to > > > create another operation that takes a pad index as argument, and returns > > > the list of pads (possibly as a bitmask ?) or connected pads. > > > media_entity_has_route() could easily be implemented on top of that, and > > > these new semantics may be easier for subdevs to implement. > > > > > > > I see, but if subdevs can easily elaborate that list, they could as > > well easily check if the pad provided as argument is on that list. > > Possibly. In any case, if we keep this operation as-is, I wouldn't try > to split the logic between the subdev drivers and the core, that would > be asking for trouble. If it gets too complex to implement for subdev > drivers, then we need a different operation with a different logic in > the subdev API, and a helper that wraps around it. In v3 I have removed support for indirect routes from the framework part. It's all on the subdevice driver for now. > > > >>>>> 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. > > >> > > >> In v3 I have taken both suggestions in: try the "sink then source" order > > >> first, then order by index in case the pads are of the same time. This > > >> needs to be documented in has_route() operation definition though. > > >> > > >> Would that be fine with you? > > > > > > I think that's the worst of both worlds from a subdev point of view :-) > > > > Possibly :) > > > > Should we drop completely the sink-source ordering in favour of > > ordering by value, and drop [15/30] that adds support for trivial > > indirect routes? > > > > Let's reach consensus so I could send v3. > > I would certainly drop 15/30, and I don't think ordering by value would > help subdev drivers much. Yes, but sorting by index makes it easier to deal with the sink-sink and source-source use cases, if the subdevice supports indirect routes. I have dropped 15/30 and specified pads are passed by index in v3. Thanks j > > > >>>>>> 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
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);