Message ID | 20221205152149.1364185-4-dave.stevenson@raspberrypi.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Sakari Ailus |
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1p2DLT-00D2Hx-Qa; Mon, 05 Dec 2022 15:25:28 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232572AbiLEPZ0 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Mon, 5 Dec 2022 10:25:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59618 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232422AbiLEPZA (ORCPT <rfc822;linux-media@vger.kernel.org>); Mon, 5 Dec 2022 10:25:00 -0500 Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7101A20BFF for <linux-media@vger.kernel.org>; Mon, 5 Dec 2022 07:22:14 -0800 (PST) Received: by mail-wr1-x432.google.com with SMTP id y16so19125284wrm.2 for <linux-media@vger.kernel.org>; Mon, 05 Dec 2022 07:22:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=jW/wraYzdeVh5VN6Plj6mY5qgvxc6yfVXPI0v0RffG8=; b=pxT3QuEFaoyxydtMf1y6iaI+cyLUMfjibZVa4ik2l2fDGq2tqQoiCt68EvrEWGEhYF lZVF+cyG9lY1L9z03QG1IK86BXAbmiGC69f8R9scCn69iK9v5fvD34dQUR7xyA2qxUfa 2yM1KBLo62rbmk4boVkcgSqLe3K/eKCdBeWuBE9crJETOvlfUu57x2qYc9P0C0jzHvWw Cx2xyiWsbTz0659TI7l8rAntnJUmElLobmFZ2H0m+mJHFNerXLTC1U7+XbKW1SaQTIDV Jw25hMCKjjOuIQvqpa+gqF6cdb+jQZbVJBPGTuB2XPR17BfGda1zVD6k6SfpL6WMqNx/ WEsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jW/wraYzdeVh5VN6Plj6mY5qgvxc6yfVXPI0v0RffG8=; b=CiUvFJWsybZ3zfe7FbW4hILZs0V5/x8NWljGSpyBHwPO01NmhmJG43w7KS4WPcubB5 JDrtouketFCG1wdVZub4gg3kC7/vW4pqgUMeKPUQShragpljgk22GAsSqSCKgidwGPOw 1H2KU3jJu47jk9mdRGjpT+mHLXsaYbfTD44Sv4vB6SKxEM1yeVZVy/yT9tjoMpbMUrUa pP770qV+205oijssHG1knx5Kidknw+yq3CyLGDORlHT8RBvLgyIqyRbXukoRouU3pKav vydDyQI5sbJWuE8U36g+a80RfFN8FqdIycp+XjwqvlDQ4tUj7wJTUx5aLK1UP4cLhOLB t5Cw== X-Gm-Message-State: ANoB5pnPMJ8xnkx8BfU8hYyg0p0IYUGLeqcq0lwx97FwPfSabFD+MLvP zpefS5g5KvUklbUeDJMdqBk8jA== X-Google-Smtp-Source: AA0mqf6U348ZFOAy8ENh041iW1hN7Fiobz6Mc+oenTnGyPpiQbIzcgFolNufEWQZRRH3iR57U/IyQw== X-Received: by 2002:adf:de08:0:b0:242:1d2c:9d78 with SMTP id b8-20020adfde08000000b002421d2c9d78mr20515260wrm.490.1670253732168; Mon, 05 Dec 2022 07:22:12 -0800 (PST) Received: from dave-Ubuntu2204.pitowers.org ([93.93.133.154]) by smtp.googlemail.com with ESMTPSA id fc13-20020a05600c524d00b003d04e4ed873sm25144905wmb.22.2022.12.05.07.22.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Dec 2022 07:22:11 -0800 (PST) From: Dave Stevenson <dave.stevenson@raspberrypi.com> To: Rui Miguel Silva <rmfrfs@gmail.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, Bingbu Cao <bingbu.cao@intel.com>, Tianshu Qiu <tian.shu.qiu@intel.com>, Jimmy Su <jimmy.su@intel.com>, linux-media@vger.kernel.org Cc: Dave Stevenson <dave.stevenson@raspberrypi.com> Subject: [PATCH v2 3/5] media: i2c: imx319: Set V4L2_CTRL_FLAG_MODIFY_LAYOUT on flips Date: Mon, 5 Dec 2022 15:21:47 +0000 Message-Id: <20221205152149.1364185-4-dave.stevenson@raspberrypi.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221205152149.1364185-1-dave.stevenson@raspberrypi.com> References: <20221205152149.1364185-1-dave.stevenson@raspberrypi.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.5 (--) X-LSpam-Report: No, score=-2.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
Ensure sensor drivers set V4L2_CTRL_FLAG_MODIFY_LAYOUT for flips
|
|
Commit Message
Dave Stevenson
Dec. 5, 2022, 3:21 p.m. UTC
The driver changes the Bayer order based on the flips, but
does not define the control correctly with the
V4L2_CTRL_FLAG_MODIFY_LAYOUT flag.
Add the V4L2_CTRL_FLAG_MODIFY_LAYOUT flag.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
drivers/media/i2c/imx319.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
Stevenson, Thanks for your patch. I am wondering how V4L2_CTRL_FLAG_MODIFY_LAYOUT flag was used in current v4l2 ctrl framework, literally it means the v4l2 ctrl will change the buffer layout. From my understanding, such as 90 degrees rotation apparently change the layout. But I am not sure this is also the case for vflip/hflip, user can notice the bayer order update from get_fmt. Sakari, what do you think?
Hi On Tue, 6 Dec 2022 at 03:42, Cao, Bingbu <bingbu.cao@intel.com> wrote: > > Stevenson, > > Thanks for your patch. > > I am wondering how V4L2_CTRL_FLAG_MODIFY_LAYOUT flag was used in current > v4l2 ctrl framework, literally it means the v4l2 ctrl will change the buffer > layout. From my understanding, such as 90 degrees rotation apparently change > the layout. But I am not sure this is also the case for vflip/hflip, user can > notice the bayer order update from get_fmt. Documentation at [1] "3.5.1. Interactions between formats, controls and buffers ... The set of information needed to interpret the content of a buffer (e.g. the pixel format, the line stride, the tiling orientation or the rotation) is collectively referred to in the rest of this section as the buffer layout." pixel_format is part of the buffer layout. V4L2_CTRL_FLAG_MODIFY_LAYOUT is telling the userspace that it must call get_fmt after changing the control in order to find out how the format has changed. Without it there is no obligation to call get_fmt, and userspace can legitimately expect the format/layout to be the same. Not all sensors change the Bayer order with flips (OnSemi sensors in particular tend not to), so you can't make assumptions over the behaviour. > Sakari, what do you think? Seeing as Sakari has accepted the patches and created a pull request to Mauro including them suggests that this is indeed the correct thing to do. There is now a unified behaviour across all sensor drivers that change Bayer order due to flips. libcamera is relying on correct behaviour in order to be able to work out the native Bayer order of the sensor, and that is why I was checking that all mainline drivers were doing the right thing. Thanks Dave [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#interactions-between-formats-controls-and-buffers > ________________________ > BRs, > VTG - Linux&Chrome IPU SW > Bingbu Cao > > > -----Original Message----- > > From: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Sent: Monday, December 5, 2022 23:22 > > To: Rui Miguel Silva <rmfrfs@gmail.com>; Sakari Ailus > > <sakari.ailus@linux.intel.com>; Cao, Bingbu <bingbu.cao@intel.com>; Qiu, > > Tian Shu <tian.shu.qiu@intel.com>; Su, Jimmy <jimmy.su@intel.com>; linux- > > media@vger.kernel.org > > Cc: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Subject: [PATCH v2 3/5] media: i2c: imx319: Set V4L2_CTRL_FLAG_MODIFY_LAYOUT > > on flips > > > > The driver changes the Bayer order based on the flips, but does not define > > the control correctly with the V4L2_CTRL_FLAG_MODIFY_LAYOUT flag. > > > > Add the V4L2_CTRL_FLAG_MODIFY_LAYOUT flag. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > --- > > drivers/media/i2c/imx319.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c index > > 245a18fb40ad..45b1b61b2880 100644 > > --- a/drivers/media/i2c/imx319.c > > +++ b/drivers/media/i2c/imx319.c > > @@ -2328,8 +2328,12 @@ static int imx319_init_controls(struct imx319 *imx319) > > > > imx319->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, > > V4L2_CID_HFLIP, 0, 1, 1, 0); > > + if (imx319->hflip) > > + imx319->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > imx319->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, > > V4L2_CID_VFLIP, 0, 1, 1, 0); > > + if (imx319->vflip) > > + imx319->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; > > > > v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, > > IMX319_ANA_GAIN_MIN, IMX319_ANA_GAIN_MAX, > > -- > > 2.34.1 >
Stevenson,
Thanks for your explanation.
Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c index 245a18fb40ad..45b1b61b2880 100644 --- a/drivers/media/i2c/imx319.c +++ b/drivers/media/i2c/imx319.c @@ -2328,8 +2328,12 @@ static int imx319_init_controls(struct imx319 *imx319) imx319->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0); + if (imx319->hflip) + imx319->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; imx319->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); + if (imx319->vflip) + imx319->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; v4l2_ctrl_new_std(ctrl_hdlr, &imx319_ctrl_ops, V4L2_CID_ANALOGUE_GAIN, IMX319_ANA_GAIN_MIN, IMX319_ANA_GAIN_MAX,