From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [185.203.201.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 59A792E8885 for ; Wed, 23 Jul 2025 13:48:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.203.201.7 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753278502; cv=none; b=YdmXXQAsbvsEXKumbNjuqtHPycCkJWyIqvh1slXgKjzkz2O2/np1mDf7URdY3e6agauGtOLgWNiXyhlNkYS9e3oalTCoAROrPiOH+Ll3alXrWCEpzCO8tjI+BJ/ONBkQL5DvINIu0k4PCPJ5Jcjgqv0fnXz5iMW7apQ361mu26Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753278502; c=relaxed/simple; bh=/+0+g8aDBMl+cpTpMeTuPc0fakIIDxsbYnyjeKL31FY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=WFbJUYwxuSMEWd2Ok20WKgO5Jkcmr48gPFeWgMOT+AMQZzv2lHWMNeynMx8wO6TMBM7kPdXOVhziPV+h6OuCugmV2XPWVcO5W7smy7IZrLz1cp83WQbm8hu00ufy56kOotB9CLdtW691zi1vZM+lJ2ae6WaAqvQxQRwxJFTP7a0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de; spf=pass smtp.mailfrom=pengutronix.de; arc=none smtp.client-ip=185.203.201.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pengutronix.de Received: from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77] helo=[127.0.0.1]) by metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1ueZpM-0003zw-SL; Wed, 23 Jul 2025 15:48:12 +0200 Message-ID: Date: Wed, 23 Jul 2025 15:48:11 +0200 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 0/2] parse horizontal/vertical flip properties To: Dave Stevenson , Jacopo Mondi Cc: Rob Herring , Conor Dooley , devicetree@vger.kernel.org, Jacopo Mondi , linux-kernel@vger.kernel.org, Sakari Ailus , entwicklung@pengutronix.de, Krzysztof Kozlowski , Mauro Carvalho Chehab , linux-media@vger.kernel.org, hansg@kernel.org References: <20250718-fpf-media-dt-flip-v1-0-75b3a938b4be@pengutronix.de> <35debf21-bca7-480f-a61e-7b0494f10ca5@pengutronix.de> <3ac271c7-a67a-4f6f-935d-256937516068@pengutronix.de> Content-Language: en-US, de-DE From: Fabian Pfitzner In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2a0a:edc0:0:900:1d::77 X-SA-Exim-Mail-From: f.pfitzner@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: devicetree@vger.kernel.org On 7/23/25 15:00, Dave Stevenson wrote: > Hi Jacopo and Fabian > > On Wed, 23 Jul 2025 at 13:21, Jacopo Mondi > wrote: >> Hi Fabian >> >> On Wed, Jul 23, 2025 at 12:09:58PM +0200, Fabian Pfitzner wrote: >>> On 7/23/25 11:44, Jacopo Mondi wrote: >>>> On Wed, Jul 23, 2025 at 11:29:27AM +0200, Fabian Pfitzner wrote: >>>>> On 7/23/25 11:17, Jacopo Mondi wrote: >>>>>> Hi Fabian >>>>>> >>>>>> On Wed, Jul 23, 2025 at 10:58:28AM +0200, Fabian Pfitzner wrote: >>>>>>> There are cameras containing a mirror on their optical path e. g. when >>>>>>> mounted upside down. >>>>>> How is this different from 'rotation = 180' ? >>>>> If you simply want to flip the output (e. g. horizontally), you cannot do >>>>> this with a rotation. >>>>> The camera I'm referring to is not only upside down, but also flipped >>>>> horizontally. >>>> 180 degress rotation = HFLIP + VFLIP >>> I do not want to do both. Only one of them. >>>> Yes, you can't express 'mirror' in DTS, because DTS are about the >>>> physical mounting rotation of the camera. Sensor drivers shall not >>>> apply any flip control automatically, it's userspace that by parsing >>>> the rotation property through the associated v4l2 controls should decide >>>> if it has to apply flips or not to correct the images. >>>> >>>> What is the use case you had in mind ? Tell the driver through a DTS >>>> property it has to apply flips to auto-compensate ? Because I think we >>>> shouldn't and if I'm not mistaken we also document it: >>>> https://www.kernel.org/doc/html/latest/userspace-api/media/drivers/camera-sensor.html#rotation-orientation-and-flipping >>> I have a camera that does a horizontal flip in its hardware, so the output >> Sorry, I don't want to be annoying, but what does it mean "does a >> horizontal flip in the hardware" ? >> >> In my understanding either "in hardware" means you can't control it >> from software (and so there's no point in telling drivers what to do) >> or you can control it from software and it's a regular HFLIP. > Can you say what this sensor/module is? ClairPixel 8320 > > To change flips due to physical sensor orientation is a very unusual > one. That would imply some weird mechanics in the sensor to add the > mirror and some form of orientation sensor being built in. Really? Imagine a door bell where an arbitrary camera is mounted such that it faces upwards (e. g. due to space limitations). Then you need a mirror in order to point into the "correct" direction. Fixing the driver for an arbitrary camera driver does not seem to be a good solution. > > The closest instance I can think of would be ov5647 where the sense of > the H & V flip register bits are in opposition, but that doesn't > change based on how the sensor is mounted. > In that case the driver just needs to account for it when programming > those registers [1]. And I now note that I haven't upstreamed the > patch adding flip controls - another one for the to-do list. The > hardcoded register set in the mainline driver sets HFLIP (0x3821 bit > 2) but not VFLIP (0x3820 bit 2) [2]. > > Dave > > [1] https://github.com/raspberrypi/linux/commit/9e5d3fd3f47e91806a5c26f96732284f39098a58 > [2] https://elixir.bootlin.com/linux/v6.15.7/source/drivers/media/i2c/ov5647.c#L153 > >>> is not what I want. My example above was misleading. The rotation fixes the >>> "upside down" problem, but does not fix the flip. >>> >>> Doing that in userspace might be a solution, but in my opinion it is a bit >>> ugly to write a script that always sets the flip property from userspace >>> when the device was started. >>> A much cleaner way would be to simply set this property in the device tree >>> such that the driver can be initially configured with the proper values. >> Sorry, don't agree here. What if a sensor is mounted 90/270 degrees >> rotated (typical for mobile devices in example) ? You can't compensate >> it completely with flips, would you 270+HFLIP=90 ? would you leave it >> unmodified ? Userspace has to know and act accordingly, doing things >> in driver (will all drivers behave the same ? Will some compensate or >> other won't ?) is a recipe for more complex behaviours to handle. >> >>> PS: I have to send this email twice. The first one contained HTML parts that >>> were rejected by some receivers... >>> >>>> TL;DR drivers shall not flip, userspace should. Mirroring is an effect >>>> of drivers applying an HFLIP, because unless I'm missing something >>>> obvious, 'mirror' is not a physical mounting configuration of the camera >>>> sensor. >>>> >>>> FIY we're talking about something similar in libcamera >>>> https://lists.libcamera.org/pipermail/libcamera-devel/2025-July/051533.html >>>> >>>>>>> Introduce two options to change the device's flip property via device tree. >>>>>>> >>>>>>> As there is already support for the panel-common driver [1], add it for cameras in the same way. >>>>>>> >>>>>>> [1] commit 3c0ecd83eee9 ("dt-bindings: display: panel: Move flip properties to panel-common") >>>>>>> >>>>>>> Signed-off-by: Fabian Pfitzner >>>>>>> --- >>>>>>> Fabian Pfitzner (2): >>>>>>> media: dt-bindings: add flip properties >>>>>>> media: v4l: fwnode: parse horizontal/vertical flip properties >>>>>>> >>>>>>> .../devicetree/bindings/media/video-interface-devices.yaml | 8 ++++++++ >>>>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 3 +++ >>>>>>> include/media/v4l2-fwnode.h | 4 ++++ >>>>>>> 3 files changed, 15 insertions(+) >>>>>>> --- >>>>>>> base-commit: 6832a9317eee280117cd695fa885b2b7a7a38daf >>>>>>> change-id: 20250718-fpf-media-dt-flip-7fcad30bcfb7 >>>>>>> >>>>>>> Best regards, >>>>>>> -- >>>>>>> Fabian Pfitzner >>>>>>> >>>>> -- >>>>> Pengutronix e.K. | Fabian Pfitzner | >>>>> Steuerwalder Str. 21 | https://www.pengutronix.de/ | >>>>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >>>>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | >>>>> >>> -- >>> Pengutronix e.K. | Fabian Pfitzner | >>> Steuerwalder Str. 21 | https://www.pengutronix.de/ | >>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 | >>> > -- Pengutronix e.K. | Fabian Pfitzner | Steuerwalder Str. 21 | https://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |