On 16/12/2022 02:36, Laurent Pinchart wrote:
Hi Tomi,
On Thu, Dec 15, 2022 at 03:29:13PM +0200, Tomi Valkeinen wrote:
On 12/12/2022 16:16, Laurent Pinchart wrote:
Replace usage of the deprecated media graph walk API with the new
media_pipeline_for_each_pad() macro.
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
drivers/media/platform/xilinx/xilinx-dma.c | 28 +++++-----------------
1 file changed, 6 insertions(+), 22 deletions(-)
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
index 0a7fd8642a65..fee02c8c85fd 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -173,31 +173,19 @@ static int xvip_pipeline_set_stream(struct xvip_pipeline *pipe, bool on)
static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
struct xvip_dma *start)
{
- struct media_graph graph;
- struct media_entity *entity = &start->video.entity;
- struct media_device *mdev = entity->graph_obj.mdev;
+ struct media_pipeline_pad_iter iter;
unsigned int num_inputs = 0;
unsigned int num_outputs = 0;
- int ret;
+ struct media_pad *pad;
- mutex_lock(&mdev->graph_mutex);
-
- /* Walk the graph to locate the video nodes. */
- ret = media_graph_walk_init(&graph, mdev);
- if (ret) {
- mutex_unlock(&mdev->graph_mutex);
- return ret;
- }
-
- media_graph_walk_start(&graph, entity);
-
- while ((entity = media_graph_walk_next(&graph))) {
+ /* Locate the video nodes in the pipeline. */
+ media_pipeline_for_each_pad(&pipe->pipe, &iter, pad) {
struct xvip_dma *dma;
- if (entity->function != MEDIA_ENT_F_IO_V4L)
+ if (pad->entity->function != MEDIA_ENT_F_IO_V4L)
continue;
Why do you iterate the pads here, instead of using
media_pipeline_for_each_entity()?
The entity iterator still iterates over all pads internally, so it's not
more efficient. It also requires explicit initialization and cleanup, so
is more complicated to use. In this specific case, the driver ignores
all pads belonging to entities that are not V4L2 video nodes
(MEDIA_ENT_F_IO_V4L). As such entities have a single pad, there's a
guarantee they will not be visited twice.
If you think this is too fragile due to the assumption that
MEDIA_ENT_F_IO_V4L entities have a single pad, I can use the entity
iterator, it's not a big deal.
Well, the reason I commented was that the old code iterates entities, in
this series you add a new way to iterate entities, and then... you don't
use it for iterating entities =).
I think it hints that the entity iteration is not as good as one could
wish, but I don't right away see the means to improve it. I do miss C++
in cases like this.
I think I would have gone with the entity iterator even if it's slightly
more complex to use, just to highlight the point that this is iterating
entities. But I don't mind either way.
Tomi
