Hi Laurent, On 17/08/17 18:58, Laurent Pinchart wrote: > 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+rene...@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 ? >
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. -- Kieran
