Re: [PATCH v2 6/8] v4l: vsp1: Adapt entities to configure into a body

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Kieran,

Thank you for the patch.

On Monday 14 Aug 2017 16:13:29 Kieran Bingham wrote:
> Currently the entities store their configurations into a display list.
> Adapt this such that the code can be configured into a body fragment
> directly, allowing greater flexibility and control of the content.
> 
> All users of vsp1_dl_list_write() are removed in this process, thus it
> too is removed.
> 
> A helper, vsp1_dl_list_body() is provided to access the internal body0
> from the display list.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/vsp1/vsp1_bru.c    | 22 ++++++------
>  drivers/media/platform/vsp1/vsp1_clu.c    | 22 ++++++------
>  drivers/media/platform/vsp1/vsp1_dl.c     | 12 ++-----
>  drivers/media/platform/vsp1/vsp1_dl.h     |  2 +-
>  drivers/media/platform/vsp1/vsp1_drm.c    | 14 +++++---
>  drivers/media/platform/vsp1/vsp1_entity.c | 16 ++++-----
>  drivers/media/platform/vsp1/vsp1_entity.h | 12 ++++---
>  drivers/media/platform/vsp1/vsp1_hgo.c    | 16 ++++-----
>  drivers/media/platform/vsp1/vsp1_hgt.c    | 18 +++++-----
>  drivers/media/platform/vsp1/vsp1_hsit.c   | 10 +++---
>  drivers/media/platform/vsp1/vsp1_lif.c    | 13 +++----
>  drivers/media/platform/vsp1/vsp1_lut.c    | 21 ++++++------
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  4 +-
>  drivers/media/platform/vsp1/vsp1_pipe.h   |  3 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c    | 43 +++++++++++-------------
>  drivers/media/platform/vsp1/vsp1_sru.c    | 14 ++++----
>  drivers/media/platform/vsp1/vsp1_uds.c    | 24 +++++++------
>  drivers/media/platform/vsp1/vsp1_uds.h    |  2 +-
>  drivers/media/platform/vsp1/vsp1_video.c  | 11 ++++--
>  drivers/media/platform/vsp1/vsp1_wpf.c    | 42 ++++++++++++-----------
>  20 files changed, 168 insertions(+), 153 deletions(-)

This is quite intrusive, and it bothers me slightly that we need to pass both 
the DL and the DLB to the configure function in order to add fragments to the 
DL in the CLU and LUT modules. Wouldn't it be simpler to add a pointer to the 
current body in the DL structure, and modify vsp1_dl_list_write() to write to 
the current fragment ?

-- 
Regards,

Laurent Pinchart