From patchwork Sun Oct 2 00:36:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17482 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 11DEABD16B for ; Sun, 2 Oct 2022 00:36:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3FF9B61F74; Sun, 2 Oct 2022 02:36:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1664670975; bh=lljH7y75bGL/5bG7xoTP2dAf9d3ND5wIJK5UH1gk78I=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=1h6GE/3Iu66CwFKHOGWzo1lSPD9Ellk7qa6oMkO9KlnukSERwCyjGuqq3ZVyin4nC AipHQQPIWrvQ6CATEkMPxJNSvbMHgLAJ9HO9nFrXlofk/Q31wlfvQZQZYRrMLolVeh nHgYlSUeQ8+8Xk6WvXscPpL76xqVVCH2qal0A5BIb5l+kxxtGE6DuiJ0Of3HP1C/My lrwO3CnzhSPnVcwkKzU91sNNsyeHSOAKTCGE8goIgsv6QPPlf1hxPUMoCEzxVBqGb3 ddIBl+Tbud3VDbN/JGzg9sKELPMb6S7QHb3v9F0oOgSpZNtyxhgGEfjlAMrA5keHpd 8TF2KjRiD8zhQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 59A6261F74 for ; Sun, 2 Oct 2022 02:36:14 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GoKGc+w0"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 93FCA2D8 for ; Sun, 2 Oct 2022 02:36:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1664670973; bh=lljH7y75bGL/5bG7xoTP2dAf9d3ND5wIJK5UH1gk78I=; h=From:To:Subject:Date:From; b=GoKGc+w06m5wpMta+nr6fnItGMic5y1WSLrQP7qYY4etFa2OzxByS4a2/9Uw/li/F 8B/KjRM+2BLfr9VuStoXJkx8gOKSZAcl+0QdDc/G533vBGAqsR1vMoockS/xNZenx1 kJRaPfwPpDZDh786lrAnMp+eI1o+S09WqFAOXIgk= To: libcamera-devel@lists.libcamera.org Date: Sun, 2 Oct 2022 03:36:08 +0300 Message-Id: <20221002003612.13603-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 0/4] libcamera: Fix kernel deprecation warning with output buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Hello, This patch series fixes a warning printed by videobuf2. The V4L2 API specification indicates that, for an output video device, the driver will interpret a zero bytesused value as the buffer length (for v4l2_buffer) or the plane length (for v4l2_plane). The videobuf2 framework implements this behaviour, but also considers this case as deprecated and prints a warning: [ 54.375534] use of bytesused == 0 is deprecated and will be removed in the future, [ 54.388026] use the actual size instead. This is triggered by the RkISP1 and IPU3 pipeline handlers, which don't set bytesused for the parameters buffers. There are multiple ways to fix this issue, and as I couldn't decide which one is best, I've included two in this series. Patch 1/4 is the simple fix, it sets bytesused to the buffer or plane length before calling VIDIOC_QBUF, essentially replicating in userspace the kernel behaviour that videobuf2 considers deprecated. It may however not be considered right, one may argue that the user of the V4L2VideoDevice should always set the bytesused values correctly. Patches 2/4 to 4/4 are an attempt at setting bytesused correctly. As the FrameBuffer metadata is private and only accessible in const form outside of the class (with the exception of V4L2VideoDevice that is listed as a friend), patch 2/4 first makes mutable access to the metadata possible. In order to avoid exposing mutable access to applications, it moves the metadata (and the other remaining private members of the FrameBuffer class) to the FrameBuffer::Private class, and exposes a mutable accessor there. Patches 3/4 and 4/4 then use that accessor to set the bytesused values in the RkISP1 and IPU3 pipeline handlers. One nice side effect of patch 2/4 is that the V4L2VideoDevice class doesn't need to be listed as a friend of the FrameBuffer class anymore. If 1/4 is considered a good fix, I would thus be tempted to still merge 2/4 just to drop the friend statement. 3/4 and 4/4 currently hardcode the bytesused value to the size of the parameters buffer structure. This works fine with the RkISP1 and IPU3 pipeline handlers as the parameters buffers have a fixed size, but other devices in the future may use a variable-size buffer which would require the IPA module to pass the data size to the pipeline handler. I was tempted to do the same for RkISP1 and IPU3 for correctness, but decided it was probably not worth it. Note also that, if a future device requires variable-size buffers, the approach in patch 1/4 won't work anymore, the more complex fix will then be required. Laurent Pinchart (4): libcamera: v4l2_videodevice: Ensure non-zero bytesused for output buffers libcamera: framebuffer: Move remaining private data to Private class pipeline: ipu3: Set bytesused before queuing parameters buffer pipeline: rkisp1: Set bytesused before queuing parameters buffer include/libcamera/framebuffer.h | 19 ++---- include/libcamera/internal/framebuffer.h | 10 +++- .../mm/cros_frame_buffer_allocator.cpp | 8 +-- .../mm/generic_frame_buffer_allocator.cpp | 9 +-- src/libcamera/framebuffer.cpp | 58 ++++++++++++------- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++ src/libcamera/v4l2_videodevice.cpp | 30 +++++----- 8 files changed, 85 insertions(+), 57 deletions(-) base-commit: 79f0fc937d95cbf1bd39f04dfd8b83206bda5098