From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61A9DC433E0 for ; Tue, 2 Jun 2020 12:24:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3D28B2053B for ; Tue, 2 Jun 2020 12:24:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=xs4all.nl header.i=@xs4all.nl header.b="MZWMXgP1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726130AbgFBMYt (ORCPT ); Tue, 2 Jun 2020 08:24:49 -0400 Received: from lb3-smtp-cloud8.xs4all.net ([194.109.24.29]:51109 "EHLO lb3-smtp-cloud8.xs4all.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725940AbgFBMYt (ORCPT ); Tue, 2 Jun 2020 08:24:49 -0400 Received: from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d]) by smtp-cloud8.xs4all.net with ESMTPA id g5yFjjTQAnv5ng5yIjpM3S; Tue, 02 Jun 2020 14:24:46 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1; t=1591100687; bh=QJyBxueTazKFkTqWF58Ggg0h2EATDIQfTVWKCSwHXGQ=; h=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From: Subject; b=MZWMXgP1pYS9MYeGlQn34cZqx2Lpx9MjjE+RaChodLv+m/f+9/HkrlUgJwhFztlkg Q3aOanSfNPt5Ed9BGtvanGR8P53/JlaRgqSAhMK/nZEjSTMZ3E1KoA9hz9/UkuZjf/ ggFCzTnJ8HC4LjkhmGrsGB76lugFNWD85CLhI2jLelWLYeVZwlfIst5Fp6vbFSB5qQ sl5IxPJZDkBSDDmYnyP1E48z8U4SEndBx4XLmKWKUbN6RdOZb5IAwBKGM5B2XzwIY2 MVjwihqWpujnCZrOB2tXdDSbVRVze+MxboLl/qJF1hA496IGLqaOuX4qrF6qp1VaE6 8S3bEQo9Ai27w== Subject: Re: [PATCH v6 03/14] videobuf2: handle V4L2 buffer cache flags To: Tomasz Figa Cc: Sergey Senozhatsky , Hans Verkuil , Mauro Carvalho Chehab , Linux Media Mailing List , Linux Kernel Mailing List , Sergey Senozhatsky References: <20200514160153.3646-1-sergey.senozhatsky@gmail.com> <20200514160153.3646-4-sergey.senozhatsky@gmail.com> From: Hans Verkuil Message-ID: Date: Tue, 2 Jun 2020 14:24:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-CMAE-Envelope: MS4wfEBfkBJht8YpQj63rNYopBeCmw9rfXwtK/dJdjGpdMJzx5R/vzwrQ+ttN9O5LlW+Na3PFMj+kz+JFK4t1tHjykFrHwc7yTOpFzBUkcdDCzv72VHLofT0 137elGGTAJLAAFV02Q2S6RS1HZg7W61RUevimOA2gX0npzSegrMF8IU7GZPXXTyeT5Mw3XtRxGNJYdupAbvqywJGZv45MO4HywGYRoNQKczbN9igCFh07R4K JThMaFP+FFOCyav32RCK5ZI8VOi9UprfwZWWgckzHeC5Mg62AK8Blqr653V++r8/5qVLj4fF2tpG50CkxiMq0T+KthRBzpEmr6BngFu18/IsJzmQqVtUoZHf UHQKGkllDJxaptxu4xqaAIS8vnOAYgiWtFjZ1Xa2h7gvBrUQgm8= Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On 02/06/2020 14:22, Tomasz Figa wrote: > Hi Hans, > > On Tue, Jun 2, 2020 at 11:51 AM Hans Verkuil wrote: >> >> Hi Sergey, >> >> While doing final testing for this patch series (together with the v4l-utils patch) >> I found one remaining issue: >> >> On 14/05/2020 18:01, Sergey Senozhatsky wrote: >>> From: Sergey Senozhatsky >>> >>> Set video buffer cache management flags corresponding to V4L2 cache >>> flags. >>> >>> Both ->prepare() and ->finish() cache management hints should be >>> passed during this stage (buffer preparation), because there is >>> no other way for user-space to tell V4L2 to avoid ->finish() cache >>> flush. >>> >>> Signed-off-by: Sergey Senozhatsky >>> --- >>> .../media/common/videobuf2/videobuf2-v4l2.c | 48 +++++++++++++++++++ >>> include/media/videobuf2-core.h | 11 +++++ >>> 2 files changed, 59 insertions(+) >>> >>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> index eb5d5db96552..f13851212cc8 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> @@ -337,6 +337,53 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b >>> return 0; >>> } >>> >>> +static void set_buffer_cache_hints(struct vb2_queue *q, >>> + struct vb2_buffer *vb, >>> + struct v4l2_buffer *b) >>> +{ >>> + /* >>> + * DMA exporter should take care of cache syncs, so we can avoid >>> + * explicit ->prepare()/->finish() syncs. For other ->memory types >>> + * we always need ->prepare() or/and ->finish() cache sync. >>> + */ >>> + if (q->memory == VB2_MEMORY_DMABUF) { >>> + vb->need_cache_sync_on_finish = 0; >>> + vb->need_cache_sync_on_prepare = 0; >>> + return; >>> + } >>> + >>> + /* >>> + * Cache sync/invalidation flags are set by default in order to >>> + * preserve existing behaviour for old apps/drivers. >>> + */ >>> + vb->need_cache_sync_on_prepare = 1; >>> + vb->need_cache_sync_on_finish = 1; >>> + >>> + if (!vb2_queue_allows_cache_hints(q)) { >>> + /* >>> + * Clear buffer cache flags if queue does not support user >>> + * space hints. That's to indicate to userspace that these >>> + * flags won't work. >>> + */ >>> + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_INVALIDATE; >>> + b->flags &= ~V4L2_BUF_FLAG_NO_CACHE_CLEAN; >>> + return; >>> + } >> >> These two flags need to be cleared for VB2_MEMORY_DMABUF as well in the test above. >> This bug is causing v4l2-compliance failures (use the test-media script in contrib/test >> in v4l-utils: 'sudo test-media vim2m'). > > Would you be able to paste the failures, so that we know that we > reproduce the same problems? Thanks! For vim2m (but looks the same for vivid/vimc/vicodec): Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture: Captured 8 buffers test MMAP (no poll): OK Video Capture: Captured 8 buffers test MMAP (select): OK Video Capture: Captured 8 buffers test MMAP (epoll): OK Video Capture: Captured 8 buffers test USERPTR (no poll): OK Video Capture: Captured 8 buffers test USERPTR (select): OK fail: v4l2-test-buffers.cpp(1874): flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE fail: v4l2-test-buffers.cpp(1937): setupDmaBuf(expbuf_node, node, q, exp_q) test DMABUF (no poll): FAIL fail: v4l2-test-buffers.cpp(1874): flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE fail: v4l2-test-buffers.cpp(1937): setupDmaBuf(expbuf_node, node, q, exp_q) test DMABUF (select): FAIL Regards, Hans > > Best regards, > Tomasz > >> >> It's enough to post a v6.1 for this patch, everything else is fine. >> >> Regards, >> >> Hans >> >>> + >>> + /* >>> + * ->finish() cache sync can be avoided when queue direction is >>> + * TO_DEVICE. >>> + */ >>> + if (q->dma_dir == DMA_TO_DEVICE) >>> + vb->need_cache_sync_on_finish = 0; >>> + >>> + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) >>> + vb->need_cache_sync_on_finish = 0; >>> + >>> + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) >>> + vb->need_cache_sync_on_prepare = 0; >>> +} >>> + >>> static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, >>> struct v4l2_buffer *b, bool is_prepare, >>> struct media_request **p_req) >>> @@ -381,6 +428,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md >>> } >>> >>> if (!vb->prepared) { >>> + set_buffer_cache_hints(q, vb, b); >>> /* Copy relevant information provided by the userspace */ >>> memset(vbuf->planes, 0, >>> sizeof(vbuf->planes[0]) * vb->num_planes); >>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h >>> index 7f39d9fffc8c..ccc5c498d3e3 100644 >>> --- a/include/media/videobuf2-core.h >>> +++ b/include/media/videobuf2-core.h >>> @@ -635,6 +635,17 @@ struct vb2_queue { >>> #endif >>> }; >>> >>> +/** >>> + * vb2_queue_allows_cache_hints() - Return true if the queue allows cache >>> + * and memory consistency hints. >>> + * >>> + * @q: pointer to &struct vb2_queue with videobuf2 queue >>> + */ >>> +static inline bool vb2_queue_allows_cache_hints(struct vb2_queue *q) >>> +{ >>> + return q->allow_cache_hints && q->memory == VB2_MEMORY_MMAP; >>> +} >>> + >>> /** >>> * vb2_plane_vaddr() - Return a kernel virtual address of a given plane. >>> * @vb: pointer to &struct vb2_buffer to which the plane in >>> >>