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=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, 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 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 D4A59C433E3 for ; Wed, 19 Aug 2020 11:17:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B566B207BB for ; Wed, 19 Aug 2020 11:17:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727120AbgHSLRh (ORCPT ); Wed, 19 Aug 2020 07:17:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727986AbgHSLRR (ORCPT ); Wed, 19 Aug 2020 07:17:17 -0400 Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7C0AAC061343 for ; Wed, 19 Aug 2020 04:17:09 -0700 (PDT) Received: by mail-ed1-x542.google.com with SMTP id cq28so17771782edb.10 for ; Wed, 19 Aug 2020 04:17:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=TY3dWKpHPgBS4i2hBxB3qyVclx38DilGGNvXEO1KY6DdtDeXEoGZm+CpQTvTtIHkXd Mcr3Pk9nt9wamUctXqaJ9Vtgv5USRdqm6e3DuHcWYt6KqN8765DkPmJpC9RhYBD7OIFb P4F+orrJbsNWzNa41CM69WvJLumBNeOU4MacvsCNdQMRr5Vn5NVujdNifhpkpMb/r5C3 g/74C1i4iUUrLlf3VHdV3aGQXIpp8xUG0FWHNvOgmSJa58fFgIkI1p/KphiIAFUCV9/f +Xkc3wMVRi7vfnTJxaNstVmNexfYhUVPXoxolqIt9Pntks5fn5bKbrwlsmuzqWADiaY0 eAEA== X-Gm-Message-State: AOAM530UR+1Zm0ERR+gbl1lZ0L4rJ2dNX/wjdNxT+YxMe48HHdKz2wYU hIfUKxS2UhP7e+V+HDKQVsgrdb75KxOzxA== X-Google-Smtp-Source: ABdhPJyPbsppPnFt87adMRGFtQhiEJj5dls5O7weAO75ai6ClEYvn0pPLBQyeA8k2cwsfQ2hmvXK5A== X-Received: by 2002:a05:6402:305b:: with SMTP id bu27mr24549811edb.300.1597835827698; Wed, 19 Aug 2020 04:17:07 -0700 (PDT) Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com. [209.85.128.51]) by smtp.gmail.com with ESMTPSA id lc18sm18492870ejb.29.2020.08.19.04.17.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:06 -0700 (PDT) Received: by mail-wm1-f51.google.com with SMTP id t14so1777378wmi.3 for ; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig Cc: Mauro Carvalho Chehab , Thomas Bogendoerfer , "James E.J. Bottomley" , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Ben Skeggs , Pawel Osciak , Marek Szyprowski , Matt Porter , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Tom Lendacky , alsa-devel@alsa-project.org, linux-samsung-soc , linux-ia64@vger.kernel.org, linux-scsi@vger.kernel.org, linux-parisc@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, Linux Kernel Mailing List , linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Linux Media Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-parisc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 AECCAC433E1 for ; Wed, 19 Aug 2020 11:17:26 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6D84F206DA for ; Wed, 19 Aug 2020 11:17:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="uMJWM6S5"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6D84F206DA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-nvme-bounces+linux-nvme=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=270vRaxHx99uq61TtQgP0vKZ9I/27Dye3zolIUCOxC8=; b=uMJWM6S5PQHzBgQ7zL6rpS/Us p6fP73o3DY9vCw3GwHay8tjpZLk/XZRA+k+3jDxrTnATpmP66QLl1mvXOZDMi5a13PA9J84vAZ6qU C0RVrBI2ojjHT75kh5d9Jcng4DGB3M4z+G08syCC1W9w86tHyJo3efigp6sldx1RZM4J8jEIY4a92 YvVhvLb7sdSG9jStUi9qxj3Mmh8N0YwDI0lUBL2NDgCQjW6m7JNDPimZ4mbyv4fgw/Ow/ABfspDak cdH3HvnqdEsLDHNeGx5MIcxbFyMpxTyuAmW3E1r7hvPmsRGLaYaebL93DwwOxSh6blvEjln2iAZGf h62wKou5w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8M5p-00083e-5T; Wed, 19 Aug 2020 11:17:21 +0000 Received: from mail-ej1-x642.google.com ([2a00:1450:4864:20::642]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8M5e-00081h-7D for linux-nvme@lists.infradead.org; Wed, 19 Aug 2020 11:17:12 +0000 Received: by mail-ej1-x642.google.com with SMTP id bo3so25780136ejb.11 for ; Wed, 19 Aug 2020 04:17:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=uLeVtPS06vjmfQ9c+dScLrZAmph41ET9Xc+UKrsB3oLaxK86WcSI4/voEVXZ/HYY5c jgpO3WgIgH8Xab7BjZXDGn4qHi1CsryajjDP6dCspxttpTd2zQALXm0sAZbDTX2eVJ6Q BfEyFhxGYUudogSLqGtq9coRN5g/BvB0IJPiyAAz4Det/t5g9svwlr1/NkENHVCM/6Fp dkOHsLpdVslhObsKGAHxjz3/FyQkctFNZP0cyVMy+vGomUpfpnZ22wx9ff1DMYggwHJ9 4lHBtAV14vXbV4RBtCYj5t11G2lwPrLglNUl8g2r/F5fJ93MwiEoZf2cOmS72645yp7X Q1Bw== X-Gm-Message-State: AOAM531Bj8ja8bOYatqPogHVpjP9d0TnsF8HXe+qF3Ogs79PSXcNrmly MJijVnNw+WB/Jr21bfyj+Ge3S7V+rH+78g== X-Google-Smtp-Source: ABdhPJyj7UNBpRmFGCBl+aDVa81MyiO+fa7jzwEkFOMMnvJfofs6ODlr+wYxDRs969ko0R3Gjqt5CA== X-Received: by 2002:a17:906:3cc:: with SMTP id c12mr23725374eja.222.1597835828449; Wed, 19 Aug 2020 04:17:08 -0700 (PDT) Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com. [209.85.128.50]) by smtp.gmail.com with ESMTPSA id re10sm18705661ejb.68.2020.08.19.04.17.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:07 -0700 (PDT) Received: by mail-wm1-f50.google.com with SMTP id c19so1444218wmd.1 for ; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200819_071710_325092_655B1741 X-CRM114-Status: GOOD ( 40.97 ) X-BeenThere: linux-nvme@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel , " Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-nvme" Errors-To: linux-nvme-bounces+linux-nvme=archiver.kernel.org@lists.infradead.org Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme 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=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, 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 autolearn=ham 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 DA42AC433E1 for ; Wed, 19 Aug 2020 11:17:12 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 62FEE206DA for ; Wed, 19 Aug 2020 11:17:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 62FEE206DA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B7BF86B0073; Wed, 19 Aug 2020 07:17:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B044C8D000A; Wed, 19 Aug 2020 07:17:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9A5896B007B; Wed, 19 Aug 2020 07:17:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0137.hostedemail.com [216.40.44.137]) by kanga.kvack.org (Postfix) with ESMTP id 7A5336B0073 for ; Wed, 19 Aug 2020 07:17:11 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 35648180AD82F for ; Wed, 19 Aug 2020 11:17:11 +0000 (UTC) X-FDA: 77167066662.09.box70_0c0600327027 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin09.hostedemail.com (Postfix) with ESMTP id 12CB8180AD81F for ; Wed, 19 Aug 2020 11:17:11 +0000 (UTC) X-HE-Tag: box70_0c0600327027 X-Filterd-Recvd-Size: 18622 Received: from mail-ej1-f66.google.com (mail-ej1-f66.google.com [209.85.218.66]) by imf45.hostedemail.com (Postfix) with ESMTP for ; Wed, 19 Aug 2020 11:17:10 +0000 (UTC) Received: by mail-ej1-f66.google.com with SMTP id c16so25798144ejx.12 for ; Wed, 19 Aug 2020 04:17:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=bRlpCRNrkWlHVV290ErShFQxKlI2l323Fq2Sr9lxluPbxi+lC8wXEDqhLf4mgF5j6c xQwKBlpNH3pW3fu8udmHVwkqbxmrE9haFesSrn61h4lgPrEa6so7zX49IO/OwETBIjCk +49G3Pi8G2KDu/4+VCGJJRgXvCs69DDOtw/AcST1/JYJzwji8UJK2ePvl2PiQfORRITg a5lscvbkTCfjOwvViXIEQ3TsMtHoSQhQYxpsjWTVRS+DaequrjsiyyPEYRCYr0JAJERK jKVJVqv14TzC0sfmFboIQQABYAg6E7VKbBy04CFhPpmJItvUPWGbRdJmWEgqWDXtXE2T By0Q== X-Gm-Message-State: AOAM533a+4lLRMIAyl78JT8C8GEpCvtbc9KTPXhYKe5ndeKrYhNIHGLo d4Oku3FIfTgOcdW9GPbU5/khFsRubzBuxQ== X-Google-Smtp-Source: ABdhPJwJYsoZCunYGNl9emTtu1ezJufdWzyY+1c66GfAdbwhIMQPIoqhCnD23pHt8cAN2iMB5ADEZw== X-Received: by 2002:a17:906:4a0d:: with SMTP id w13mr17178898eju.247.1597835828552; Wed, 19 Aug 2020 04:17:08 -0700 (PDT) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com. [209.85.128.43]) by smtp.gmail.com with ESMTPSA id c20sm17781072edy.40.2020.08.19.04.17.07 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:08 -0700 (PDT) Received: by mail-wm1-f43.google.com with SMTP id 9so1686536wmj.5 for ; Wed, 19 Aug 2020 04:17:07 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig Cc: Mauro Carvalho Chehab , Thomas Bogendoerfer , "James E.J. Bottomley" , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Ben Skeggs , Pawel Osciak , Marek Szyprowski , Matt Porter , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Tom Lendacky , alsa-devel@alsa-project.org, linux-samsung-soc , linux-ia64@vger.kernel.org, linux-scsi@vger.kernel.org, linux-parisc@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, Linux Kernel Mailing List , linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Linux Media Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 12CB8180AD81F X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam03 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-9.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 C9751C433DF for ; Fri, 21 Aug 2020 07:49:00 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5301D208DB for ; Fri, 21 Aug 2020 07:49:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="cqLCNsYA"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5301D208DB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id C21CD1669; Fri, 21 Aug 2020 09:48:08 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz C21CD1669 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1597996138; bh=3R6CBpf4dsU9w13339ppYikQpe920JmCQwX1FYO+jYM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=cqLCNsYAN+qGm79UWdSqiGenWwzXX+bntNCutojNkagUG7ELH/coTyVYV2VJv6T47 nEp8wy/cpZCSo989fabRNCYG2e69iUgLz4GmaXHhnM24zEiB4wUbqZ+Rect1/18myt JNErOg0hx2o58Xd60sirvEr0SOGqoOYwlgM7w2PA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 893D5F802C4; Fri, 21 Aug 2020 09:36:19 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id F3A54F802A9; Wed, 19 Aug 2020 13:17:19 +0200 (CEST) Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 2467CF8028F for ; Wed, 19 Aug 2020 13:17:08 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 2467CF8028F Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" Received: by mail-ed1-x542.google.com with SMTP id ba10so17766558edb.3 for ; Wed, 19 Aug 2020 04:17:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=tTDPKGgTBvOHOr4rqT1BPLQRbwz2XtRDb+CFO3S8QACHfMSOoSnlI3AydA3iBgx84w 8ZCwQthg6wY9ORGuPsM91wxVqXhx7QYWqewH9ht9PpB2bwBwRime+mLHa9dw9BH9Fq29 ET1U1Bpo1K2drzLfpBqi0h97f94DR2b6ASc9SqpBfPIKfBFf+7Vd8FuQzCPqeZgJ/DeH Qrz7HiU6saLj3c+i5k9tagtZasSD8QpUBaywJ1Ncb4+tddydqsTuk77+l1V6DQLXyxjI KLFeAMAXY39P89PkncST4D52OIey68aNnUHAU18XlVAepgIhnqFsjaosqcosK/Zg8nGg B4lA== X-Gm-Message-State: AOAM5329p/Xu4fjZsv0xz60Xsj1kZu1ZpTThpIZkbN1gI1vy7eEJ0XZV zJOwGB/mW1g1e3c552RIsXtaYrlrqnfNmA== X-Google-Smtp-Source: ABdhPJyOqx/0hfPWyD7BtjK0rNp52nmV2r+bG8i3GU0UgJI/nj/7l1MF7GcU0YysLq71FxW90bS+qg== X-Received: by 2002:a05:6402:2033:: with SMTP id ay19mr23806098edb.361.1597835827759; Wed, 19 Aug 2020 04:17:07 -0700 (PDT) Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com. [209.85.128.48]) by smtp.gmail.com with ESMTPSA id i14sm17772250edr.15.2020.08.19.04.17.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:06 -0700 (PDT) Received: by mail-wm1-f48.google.com with SMTP id d190so1696241wmd.4 for ; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig Content-Type: text/plain; charset="UTF-8" X-Mailman-Approved-At: Fri, 21 Aug 2020 09:36:14 +0200 Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel , " X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT Date: Wed, 19 Aug 2020 13:16:51 +0200 Message-ID: References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20200819065555.1802761-6-hch-jcswGhMUV9g@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Sender: "iommu" To: Christoph Hellwig Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org, linux-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Doc Mailing List , nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-samsung-soc , Joonyoung Shim , linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS , Joerg Roedel , " , Thomas List-Id: nouveau.vger.kernel.org Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-9.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 83BFBC433DF for ; Wed, 19 Aug 2020 11:17:16 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4DAA5206DA for ; Wed, 19 Aug 2020 11:17:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4DAA5206DA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 1DFBC86F21; Wed, 19 Aug 2020 11:17:16 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8fXH0Lsb7Qed; Wed, 19 Aug 2020 11:17:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 06FC884F76; Wed, 19 Aug 2020 11:17:14 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id DD334C0889; Wed, 19 Aug 2020 11:17:14 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 04CA9C0051 for ; Wed, 19 Aug 2020 11:17:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id EABFA203F7 for ; Wed, 19 Aug 2020 11:17:12 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ucChGwV9gPEx for ; Wed, 19 Aug 2020 11:17:10 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-ed1-f65.google.com (mail-ed1-f65.google.com [209.85.208.65]) by silver.osuosl.org (Postfix) with ESMTPS id 2A84C20012 for ; Wed, 19 Aug 2020 11:17:10 +0000 (UTC) Received: by mail-ed1-f65.google.com with SMTP id ba10so17766572edb.3 for ; Wed, 19 Aug 2020 04:17:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=Mz0RXG2scBtbyIIQuM1A2i2vTZcDKiGvIdwgwrXPfns5nLMixf9GSuPSc+3CKKrHve 7aB2s2RqY6U4jWRfDDjL0fy5nzs6qyU5c1lCZ7UoHqjOa1Re2N+T6cvJ9qZ3hXkNYGPd iQl1JBmT+o11z34zwYfb1mVDBbZVUEYNi5wsGl8A3dPoB8jHT8lJps1vkm82stRAjx/g E3N+ZicFoKnkt8SxIWFN8WNf9vIvIznnNrBWnBpbNkCIGM9AAwXkNZ8gIjsLnT2zgVPt TEo/DXasxWLvNo6J1O8nIrOZtrpaJ9xZCugFlfelPtLlAxvtNu/+iAMZu0osqfrNCPm7 kCgQ== X-Gm-Message-State: AOAM532DLoFNdXB1IePImR4BIoerY/BTpZXntHXmmsiVuNWMYpZtDnCM ldn0aRnb1RNyoVobbI6hWHDX7N6nqs9c0Q== X-Google-Smtp-Source: ABdhPJzjlPvatzsfbIUyk4PH8gocAt9gc2Rt7bTTkazLK9pYzZPiS/wy48BGdY1w/UD0u7JCccr7FA== X-Received: by 2002:aa7:da0e:: with SMTP id r14mr24520162eds.236.1597835827924; Wed, 19 Aug 2020 04:17:07 -0700 (PDT) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com. [209.85.128.41]) by smtp.gmail.com with ESMTPSA id t6sm18509816ejc.40.2020.08.19.04.17.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:06 -0700 (PDT) Received: by mail-wm1-f41.google.com with SMTP id 9so1686487wmj.5 for ; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel , " X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: iommu-bounces@lists.linux-foundation.org Sender: "iommu" Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 1F48EC433E3 for ; Wed, 19 Aug 2020 11:18:41 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D8A19206DA for ; Wed, 19 Aug 2020 11:18:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="soiBWyPM"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KGE8DsZN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D8A19206DA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kV/wCXrZDeCiUK9l4Fh5oSEfZ6ma7ttcfeXXuPdXD38=; b=soiBWyPMqG9v4CykqA/Dj4nui tL2IsygMXMqzUW3YaU7XIOPta3y7wjxhY1y/95gY8rWr40D0hq/vFlkxViTdbryU5EMv53ySNMyTQ c8ulJ2rRpY+TIQ1f6kR7JB5MySVWHc1RABKEG+TleN10RJEy/FpMLvBfYSi0u/5zcapsnCZ5ZudxA pN+Gzq77kZeAROGgUMwmY0MKiTjRuNK9+knF/QFpVunfgtU8weHzyTheq+5CZMgHhPdkIXOWymM6L tf9VNccNf/ObiImku30goByv2uiO4BF3OxTLF1Dv0W+7YvbFX0DUmssQpdlt4GOujjkrrrCNhsjvx /hVsvu/FA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8M5l-00083P-HA; Wed, 19 Aug 2020 11:17:17 +0000 Received: from mail-ej1-x641.google.com ([2a00:1450:4864:20::641]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1k8M5d-00081P-VS for linux-arm-kernel@lists.infradead.org; Wed, 19 Aug 2020 11:17:11 +0000 Received: by mail-ej1-x641.google.com with SMTP id t10so25812763ejs.8 for ; Wed, 19 Aug 2020 04:17:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=KGE8DsZNPdo6bqqkNs6edk3dK4MOXuK9NWd33hMcfdeP5b5y7CI7bU8uUvoUNKNbCL BCUpAU5pNdEGRxiF1Tt31HfrXC3dO7lThncHiUa8bV75+8FdTXmnmpT0noHZVFfmMalt 6eJ1XcN/dw5x7iEHscOqP6Asl2A4A3kLb2fEg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L/GQUtqJB7h4Ol5M3PCzHtYsEa5HLvnGnATVGMPF8nw=; b=YwnCuYWLcfttpNx+E47HCGTDuh5nv+iKEZv0bsUB63RpGcipUvAp0fv9jOiBACRzsx TZs4fbGgOayMft6L2q5jRCdnaI+U12skFnMTm9BhkAlDqK5XsyMBw4cXEsarknpZUjuZ nHnkQTbjR+cw4t8BiH/hVYH7ZLkVYjQrD+A53QXKxUTer54131AXwH0K5SYh0VY3H7Kb GJqs6Ix2MvVxtXLCYxbg6AftOq3EbT4fcdYFZa3yPgebZ5AItEAaA4AJsTck4B8lSvLH PowdHvm/NxDuL85uiOc57D2dnTC6wSj089FPvrsrDixyxoQjwHWDu6juGXapCLAetaPQ drXA== X-Gm-Message-State: AOAM531gFLzVY5gs8KCvluhtCz9aGzJ0r+rekLyl5Mpt/2Cz1BgtAnxV 5NNaA3z/HJZOjglKl+k0Vzt7CHRjtZSSeg== X-Google-Smtp-Source: ABdhPJyR+vkXDikKgBpdUPJArhYoeng8I84AYAOLHPiVqavY6AOcR2xNzm5ll+rHyRMgHEYXNZOLuQ== X-Received: by 2002:a17:906:3b4e:: with SMTP id h14mr25743403ejf.546.1597835826984; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com. [209.85.128.43]) by smtp.gmail.com with ESMTPSA id du2sm19384583ejc.2.2020.08.19.04.17.06 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Aug 2020 04:17:06 -0700 (PDT) Received: by mail-wm1-f43.google.com with SMTP id t14so1777389wmi.3 for ; Wed, 19 Aug 2020 04:17:06 -0700 (PDT) X-Received: by 2002:a1c:9c91:: with SMTP id f139mr4664795wme.134.1597835823094; Wed, 19 Aug 2020 04:17:03 -0700 (PDT) MIME-Version: 1.0 References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> From: Tomasz Figa Date: Wed, 19 Aug 2020 13:16:51 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT To: Christoph Hellwig X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200819_071710_051570_75A01F0B X-CRM114-Status: GOOD ( 40.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alsa-devel@alsa-project.org, linux-ia64@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, linux-nvme@lists.infradead.org, Linux Kernel Mailing List , "James E.J. Bottomley" , linux-mm@kvack.org, Marek Szyprowski , linux-samsung-soc , Joonyoung Shim , linux-scsi@vger.kernel.org, Kyungmin Park , Ben Skeggs , Matt Porter , Linux Media Mailing List , Tom Lendacky , Pawel Osciak , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Thomas Bogendoerfer , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Seung-Woo Kim , linux-mips@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel , " Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count == 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers == VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count == 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count == 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Date: Wed, 19 Aug 2020 11:16:51 +0000 Subject: Re: [PATCH 05/28] media/v4l2: remove V4L2-FLAG-MEMORY-NON-CONSISTENT Message-Id: List-Id: References: <20200819065555.1802761-1-hch@lst.de> <20200819065555.1802761-6-hch@lst.de> In-Reply-To: <20200819065555.1802761-6-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Christoph Hellwig Cc: Mauro Carvalho Chehab , Thomas Bogendoerfer , "James E.J. Bottomley" , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Ben Skeggs , Pawel Osciak , Marek Szyprowski , Matt Porter , "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Tom Lendacky , alsa-devel@alsa-project.org, linux-samsung-soc , linux-ia64@vger.kernel.org, linux-scsi@vger.kernel.org, linux-parisc@vger.kernel.org, Linux Doc Mailing List , nouveau@lists.freedesktop.org, Linux Kernel Mailing List , linux-nvme@lists.infradead.org, linux-mips@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, "list@263.net:IOMMU DRIVERS , Joerg Roedel ," , Linux Media Mailing List Hi Christoph, On Wed, Aug 19, 2020 at 8:56 AM Christoph Hellwig wrote: > > The V4L2-FLAG-MEMORY-NON-CONSISTENT flag is entirely unused, Could you explain what makes you think it's unused? It's a feature of the UAPI generally supported by the videobuf2 framework and relied on by Chromium OS to get any kind of reasonable performance when accessing V4L2 buffers in the userspace. > and causes > weird gymanstics with the DMA_ATTR_NON_CONSISTENT flag, which is > unimplemented except on PARISC and some MIPS configs, and about to be > removed. It is implemented by the generic DMA mapping layer [1], which is used by a number of architectures including ARM64 and supposed to be used by new architectures going forward. [1] https://elixir.bootlin.com/linux/v5.9-rc1/source/kernel/dma/mapping.c#L341 When removing features from generic kernel code, I'd suggest first providing viable alternatives for its users, rather than killing the users altogether. Given the above, I'm afraid I have to NAK this. Best regards, Tomasz > > Signed-off-by: Christoph Hellwig > --- > .../userspace-api/media/v4l/buffer.rst | 17 --------- > .../media/v4l/vidioc-reqbufs.rst | 1 - > .../media/common/videobuf2/videobuf2-core.c | 36 +------------------ > .../common/videobuf2/videobuf2-dma-contig.c | 19 ---------- > .../media/common/videobuf2/videobuf2-dma-sg.c | 3 +- > .../media/common/videobuf2/videobuf2-v4l2.c | 12 ------- > include/media/videobuf2-core.h | 3 +- > include/uapi/linux/videodev2.h | 2 -- > 8 files changed, 3 insertions(+), 90 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst > index 57e752aaf414a7..2044ed13cd9d7d 100644 > --- a/Documentation/userspace-api/media/v4l/buffer.rst > +++ b/Documentation/userspace-api/media/v4l/buffer.rst > @@ -701,23 +701,6 @@ Memory Consistency Flags > :stub-columns: 0 > :widths: 3 1 4 > > - * .. _`V4L2-FLAG-MEMORY-NON-CONSISTENT`: > - > - - ``V4L2_FLAG_MEMORY_NON_CONSISTENT`` > - - 0x00000001 > - - A buffer is allocated either in consistent (it will be automatically > - coherent between the CPU and the bus) or non-consistent memory. The > - latter can provide performance gains, for instance the CPU cache > - sync/flush operations can be avoided if the buffer is accessed by the > - corresponding device only and the CPU does not read/write to/from that > - buffer. However, this requires extra care from the driver -- it must > - guarantee memory consistency by issuing a cache flush/sync when > - consistency is needed. If this flag is set V4L2 will attempt to > - allocate the buffer in non-consistent memory. The flag takes effect > - only if the buffer is used for :ref:`memory mapping ` I/O and the > - queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS > - ` capability. > - > .. c:type:: v4l2_memory > > enum v4l2_memory > diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > index 75d894d9c36c42..3180c111d368ee 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst > @@ -169,7 +169,6 @@ aborting or finishing any DMA in progress, an implicit > - This capability is set by the driver to indicate that the queue supports > cache and memory management hints. However, it's only valid when the > queue is used for :ref:`memory mapping ` streaming I/O. See > - :ref:`V4L2_FLAG_MEMORY_NON_CONSISTENT `, > :ref:`V4L2_BUF_FLAG_NO_CACHE_INVALIDATE ` and > :ref:`V4L2_BUF_FLAG_NO_CACHE_CLEAN `. > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index f544d3393e9d6b..66a41cef33c1b1 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -721,39 +721,14 @@ int vb2_verify_memory_type(struct vb2_queue *q, > } > EXPORT_SYMBOL(vb2_verify_memory_type); > > -static void set_queue_consistency(struct vb2_queue *q, bool consistent_mem) > -{ > - q->dma_attrs &= ~DMA_ATTR_NON_CONSISTENT; > - > - if (!vb2_queue_allows_cache_hints(q)) > - return; > - if (!consistent_mem) > - q->dma_attrs |= DMA_ATTR_NON_CONSISTENT; > -} > - > -static bool verify_consistency_attr(struct vb2_queue *q, bool consistent_mem) > -{ > - bool queue_is_consistent = !(q->dma_attrs & DMA_ATTR_NON_CONSISTENT); > - > - if (consistent_mem != queue_is_consistent) { > - dprintk(q, 1, "memory consistency model mismatch\n"); > - return false; > - } > - return true; > -} > - > int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > unsigned int flags, unsigned int *count) > { > unsigned int num_buffers, allocated_buffers, num_planes = 0; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > unsigned int i; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->streaming) { > dprintk(q, 1, "streaming active\n"); > return -EBUSY; > @@ -765,8 +740,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > } > > if (*count = 0 || q->num_buffers != 0 || > - (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || > - !verify_consistency_attr(q, consistent_mem)) { > + (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > /* > * We already have buffers allocated, so first check if they > * are not in use and can be freed. > @@ -803,7 +777,6 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > > /* > * Ask the driver how many buffers and planes per buffer it requires. > @@ -894,12 +867,8 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > { > unsigned int num_planes = 0, num_buffers, allocated_buffers; > unsigned plane_sizes[VB2_MAX_PLANES] = { }; > - bool consistent_mem = true; > int ret; > > - if (flags & V4L2_FLAG_MEMORY_NON_CONSISTENT) > - consistent_mem = false; > - > if (q->num_buffers = VB2_MAX_FRAME) { > dprintk(q, 1, "maximum number of buffers already allocated\n"); > return -ENOBUFS; > @@ -912,15 +881,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, > } > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > q->memory = memory; > - set_queue_consistency(q, consistent_mem); > q->waiting_for_buffers = !q->is_output; > } else { > if (q->memory != memory) { > dprintk(q, 1, "memory model mismatch\n"); > return -EINVAL; > } > - if (!verify_consistency_attr(q, consistent_mem)) > - return -EINVAL; > } > > num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > index ec3446cc45b8da..7b1b86ec942d7d 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c > @@ -42,11 +42,6 @@ struct vb2_dc_buf { > struct dma_buf_attachment *db_attach; > }; > > -static inline bool vb2_dc_buffer_consistent(unsigned long attr) > -{ > - return !(attr & DMA_ATTR_NON_CONSISTENT); > -} > - > /*********************************************/ > /* scatterlist table functions */ > /*********************************************/ > @@ -341,13 +336,6 @@ static int > vb2_dc_dmabuf_ops_begin_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > @@ -355,13 +343,6 @@ static int > vb2_dc_dmabuf_ops_end_cpu_access(struct dma_buf *dbuf, > enum dma_data_direction direction) > { > - struct vb2_dc_buf *buf = dbuf->priv; > - struct sg_table *sgt = buf->dma_sgt; > - > - if (vb2_dc_buffer_consistent(buf->attrs)) > - return 0; > - > - dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); > return 0; > } > > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > index 0a40e00f0d7e5c..a86fce5d8ea8bf 100644 > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c > @@ -123,8 +123,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs, > /* > * NOTE: dma-sg allocates memory using the page allocator directly, so > * there is no memory consistency guarantee, hence dma-sg ignores DMA > - * attributes passed from the upper layer. That means that > - * V4L2_FLAG_MEMORY_NON_CONSISTENT has no effect on dma-sg buffers. > + * attributes passed from the upper layer. > */ > buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *), > GFP_KERNEL | __GFP_ZERO); > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index 30caad27281e1a..de83ad48783821 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -722,20 +722,11 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > #endif > } > > -static void clear_consistency_attr(struct vb2_queue *q, > - int memory, > - unsigned int *flags) > -{ > - if (!q->allow_cache_hints || memory != V4L2_MEMORY_MMAP) > - *flags &= ~V4L2_FLAG_MEMORY_NON_CONSISTENT; > -} > - > int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) > { > int ret = vb2_verify_memory_type(q, req->memory, req->type); > > fill_buf_caps(q, &req->capabilities); > - clear_consistency_attr(q, req->memory, &req->flags); > return ret ? ret : vb2_core_reqbufs(q, req->memory, > req->flags, &req->count); > } > @@ -769,7 +760,6 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) > unsigned i; > > fill_buf_caps(q, &create->capabilities); > - clear_consistency_attr(q, create->memory, &create->flags); > create->index = q->num_buffers; > if (create->count = 0) > return ret != -EBUSY ? ret : 0; > @@ -998,7 +988,6 @@ int vb2_ioctl_reqbufs(struct file *file, void *priv, > int res = vb2_verify_memory_type(vdev->queue, p->memory, p->type); > > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > if (res) > return res; > if (vb2_queue_is_busy(vdev, file)) > @@ -1021,7 +1010,6 @@ int vb2_ioctl_create_bufs(struct file *file, void *priv, > > p->index = vdev->queue->num_buffers; > fill_buf_caps(vdev->queue, &p->capabilities); > - clear_consistency_attr(vdev->queue, p->memory, &p->flags); > /* > * If count = 0, then just check if memory and type are valid. > * Any -EBUSY result from vb2_verify_memory_type can be mapped to 0. > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 52ef92049073e3..4c7f25b07e9375 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -744,8 +744,7 @@ void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb); > * vb2_core_reqbufs() - Initiate streaming. > * @q: pointer to &struct vb2_queue with videobuf2 queue. > * @memory: memory type, as defined by &enum vb2_memory. > - * @flags: auxiliary queue/buffer management flags. Currently, the only > - * used flag is %V4L2_FLAG_MEMORY_NON_CONSISTENT. > + * @flags: auxiliary queue/buffer management flags. > * @count: requested buffer count. > * > * Videobuf2 core helper to implement VIDIOC_REQBUF() operation. It is called > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c7b70ff53bc1dd..5c00f63d9c1b58 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -191,8 +191,6 @@ enum v4l2_memory { > V4L2_MEMORY_DMABUF = 4, > }; > > -#define V4L2_FLAG_MEMORY_NON_CONSISTENT (1 << 0) > - > /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ > enum v4l2_colorspace { > /* > -- > 2.28.0 > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu