On 29/07/2022 12:19, Satish Nagireddy wrote:
On Fri, Jul 29, 2022 at 1:53 AM Tomi Valkeinen
<tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
On 29/07/2022 11:45, Satish Nagireddy wrote:
@@ -1011,7 +1342,7 @@ EXPORT_SYMBOL_GPL(media_entity_get_fwnode_pad);
struct media_pipeline *media_entity_pipeline(struct media_entity *entity)
{
- return entity->pipe;
+ return entity->pads->pipe;
I am not sure If it is always safe to return the pipe associated with
the first pad. I think this will work with all the existing drivers.
Let's say If pads of an entity are associated with different pipes,
this function might require extending the support of returning
pipe based on pad index. Please let me know your opinion.
That's true. The kdoc for this function says:
* In general, entities can be part of multiple pipelines, when carrying
* multiple streams (either on different pads, or on the same pad using
* multiplexed streams). This function is ill-defined in that case. It
* currently returns the pipeline associated with the first pad of the
entity.
I did consider adding a warning if the function is called for entities
with more than one pad. But that probably would give false warnings,
e.g. for a simple entity with one sink and one source pad. In that case
both pads are always part of the same pipeline, and
media_entity_pipeline() works correctly.
We could perhaps add a check here which verifies that all the pads in
the entity have the same pipe.
Perhaps something like:
struct media_pipeline *media_entity_pipeline(struct media_entity *entity)
{
struct media_pipeline *pipe;
struct media_pad *pad;
if (entity->num_pads == 0)
return NULL;
pipe = entity->pads->pipe;
media_entity_for_each_pad(entity, pad) {
if (WARN_ON(pad->pipe != pipe))
return NULL;
}
return pipe;
}
}
EXPORT_SYMBOL_GPL(media_entity_pipeline);
nit, It would be nice to rename this function to media_entity_get_pipe
or media_entity_get_pipeline for better readability.
I'm ok with that, but we do have other functions with this style:
media_entity_remote_pad(), media_entity_id(), ...
Tomi
I could only see one function with the similar style ==>
media_entity_get_fwnode_pad
Right, so, do you agree that we should keep the name as
media_entity_pipeline, as we already have many other functions with
similar style?
Tomi
