All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	Kieran Bingham <kieran.bingham@ideasonboard.com>
Subject: Re: [PATCH v7 0/8] vsp1: TLB optimisation and DL caching
Date: Sat, 07 Apr 2018 03:30:42 +0300	[thread overview]
Message-ID: <1706399.6jG5Itdsid@avalon> (raw)
In-Reply-To: <cover.636c1ee27fc6973cc312e0f25131a435872a0a35.1520466993.git-series.kieran.bingham+renesas@ideasonboard.com>

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-0000.sh
> Test Conditions:
>   Platform          Renesas Salvator-X 2nd version board based on r8a7795
> ES2.0+ Kernel release    4.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
> Testing BRU in RGB24 with 3 inputs: pass
> Testing BRU in RGB24 with 4 inputs: pass
> Testing BRU in RGB24 with 5 inputs: pass
> Testing BRU in YUV444M with 1 inputs: pass
> Testing BRU in YUV444M with 2 inputs: pass
> Testing BRU in YUV444M with 3 inputs: pass
> Testing BRU in YUV444M with 4 inputs: pass
> Testing BRU in YUV444M with 5 inputs: pass
> - vsp-unit-test-0008.sh
> Test requires unavailable feature set `bru rpf.0 uds wpf.0': skipped
> - vsp-unit-test-0009.sh
> Test requires unavailable feature set `rpf.0 wpf.0 wpf.1': skipped
> - vsp-unit-test-0010.sh
> Testing CLU in RGB24 with zero configuration: pass
> Testing CLU in RGB24 with identity configuration: pass
> Testing CLU in RGB24 with wave configuration: pass
> Testing CLU in YUV444M with zero configuration: pass
> Testing CLU in YUV444M with identity configuration: pass
> Testing CLU in YUV444M with wave configuration: pass
> Testing LUT in RGB24 with zero configuration: pass
> Testing LUT in RGB24 with identity configuration: pass
> Testing LUT in RGB24 with gamma configuration: pass
> Testing LUT in YUV444M with zero configuration: pass
> Testing LUT in YUV444M with identity configuration: pass
> Testing LUT in YUV444M with gamma configuration: pass
> - vsp-unit-test-0011.sh
> Testing  hflip=0 vflip=0 rotate=0: pass
> Testing  hflip=1 vflip=0 rotate=0: pass
> Testing  hflip=0 vflip=1 rotate=0: pass
> Testing  hflip=1 vflip=1 rotate=0: pass
> Testing  hflip=0 vflip=0 rotate=90: pass
> Testing  hflip=1 vflip=0 rotate=90: pass
> Testing  hflip=0 vflip=1 rotate=90: pass
> Testing  hflip=1 vflip=1 rotate=90: pass
> - vsp-unit-test-0012.sh
> Testing hflip: pass
> Testing vflip: pass
> - vsp-unit-test-0013.sh
> Testing RPF unpacking in RGB332: pass
> Testing RPF unpacking in ARGB555: pass
> Testing RPF unpacking in XRGB555: pass
> Testing RPF unpacking in RGB565: pass
> Testing RPF unpacking in BGR24: pass
> Testing RPF unpacking in RGB24: pass
> Testing RPF unpacking in ABGR32: pass
> Testing RPF unpacking in ARGB32: pass
> Testing RPF unpacking in XBGR32: pass
> Testing RPF unpacking in XRGB32: pass
> - vsp-unit-test-0014.sh
> Testing RPF unpacking in NV12M: pass
> Testing RPF unpacking in NV16M: pass
> Testing RPF unpacking in NV21M: pass
> Testing RPF unpacking in NV61M: pass
> Testing RPF unpacking in UYVY: pass
> Testing RPF unpacking in VYUY: skip
> Testing RPF unpacking in YUV420M: pass
> Testing RPF unpacking in YUV422M: pass
> Testing RPF unpacking in YUV444M: pass
> Testing RPF unpacking in YVU420M: pass
> Testing RPF unpacking in YVU422M: pass
> Testing RPF unpacking in YVU444M: pass
> Testing RPF unpacking in YUYV: pass
> Testing RPF unpacking in YVYU: pass
> - vsp-unit-test-0015.sh
> Testing SRU scaling from 1024x768 to 1024x768 in RGB24: pass
> Testing SRU scaling from 1024x768 to 2048x1536 in RGB24: pass
> Testing SRU scaling from 1024x768 to 1024x768 in YUV444M: pass
> Testing SRU scaling from 1024x768 to 2048x1536 in YUV444M: pass
> - vsp-unit-test-0016.sh
> Testing  hflip=0 vflip=0 rotate=0 640x480 -> 640x480: pass
> Testing  hflip=0 vflip=0 rotate=0 640x480 -> 1024x768: pass
> Testing  hflip=0 vflip=0 rotate=0 1024x768 -> 640x480: pass
> Testing  hflip=1 vflip=0 rotate=0 640x480 -> 640x480: pass
> Testing  hflip=1 vflip=0 rotate=0 640x480 -> 1024x768: pass
> Testing  hflip=1 vflip=0 rotate=0 1024x768 -> 640x480: pass
> Testing  hflip=0 vflip=1 rotate=0 640x480 -> 640x480: pass
> Testing  hflip=0 vflip=1 rotate=0 640x480 -> 1024x768: pass
> Testing  hflip=0 vflip=1 rotate=0 1024x768 -> 640x480: pass
> Testing  hflip=1 vflip=1 rotate=0 640x480 -> 640x480: pass
> Testing  hflip=1 vflip=1 rotate=0 640x480 -> 1024x768: pass
> Testing  hflip=1 vflip=1 rotate=0 1024x768 -> 640x480: pass
> Testing  hflip=0 vflip=0 rotate=90 640x480 -> 640x480: pass
> Testing  hflip=0 vflip=0 rotate=90 640x480 -> 1024x768: pass
> Testing  hflip=0 vflip=0 rotate=90 1024x768 -> 640x480: pass
> Testing  hflip=1 vflip=0 rotate=90 640x480 -> 640x480: pass
> Testing  hflip=1 vflip=0 rotate=90 640x480 -> 1024x768: pass
> Testing  hflip=1 vflip=0 rotate=90 1024x768 -> 640x480: pass
> Testing  hflip=0 vflip=1 rotate=90 640x480 -> 640x480: pass
> Testing  hflip=0 vflip=1 rotate=90 640x480 -> 1024x768: pass
> Testing  hflip=0 vflip=1 rotate=90 1024x768 -> 640x480: pass
> Testing  hflip=1 vflip=1 rotate=90 640x480 -> 640x480: pass
> Testing  hflip=1 vflip=1 rotate=90 640x480 -> 1024x768: pass
> Testing  hflip=1 vflip=1 rotate=90 1024x768 -> 640x480: pass
> - vsp-unit-test-0017.sh
> - vsp-unit-test-0018.sh
> Testing RPF crop from (0,0)/512x384: pass
> Testing RPF crop from (32,32)/512x384: pass
> Testing RPF crop from (32,64)/512x384: pass
> Testing RPF crop from (64,32)/512x384: pass
> - vsp-unit-test-0019.sh
> - vsp-unit-test-0020.sh
> - vsp-unit-test-0021.sh
> Testing WPF packing in RGB332 during stress testing: pass
> Testing WPF packing in ARGB555 during stress testing: pass
> Testing WPF packing in XRGB555 during stress testing: pass
> Testing WPF packing in RGB565 during stress testing: pass
> Testing WPF packing in BGR24 during stress testing: pass
> Testing WPF packing in RGB24 during stress testing: pass
> Testing WPF packing in ABGR32 during stress testing: pass
> Testing WPF packing in ARGB32 during stress testing: pass
> Testing WPF packing in XBGR32 during stress testing: pass
> Testing WPF packing in XRGB32 during stress testing: pass
> ./vsp-unit-test-0021.sh: line 34:  4489 Killed                  stress --cpu
> 8 --io 4 --vm 2 --vm-bytes 128M - vsp-unit-test-0022.sh
> Testing long duration pipelines under stress: pass
> ./vsp-unit-test-0022.sh: line 38:  6457 Killed                  stress --cpu
> 8 --io 4 --vm 2 --vm-bytes 128M - vsp-unit-test-0023.sh
> Testing histogram HGT with hue areas
> 0,255,255,255,255,255,255,255,255,255,255,255: pass Testing histogram HGT
> with hue areas 0,40,40,80,80,120,120,160,160,200,200,255: pass Testing
> histogram HGT with hue areas 220,40,40,80,80,120,120,160,160,200,200,220:
> pass Testing histogram HGT with hue areas
> 0,10,50,60,100,110,150,160,200,210,250,255: pass Testing histogram HGT with
> hue areas 10,20,50,60,100,110,150,160,200,210,230,240: pass Testing
> histogram HGT with hue areas 240,20,60,80,100,120,140,160,180,200,210,220:
> pass - vsp-unit-test-0024.sh
> Test requires unavailable feature set `rpf.0 rpf.1 brs wpf.0': skipped
> 158 tests: 142 passed, 0 failed, 3 skipped
> 
> Kieran Bingham (8):
>   media: vsp1: Reword uses of 'fragment' as 'body'
>   media: vsp1: Protect bodies against overflow
>   media: vsp1: Provide a body pool
>   media: vsp1: Convert display lists to use new body pool
>   media: vsp1: Use reference counting for bodies
>   media: vsp1: Refactor display list configure operations
>   media: vsp1: Adapt entities to configure into a body
>   media: vsp1: Move video configuration to a cached dlb
> 
>  drivers/media/platform/vsp1/vsp1_bru.c    |  32 +--
>  drivers/media/platform/vsp1/vsp1_clu.c    | 102 +++---
>  drivers/media/platform/vsp1/vsp1_clu.h    |   1 +-
>  drivers/media/platform/vsp1/vsp1_dl.c     | 393 +++++++++++++----------
>  drivers/media/platform/vsp1/vsp1_dl.h     |  19 +-
>  drivers/media/platform/vsp1/vsp1_drm.c    |  35 +--
>  drivers/media/platform/vsp1/vsp1_entity.c |  26 +-
>  drivers/media/platform/vsp1/vsp1_entity.h |  38 +-
>  drivers/media/platform/vsp1/vsp1_hgo.c    |  26 +--
>  drivers/media/platform/vsp1/vsp1_hgt.c    |  28 +--
>  drivers/media/platform/vsp1/vsp1_hsit.c   |  20 +-
>  drivers/media/platform/vsp1/vsp1_lif.c    |  25 +-
>  drivers/media/platform/vsp1/vsp1_lut.c    |  77 +++--
>  drivers/media/platform/vsp1/vsp1_lut.h    |   1 +-
>  drivers/media/platform/vsp1/vsp1_pipe.c   |  11 +-
>  drivers/media/platform/vsp1/vsp1_pipe.h   |   7 +-
>  drivers/media/platform/vsp1/vsp1_rpf.c    | 183 +++++------
>  drivers/media/platform/vsp1/vsp1_sru.c    |  24 +-
>  drivers/media/platform/vsp1/vsp1_uds.c    |  75 ++--
>  drivers/media/platform/vsp1/vsp1_uds.h    |   2 +-
>  drivers/media/platform/vsp1/vsp1_video.c  |  82 ++---
>  drivers/media/platform/vsp1/vsp1_video.h  |   2 +-
>  drivers/media/platform/vsp1/vsp1_wpf.c    | 327 +++++++++----------
>  23 files changed, 845 insertions(+), 691 deletions(-)
> 
> base-commit: 8514509ba5933f4e4ade0d5d81be117f18c1ebd2

-- 
Regards,

Laurent Pinchart

      parent reply	other threads:[~2018-04-07  0:30 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-08  0:05 [PATCH v7 0/8] vsp1: TLB optimisation and DL caching Kieran Bingham
2018-03-08  0:05 ` [PATCH v7 1/8] media: vsp1: Reword uses of 'fragment' as 'body' Kieran Bingham
2018-04-06 21:38   ` Laurent Pinchart
2018-03-08  0:05 ` [PATCH v7 2/8] media: vsp1: Protect bodies against overflow Kieran Bingham
2018-03-08  0:05 ` [PATCH v7 3/8] media: vsp1: Provide a body pool Kieran Bingham
2018-04-06 22:33   ` Laurent Pinchart
2018-04-30 14:12     ` Kieran Bingham
2018-03-08  0:05 ` [PATCH v7 4/8] media: vsp1: Convert display lists to use new " Kieran Bingham
2018-04-06 22:55   ` Laurent Pinchart
2018-04-30 14:39     ` Kieran Bingham
2018-03-08  0:05 ` [PATCH v7 5/8] media: vsp1: Use reference counting for bodies Kieran Bingham
2018-04-06 23:06   ` Laurent Pinchart
2018-03-08  0:05 ` [PATCH v7 6/8] media: vsp1: Refactor display list configure operations Kieran Bingham
2018-04-06 23:38   ` Laurent Pinchart
2018-04-30 16:22     ` Kieran Bingham
2018-03-08  0:05 ` [PATCH v7 7/8] media: vsp1: Adapt entities to configure into a body Kieran Bingham
2018-04-06 23:55   ` Laurent Pinchart
2018-03-08  0:05 ` [PATCH v7 8/8] media: vsp1: Move video configuration to a cached dlb Kieran Bingham
2018-04-07  0:23   ` Laurent Pinchart
2018-04-30 17:48     ` Kieran Bingham
2018-05-01  8:28       ` Kieran Bingham
2018-05-01  9:07         ` Kieran Bingham
2018-05-17 14:35       ` Laurent Pinchart
2018-05-17 17:06         ` Kieran Bingham
2018-05-17 20:11           ` Laurent Pinchart
2018-04-07  0:30 ` Laurent Pinchart [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1706399.6jG5Itdsid@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.