linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tomasz Figa <tfiga@chromium.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags
Date: Fri, 24 Apr 2020 18:29:07 +0900	[thread overview]
Message-ID: <20200424092920.4801-1-sergey.senozhatsky@gmail.com> (raw)

Hello,

	v5 with fixes and improvements.

I have a simple v4l-compliance patch now (will send it separately) which
tests cache and consistency hints. I ran compliance against the vivid
test driver, which was additionally extended with cache_hints module param:
Trimmed v4l-compliance output:

- vivid with disabled cache hints support

[..]
Buffer ioctls (Input 3):
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
[..]

- vivid with enabled cache hints (cache_hints=1,...)

[..]
Buffer ioctls (Input 3):
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS: OK
	test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
	test VIDIOC_EXPBUF: OK
[..]


v4l-compliance revealed that we cannot reliably report
V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS, it's a bit special.
Let's look at fill_buf_caps()

	if (q->allow_cache_hints && q->io_modes & VB2_MMAP)
		*caps |= V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS;

There are several things here. First, if the queue is not setup yet
(we didn't call driver's ->queue_setup()) then ->allow_cache_hints
is expected to be 0. It's only in the ->queue_setup() that the driver
sets ->allow_cache_hints for those queues that can benefit from cache
management. What this means, is that if one does

	q.init(node->g_type(), memory);
	q.reqbufs(node);
	q.g_capabilities();

the cache hints cap will not be reported. We need to setup the queue

	q.init(node->g_type(), memory);
	q.reqbufs(node, 1);
	q.g_capabilities();

Second. Even if the queue is setup, we still can report wrong cache
hint values. Let's look at the following code

	fill_buf_caps(q, &p->capabilities);
	if (!vb2_queue_allows_cache_hints(q))
		p->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT;
	ret = vb2_core_reqbufs(...);
	return ret;

The thing here is that vb2_core_reqbufs() and vb2_core_create_bufs()
can re-initialize the queue and invoke ->queue_setup(), possibly
changing its memory model, etc. so cache hints cap which we set or
clear before vb2_core_reqbufs() and vb2_core_create_bufs() can become
invalid after we call those functions. It's the same with
``req->flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT``, we cannot clear
it before reqbufs()/create_bufs(). Therefore I added two simple
functions which fixup cache hint cap and non_consistent flag after
reqbufs()/create_bufs(). So the code looks like this now:

	fill_buf_caps(q, &p->capabilities);
	ret = vb2_core_reqbufs(...);
	fixup_consistency_attr(q, &p->flags);
	fixup_cache_hints_cap(q, &p->capabilities);
	return ret;


The rest is the pretty much the same.

Previous series:
v4 link: https://lore.kernel.org/lkml/20200302041213.27662-1-senozhatsky@chromium.org/
v3 link: https://lore.kernel.org/lkml/20200226111529.180197-1-senozhatsky@chromium.org
v2 link: https://lore.kernel.org/lkml/20200204025641.218376-1-senozhatsky@chromium.org/
v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/

Series Intro
========================================================================

	This is a reworked version of the vb2 cache hints
(V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN)
support patch series which previsouly was developed by Sakari and
Laurent [0].

The patch set attempts to preserve the existing behvaiour - cache
sync is performed in ->prepare() and ->finish() (unless the buffer
is DMA exported). User space can request “default behavior” override
with cache management hints, which are handled on a per-buffer basis
and should be supplied with v4l2_buffer ->flags during buffer
preparation. There are two possible hints:

- V4L2_BUF_FLAG_NO_CACHE_INVALIDATE
	No cache sync on ->finish()

- V4L2_BUF_FLAG_NO_CACHE_CLEAN
	No cache sync on ->prepare()

In order to keep things on the safe side, we also require driver
to explicitly state which of its queues (if any) support user space
cache management hints (such queues should have ->allow_cache_hints
bit set).

The patch set also (to some extent) simplifies allocators' ->prepare()
and ->finish() callbacks. Namely, we move cache management decision
making to the upper - core - layer. For example, if, previously, we
would have something like this

	vb2_buffer_done()
	  vb2_dc_finish()
	    if (buf->db_attach)
	      return;

