On 29/07/2022 20:00, Satish Nagireddy wrote:
On Fri, Jul 29, 2022 at 3:27 AM Tomi Valkeinen
<tomi.valkeinen@xxxxxxxxxxxxxxxx> wrote:
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;
}
The above code means that we do not support multiple pipelines per
entity. Or leave the implementation as is now and
this can be done as a different patch later, I will leave it to you.
He is what I'm thinking, assuming that every media_pad has it's own pipe.
struct media_pipeline *media_entity_pipeline(struct media_entity
*entity, u32 pad_index)
{
if (pad_index >= entity->num_pads)
return NULL;
return entity->pads[pad_index].pipe;
}
Oh, I see now where the confusion is.
media_entity_pipeline() is a convenience helper for the currently
existing drivers. They access entity->pipe directly (or when we move the
pipe to pad, entity->pads->pipe). So this function is just refactoring
the direct access away from the drivers, which makes it easier in the
future to find those drivers accessing the pipe, or change where the
pipe is stored without doing any changes to the drivers.
A driver which supports streams should not use this function, as there's
no "pipe for entity". We seem to be missing a similar helper function
for these cases, media_pad_pipeline() or such. We can add when needed.
Tomi
