Re: [PATCH v7 0/8] vsp1: TLB optimisation and DL caching
Hi Kieran, I've finished reviewing the series. For your convenience, I've rebased it on top of the BRU/BRS dynamic allocation patches, and pushed the result to git://linuxtv.org/pinchartl/media.git v4l2/vsp1/tlb-optimise (Please note it has been compile-tested only) I have also taken the liberty to incorporate both my review comments and my Reviewed-by line for the patches that have received a conditional Reviewed-by, that is patches 1/8, 5/8 and 7/8. Patches 3/8, 4/8, 6/8 and 8/8 have open questions so I haven't touched them. Please don't despair, v8 might well be the last version we will need :-) On Thursday, 8 March 2018 02:05:23 EEST Kieran Bingham wrote: > Each display list currently allocates an area of DMA memory to store > register settings for the VSP1 to process. Each of these allocations adds > pressure to the IPMMU TLB entries. > > We can reduce the pressure by pre-allocating larger areas and dividing the > area across multiple bodies represented as a pool. > > With this reconfiguration of bodies, we can adapt the configuration code to > separate out constant hardware configuration and cache it for re-use. > > The patches provided in this series can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git > tags/vsp1/tlb-optimise/v7 > > I hope that this series is at a stage where it could be integrated now. It > has had some thorough testing and is already integrated in both > renesas-drivers and renesas-bsp. (except for the minor changes in v7 that > is...) > > Please note that checkpatch complains on patch 6/8 in this series: > > v7-0006-media-vsp1-Refactor-display-list-configure-operations.patch > > -- WARNING: function definition argument 'struct > vsp1_entity *' should also have an identifier name #290: FILE: > drivers/media/platform/vsp1/vsp1_entity.h:82: > + void (*configure_stream)(struct vsp1_entity *, struct vsp1_pipeline > *, > > However - this complaint is regarding pre-existing code. I have only renamed > the function pointers. I do also disagree with checkpatch here - as there > is no need to provide an identifier name, and it does not improve > readability in this instance to state: > ...(vsp1_entity *entity, struct vsp1_pipeline *pipe) > > Thus - I have ignored these warnings. > > > Changelog: > -- > > v7: > - Rebased on to linux-media/master (v4.16-rc4) > - Clean up the formatting of the vsp1_dl_list_add_body() > - Fix formatting and white space > - s/prepare/configure_stream/ > - s/configure/configure_frame/ > > v6: > - Rebased on to linux-media/master (v4.16-rc1) > - Removed DRM/UIF (DISCOM/ColorKey) updates > > v5: > - Rebased on to renesas-drivers-2018-01-09-v4.15-rc7 to fix conflicts >with DRM and UIF updates on VSP1 driver > > v4: > - Rebased to v4.14 > * v4l: vsp1: Use reference counting for bodies >- Fix up reference handling comments > > * v4l: vsp1: Provide a body pool >- Provide comment explaining extra allocation on body pool > highlighting area for optimisation later. > > * v4l: vsp1: Refactor display list configure operations >- Fix up comment to describe yuv_mode caching rather than format > > * vsp1: Adapt entities to configure into a body >- Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0() > > * v4l: vsp1: Move video configuration to a cached dlb >- Adjust pipe configured flag to be reset on resume rather than suspend >- rename dl_child, dl_next > > Testing: > > The VSP unit tests have been run on this patch set with the following > results: > > --- Test loop 1 --- > - vsp-unit-test-.sh > Test Conditions: > Platform Renesas Salvator-X 2nd version board based on r8a7795 > ES2.0+ Kernel release4.16.0-rc4-arm64-renesas-01067-g397eb3811ec0 > convert /usr/bin/convert > compare /usr/bin/compare > killall /usr/bin/killall > raw2rgbpnm/usr/bin/raw2rgbpnm > stress/usr/bin/stress > yavta /usr/bin/yavta > - vsp-unit-test-0001.sh > Testing WPF packing in RGB332: pass > Testing WPF packing in ARGB555: pass > Testing WPF packing in XRGB555: pass > Testing WPF packing in RGB565: pass > Testing WPF packing in BGR24: pass > Testing WPF packing in RGB24: pass > Testing WPF packing in ABGR32: pass > Testing WPF packing in ARGB32: pass > Testing WPF packing in XBGR32: pass > Testing WPF packing in XRGB32: pass > - vsp-unit-test-0002.sh > Testing WPF packing in NV12M: pass > Testing WPF packing in NV16M: pass > Testing WPF packing in NV21M: pass > Testing WPF packing in NV61M: pass > Testing WPF packing in UYVY: pass > Testing WPF packing in VYUY: skip > Testing WPF packing in YUV420M: pass > Testing WPF packing in YUV422M: pass > Testing WPF packing in YUV444M: pass > Testing WPF packing in YVU420M: pass > Testing WPF packing in YVU422M:
[PATCH v7 0/8] vsp1: TLB optimisation and DL caching
Each display list currently allocates an area of DMA memory to store register settings for the VSP1 to process. Each of these allocations adds pressure to the IPMMU TLB entries. We can reduce the pressure by pre-allocating larger areas and dividing the area across multiple bodies represented as a pool. With this reconfiguration of bodies, we can adapt the configuration code to separate out constant hardware configuration and cache it for re-use. The patches provided in this series can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git tags/vsp1/tlb-optimise/v7 I hope that this series is at a stage where it could be integrated now. It has had some thorough testing and is already integrated in both renesas-drivers and renesas-bsp. (except for the minor changes in v7 that is...) Please note that checkpatch complains on patch 6/8 in this series: v7-0006-media-vsp1-Refactor-display-list-configure-operations.patch -- WARNING: function definition argument 'struct vsp1_entity *' should also have an identifier name #290: FILE: drivers/media/platform/vsp1/vsp1_entity.h:82: + void (*configure_stream)(struct vsp1_entity *, struct vsp1_pipeline *, However - this complaint is regarding pre-existing code. I have only renamed the function pointers. I do also disagree with checkpatch here - as there is no need to provide an identifier name, and it does not improve readability in this instance to state: ...(vsp1_entity *entity, struct vsp1_pipeline *pipe) Thus - I have ignored these warnings. Changelog: -- v7: - Rebased on to linux-media/master (v4.16-rc4) - Clean up the formatting of the vsp1_dl_list_add_body() - Fix formatting and white space - s/prepare/configure_stream/ - s/configure/configure_frame/ v6: - Rebased on to linux-media/master (v4.16-rc1) - Removed DRM/UIF (DISCOM/ColorKey) updates v5: - Rebased on to renesas-drivers-2018-01-09-v4.15-rc7 to fix conflicts with DRM and UIF updates on VSP1 driver v4: - Rebased to v4.14 * v4l: vsp1: Use reference counting for bodies - Fix up reference handling comments * v4l: vsp1: Provide a body pool - Provide comment explaining extra allocation on body pool highlighting area for optimisation later. * v4l: vsp1: Refactor display list configure operations - Fix up comment to describe yuv_mode caching rather than format * vsp1: Adapt entities to configure into a body - Rename vsp1_dl_list_get_body() to vsp1_dl_list_get_body0() * v4l: vsp1: Move video configuration to a cached dlb - Adjust pipe configured flag to be reset on resume rather than suspend - rename dl_child, dl_next Testing: The VSP unit tests have been run on this patch set with the following results: --- Test loop 1 --- - vsp-unit-test-.sh Test Conditions: Platform Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ Kernel release4.16.0-rc4-arm64-renesas-01067-g397eb3811ec0 convert /usr/bin/convert compare /usr/bin/compare killall /usr/bin/killall raw2rgbpnm/usr/bin/raw2rgbpnm stress/usr/bin/stress yavta /usr/bin/yavta - vsp-unit-test-0001.sh Testing WPF packing in RGB332: pass Testing WPF packing in ARGB555: pass Testing WPF packing in XRGB555: pass Testing WPF packing in RGB565: pass Testing WPF packing in BGR24: pass Testing WPF packing in RGB24: pass Testing WPF packing in ABGR32: pass Testing WPF packing in ARGB32: pass Testing WPF packing in XBGR32: pass Testing WPF packing in XRGB32: pass - vsp-unit-test-0002.sh Testing WPF packing in NV12M: pass Testing WPF packing in NV16M: pass Testing WPF packing in NV21M: pass Testing WPF packing in NV61M: pass Testing WPF packing in UYVY: pass Testing WPF packing in VYUY: skip Testing WPF packing in YUV420M: pass Testing WPF packing in YUV422M: pass Testing WPF packing in YUV444M: pass Testing WPF packing in YVU420M: pass Testing WPF packing in YVU422M: pass Testing WPF packing in YVU444M: pass Testing WPF packing in YUYV: pass Testing WPF packing in YVYU: pass - vsp-unit-test-0003.sh Testing scaling from 640x640 to 640x480 in RGB24: pass Testing scaling from 1024x768 to 640x480 in RGB24: pass Testing scaling from 640x480 to 1024x768 in RGB24: pass Testing scaling from 640x640 to 640x480 in YUV444M: pass Testing scaling from 1024x768 to 640x480 in YUV444M: pass Testing scaling from 640x480 to 1024x768 in YUV444M: pass - vsp-unit-test-0004.sh Testing histogram in RGB24: pass Testing histogram in YUV444M: pass - vsp-unit-test-0005.sh Testing RPF.0: pass Testing RPF.1: pass Testing RPF.2: pass Testing RPF.3: pass Testing RPF.4: pass - vsp-unit-test-0006.sh Testing invalid pipeline with no RPF: pass Testing invalid pipeline with no WPF: pass - vsp-unit-test-0007.sh Testing BRU in RGB24 with 1 inputs: pass Testing BRU in RGB24 with 2 inputs: pass
