Hi Kieran, On Tuesday, 12 September 2017 00:42:09 EEST Kieran Bingham wrote: > On 17/08/17 18:58, Laurent Pinchart wrote: > > 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@xxxxxxxxxxxxxxxx> > >> --- > >> > >> 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 ? > > No doubt about it, 168+, 153- is certainly intrusive. > > Yes, now I'm looking back at this, I think this does look like this part is > not quite the right approach. > > Which otherwise stalls the series until I have time to reconsider. I will > likely repost the work I have done on the earlier patches, including the > 's/fragment/body/g' changes and ready myself for a v4 which will contain the > heavier reworks. Fine with me. Could you make sure to mention the open issues in the cover letter ? I want to avoid commenting on them if you know already that you will rework them later. -- Regards, Laurent Pinchart