where each allocators' ->finish() callback would either bail
out (DMA exported buffer, for instance) or sync, now that "bail
out or sync" decision is made before we call into the allocator.

Along with cache management hints, user space is also able to
adjust queue's memory consistency attributes. Memory consistency
attribute (dma_attrs) is per-queue, yet it plays its role on the
allocator level, when we allocate buffers’ private memory (planes).
For the time being, only one consistency attribute is supported:
DMA_ATTR_NON_CONSISTENT.

[0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html

Sergey Senozhatsky (13):
  videobuf2: use explicit unsigned int in vb2_queue
  videobuf2: add cache management members
  videobuf2: handle V4L2 buffer cache flags
  videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  videobuf2: add queue memory consistency parameter
  videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag
  videobuf2: factor out planes prepare/finish functions
  videobuf2: do not sync caches when we are allowed not to
  videobuf2: check ->synced flag in prepare() and finish()
  videobuf2: add begin/end cpu_access callbacks to dma-contig
  videobuf2: add begin/end cpu_access callbacks to dma-sg
  videobuf2: don't test db_attach in dma-contig prepare and finish
  media: vivid: add cache_hints module param

 Documentation/admin-guide/media/vivid.rst     |   9 ++
 .../userspace-api/media/v4l/buffer.rst        |  41 +++++-
 .../media/v4l/vidioc-create-bufs.rst          |   7 +-
 .../media/v4l/vidioc-reqbufs.rst              |  20 ++-
 .../media/common/videobuf2/videobuf2-core.c   | 121 +++++++++++++-----
 .../common/videobuf2/videobuf2-dma-contig.c   |  41 +++++-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  38 ++++--
 .../media/common/videobuf2/videobuf2-v4l2.c   |  93 +++++++++++++-
 drivers/media/dvb-core/dvb_vb2.c              |   2 +-
 drivers/media/test-drivers/vivid/vivid-core.c |   9 ++
 drivers/media/test-drivers/vivid/vivid-core.h |   1 +
 .../media/test-drivers/vivid/vivid-meta-cap.c |   5 +
 .../media/test-drivers/vivid/vivid-meta-out.c |   5 +
 .../media/test-drivers/vivid/vivid-sdr-cap.c  |   7 +
 .../test-drivers/vivid/vivid-touch-cap.c      |   5 +
 .../media/test-drivers/vivid/vivid-vbi-cap.c  |   5 +
 .../media/test-drivers/vivid/vivid-vbi-out.c  |   5 +
 .../media/test-drivers/vivid/vivid-vid-cap.c  |   5 +
 .../media/test-drivers/vivid/vivid-vid-out.c  |   5 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   9 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          |   5 +-
 include/media/videobuf2-core.h                |  47 +++++--
 include/uapi/linux/videodev2.h                |  13 +-
 23 files changed, 426 insertions(+), 72 deletions(-)

-- 
2.26.2


             reply	other threads:[~2020-04-24  9:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24  9:29 Sergey Senozhatsky [this message]
2020-04-24  9:29 ` [PATCHv5 01/13] videobuf2: use explicit unsigned int in vb2_queue Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 02/13] videobuf2: add cache management members Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 03/13] videobuf2: handle V4L2 buffer cache flags Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 04/13] videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 05/13] videobuf2: add queue memory consistency parameter Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 06/13] videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 07/13] videobuf2: factor out planes prepare/finish functions Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 08/13] videobuf2: do not sync caches when we are allowed not to Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 09/13] videobuf2: check ->synced flag in prepare() and finish() Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 10/13] videobuf2: add begin/end cpu_access callbacks to dma-contig Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 11/13] videobuf2: add begin/end cpu_access callbacks to dma-sg Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 12/13] videobuf2: don't test db_attach in dma-contig prepare and finish Sergey Senozhatsky
2020-04-24  9:29 ` [PATCHv5 13/13] media: vivid: add cache_hints module param Sergey Senozhatsky
2020-04-28 14:47 ` [PATCHv5 00/13] Implement V4L2_BUF_FLAG_NO_CACHE_* flags Hans Verkuil
2020-05-14 15:36   ` Sergey Senozhatsky

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=20200424092920.4801-1-sergey.senozhatsky@gmail.com \
    --to=sergey.senozhatsky@gmail.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=tfiga@chromium.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).