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

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

 



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+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.

--
Kieran



[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