| Message ID | 20200420003930.11463-3-slongerbeam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: Create media links in bound notifiers | expand |
Hi Steve, On Sun, Apr 19, 2020 at 05:39:09PM -0700, Steve Longerbeam wrote: > Modify the default behavior of media_entity_get_fwnode_pad() (when the > entity does not provide the get_fwnode_pad op) to first assume the entity > implements a 1:1 mapping between fwnode port number and media pad index. > > If the 1:1 mapping is not valid, e.g. the port number falls outside the > entity's pad index range, or the pad at that port number doesn't match the > given direction_flags, fall-back to the previous behavior that searches > the entity for the first pad that matches the given direction_flags. > > The previous default behavior can choose the wrong pad for entities with > multiple sink or source pads. With this change the function will choose > the correct pad index if the entity implements a 1:1 port to pad mapping > at that port. > > Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> > --- > drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++----- > include/media/media-entity.h | 6 ++++-- > 2 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > index 12b45e669bcc..b1e0259a58c5 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity, > unsigned long direction_flags) > { > struct fwnode_endpoint endpoint; > - unsigned int i; > int ret; > > + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); > + if (ret) > + return ret; > + > if (!entity->ops || !entity->ops->get_fwnode_pad) { > + unsigned int i; > + > + /* > + * for the default case, first try a 1:1 mapping between > + * fwnode port number and pad index. > + */ > + ret = endpoint.port; > + if (ret < entity->num_pads && > + (entity->pads[ret].flags & direction_flags)) > + return ret; Given the 3rd patch, is this one still meant to be here? > + > + /* > + * if that didn't work search the entity for the first > + * pad that matches the @direction_flags. > + */ > for (i = 0; i < entity->num_pads; i++) { > if (entity->pads[i].flags & direction_flags) > return i; > } > > + /* else fail */ > return -ENXIO; > } > > - ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); > - if (ret) > - return ret; > - > ret = entity->ops->get_fwnode_pad(entity, &endpoint); > if (ret < 0) > return ret; > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index cde80ad029b7..9316eb9f8486 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -871,8 +871,10 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); > * mappings of media pads. > * > * If the entity does not implement the get_fwnode_pad() operation > - * then this function searches the entity for the first pad that > - * matches the @direction_flags. > + * then this function first assumes the entity implements a 1:1 mapping > + * between fwnode port number and media pad index. If the 1:1 mapping > + * is not valid, then the function searches the entity for the first pad > + * that matches the @direction_flags. > * > * Return: returns the pad number on success or a negative error code. > */
On 4/29/20 7:42 AM, Sakari Ailus wrote: > Hi Steve, > > On Sun, Apr 19, 2020 at 05:39:09PM -0700, Steve Longerbeam wrote: >> Modify the default behavior of media_entity_get_fwnode_pad() (when the >> entity does not provide the get_fwnode_pad op) to first assume the entity >> implements a 1:1 mapping between fwnode port number and media pad index. >> >> If the 1:1 mapping is not valid, e.g. the port number falls outside the >> entity's pad index range, or the pad at that port number doesn't match the >> given direction_flags, fall-back to the previous behavior that searches >> the entity for the first pad that matches the given direction_flags. >> >> The previous default behavior can choose the wrong pad for entities with >> multiple sink or source pads. With this change the function will choose >> the correct pad index if the entity implements a 1:1 port to pad mapping >> at that port. >> >> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> >> --- >> drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++----- >> include/media/media-entity.h | 6 ++++-- >> 2 files changed, 24 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c >> index 12b45e669bcc..b1e0259a58c5 100644 >> --- a/drivers/media/mc/mc-entity.c >> +++ b/drivers/media/mc/mc-entity.c >> @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity, >> unsigned long direction_flags) >> { >> struct fwnode_endpoint endpoint; >> - unsigned int i; >> int ret; >> >> + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); >> + if (ret) >> + return ret; >> + >> if (!entity->ops || !entity->ops->get_fwnode_pad) { >> + unsigned int i; >> + >> + /* >> + * for the default case, first try a 1:1 mapping between >> + * fwnode port number and pad index. >> + */ >> + ret = endpoint.port; >> + if (ret < entity->num_pads && >> + (entity->pads[ret].flags & direction_flags)) >> + return ret; > Given the 3rd patch, is this one still meant to be here? It's true, it's not needed anymore, at least for imx, since all the imx devices have implemented get_fwnode_pad. I decided to leave it here anyway, since it does make some sense. But I have no problem removing it and possibly revisit this later. Steve > >> + >> + /* >> + * if that didn't work search the entity for the first >> + * pad that matches the @direction_flags. >> + */ >> for (i = 0; i < entity->num_pads; i++) { >> if (entity->pads[i].flags & direction_flags) >> return i; >> } >> >> + /* else fail */ >> return -ENXIO; >> } >> >> - ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); >> - if (ret) >> - return ret; >> - >> ret = entity->ops->get_fwnode_pad(entity, &endpoint); >> if (ret < 0) >> return ret; >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h >> index cde80ad029b7..9316eb9f8486 100644 >> --- a/include/media/media-entity.h >> +++ b/include/media/media-entity.h >> @@ -871,8 +871,10 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); >> * mappings of media pads. >> * >> * If the entity does not implement the get_fwnode_pad() operation >> - * then this function searches the entity for the first pad that >> - * matches the @direction_flags. >> + * then this function first assumes the entity implements a 1:1 mapping >> + * between fwnode port number and media pad index. If the 1:1 mapping >> + * is not valid, then the function searches the entity for the first pad >> + * that matches the @direction_flags. >> * >> * Return: returns the pad number on success or a negative error code. >> */
Hi Steve, On Wed, Apr 29, 2020 at 01:56:33PM -0700, Steve Longerbeam wrote: > > > On 4/29/20 7:42 AM, Sakari Ailus wrote: > > Hi Steve, > > > > On Sun, Apr 19, 2020 at 05:39:09PM -0700, Steve Longerbeam wrote: > > > Modify the default behavior of media_entity_get_fwnode_pad() (when the > > > entity does not provide the get_fwnode_pad op) to first assume the entity > > > implements a 1:1 mapping between fwnode port number and media pad index. > > > > > > If the 1:1 mapping is not valid, e.g. the port number falls outside the > > > entity's pad index range, or the pad at that port number doesn't match the > > > given direction_flags, fall-back to the previous behavior that searches > > > the entity for the first pad that matches the given direction_flags. > > > > > > The previous default behavior can choose the wrong pad for entities with > > > multiple sink or source pads. With this change the function will choose > > > the correct pad index if the entity implements a 1:1 port to pad mapping > > > at that port. > > > > > > Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> > > > --- > > > drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++----- > > > include/media/media-entity.h | 6 ++++-- > > > 2 files changed, 24 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > > > index 12b45e669bcc..b1e0259a58c5 100644 > > > --- a/drivers/media/mc/mc-entity.c > > > +++ b/drivers/media/mc/mc-entity.c > > > @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity, > > > unsigned long direction_flags) > > > { > > > struct fwnode_endpoint endpoint; > > > - unsigned int i; > > > int ret; > > > + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); > > > + if (ret) > > > + return ret; > > > + > > > if (!entity->ops || !entity->ops->get_fwnode_pad) { > > > + unsigned int i; > > > + > > > + /* > > > + * for the default case, first try a 1:1 mapping between > > > + * fwnode port number and pad index. > > > + */ > > > + ret = endpoint.port; > > > + if (ret < entity->num_pads && > > > + (entity->pads[ret].flags & direction_flags)) > > > + return ret; > > Given the 3rd patch, is this one still meant to be here? > > It's true, it's not needed anymore, at least for imx, since all the imx > devices have implemented get_fwnode_pad. I decided to leave it here anyway, > since it does make some sense. But I have no problem removing it and > possibly revisit this later. Please. We could remove the other heuristics and move it into drivers (perhaps with a similar helper), but that's for later.
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c index 12b45e669bcc..b1e0259a58c5 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -370,22 +370,37 @@ int media_entity_get_fwnode_pad(struct media_entity *entity, unsigned long direction_flags) { struct fwnode_endpoint endpoint; - unsigned int i; int ret; + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); + if (ret) + return ret; + if (!entity->ops || !entity->ops->get_fwnode_pad) { + unsigned int i; + + /* + * for the default case, first try a 1:1 mapping between + * fwnode port number and pad index. + */ + ret = endpoint.port; + if (ret < entity->num_pads && + (entity->pads[ret].flags & direction_flags)) + return ret; + + /* + * if that didn't work search the entity for the first + * pad that matches the @direction_flags. + */ for (i = 0; i < entity->num_pads; i++) { if (entity->pads[i].flags & direction_flags) return i; } + /* else fail */ return -ENXIO; } - ret = fwnode_graph_parse_endpoint(fwnode, &endpoint); - if (ret) - return ret; - ret = entity->ops->get_fwnode_pad(entity, &endpoint); if (ret < 0) return ret; diff --git a/include/media/media-entity.h b/include/media/media-entity.h index cde80ad029b7..9316eb9f8486 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -871,8 +871,10 @@ struct media_pad *media_entity_remote_pad(const struct media_pad *pad); * mappings of media pads. * * If the entity does not implement the get_fwnode_pad() operation - * then this function searches the entity for the first pad that - * matches the @direction_flags. + * then this function first assumes the entity implements a 1:1 mapping + * between fwnode port number and media pad index. If the 1:1 mapping + * is not valid, then the function searches the entity for the first pad + * that matches the @direction_flags. * * Return: returns the pad number on success or a negative error code. */
Modify the default behavior of media_entity_get_fwnode_pad() (when the entity does not provide the get_fwnode_pad op) to first assume the entity implements a 1:1 mapping between fwnode port number and media pad index. If the 1:1 mapping is not valid, e.g. the port number falls outside the entity's pad index range, or the pad at that port number doesn't match the given direction_flags, fall-back to the previous behavior that searches the entity for the first pad that matches the given direction_flags. The previous default behavior can choose the wrong pad for entities with multiple sink or source pads. With this change the function will choose the correct pad index if the entity implements a 1:1 port to pad mapping at that port. Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com> --- drivers/media/mc/mc-entity.c | 25 ++++++++++++++++++++----- include/media/media-entity.h | 6 ++++-- 2 files changed, 24 insertions(+), 7 deletions(-)