Re: [PATCH v2 12/19] media: renesas: vsp1: Keep the DRM pipeline entities sorted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Laurent Pinchart (2024-06-19 01:17:15)
> Some of the code that handles pipeline configuration assumes that
> entities in a pipeline's entities list are sorted from sink to source.
> To prepare for using that code with the DRM pipeline, insert the BRx
> just before the WPF, and the RPFs at the head of the list.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_drm.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index 1aa59a74672f..e44359b661b6 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -317,7 +317,10 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
>                         list_add_tail(&released_brx->list_pipe,
>                                       &pipe->entities);
>  
> -               /* Add the BRx to the pipeline. */
> +               /*
> +                * Add the BRx to the pipeline, inserting it just before the
> +                * WPF.

So - the pipe->output is from what I recall/can see the output wpf.
 (struct vsp1_rwpf *output)

> +                */
>                 dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
>                         __func__, pipe->lif->index, BRX_NAME(brx));
>  
> @@ -326,7 +329,8 @@ static int vsp1_du_pipeline_setup_brx(struct vsp1_device *vsp1,
>                 pipe->brx->sink = &pipe->output->entity;
>                 pipe->brx->sink_pad = 0;
>  
> -               list_add_tail(&pipe->brx->list_pipe, &pipe->entities);
> +               list_add_tail(&pipe->brx->list_pipe,
> +                             &pipe->output->entity.list_pipe);

But this reads to me as if we're adding the brx after ('the tail') of
the output WPF....

Now ... of course if we open up list_add_tail()

 * Insert a new entry before the specified head.

And that checks out - because of course the list_add adds it as the
'next' item in the list... and we're using list_add_tail as a convenient
way to provide list_add_before() ...

So I believe this is correct, but the nuance of it reads back to front to me.

Because of that it possibly deserves a better comment to be explicit on
what it's doing, or makes me wonder if list.h should have something that
explicitly impliments

#define list_add_before list_add_tail

but otherwise - it does check out.


Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>

>         }
>  
>         /*
> @@ -420,7 +424,7 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1,
>  
>                 if (!rpf->entity.pipe) {
>                         rpf->entity.pipe = pipe;
> -                       list_add_tail(&rpf->entity.list_pipe, &pipe->entities);
> +                       list_add(&rpf->entity.list_pipe, &pipe->entities);
>                 }
>  
>                 brx->inputs[i].rpf = rpf;
> -- 
> Regards,
> 
> Laurent Pinchart
>





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux